From 47d0d1c718481614f883976a06e19de3181696be Mon Sep 17 00:00:00 2001
From: Andrew Lamb <andrew@nerdnetworks.org>
Date: Tue, 28 Nov 2023 17:38:42 -0500
Subject: [PATCH] Minor: Make it easier to work with Expr::ScalarFunction

---
 datafusion/core/src/physical_planner.rs           |  6 +++---
 datafusion/expr/src/expr.rs                       | 15 ++++++++++-----
 datafusion/substrait/src/logical_plan/producer.rs | 12 ++++++------
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/datafusion/core/src/physical_planner.rs b/datafusion/core/src/physical_planner.rs
index 09f0e11dc2b5..8eabc47779da 100644
--- a/datafusion/core/src/physical_planner.rs
+++ b/datafusion/core/src/physical_planner.rs
@@ -217,13 +217,13 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result<String> {
 
             Ok(name)
         }
-        Expr::ScalarFunction(expr::ScalarFunction { func_def, args }) => {
+        Expr::ScalarFunction(fun) => {
             // function should be resolved during `AnalyzerRule`s
-            if let ScalarFunctionDefinition::Name(_) = func_def {
+            if let ScalarFunctionDefinition::Name(_) = fun.func_def {
                 return internal_err!("Function `Expr` with name should be resolved.");
             }
 
-            create_function_physical_name(func_def.name(), false, args)
+            create_function_physical_name(fun.name(), false, &fun.args)
         }
         Expr::WindowFunction(WindowFunction { fun, args, .. }) => {
             create_function_physical_name(&fun.to_string(), false, args)
diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs
index 13e488dac042..b46d204faafb 100644
--- a/datafusion/expr/src/expr.rs
+++ b/datafusion/expr/src/expr.rs
@@ -362,6 +362,13 @@ pub struct ScalarFunction {
     pub args: Vec<Expr>,
 }
 
+impl ScalarFunction {
+    // return the Function's name
+    pub fn name(&self) -> &str {
+        self.func_def.name()
+    }
+}
+
 impl ScalarFunctionDefinition {
     /// Function's name for display
     pub fn name(&self) -> &str {
@@ -1219,8 +1226,8 @@ impl fmt::Display for Expr {
                     write!(f, " NULLS LAST")
                 }
             }
-            Expr::ScalarFunction(ScalarFunction { func_def, args }) => {
-                fmt_function(f, func_def.name(), false, args, true)
+            Expr::ScalarFunction(fun) => {
+                fmt_function(f, fun.name(), false, &fun.args, true)
             }
             Expr::WindowFunction(WindowFunction {
                 fun,
@@ -1552,9 +1559,7 @@ fn create_name(e: &Expr) -> Result<String> {
                 }
             }
         }
-        Expr::ScalarFunction(ScalarFunction { func_def, args }) => {
-            create_function_name(func_def.name(), false, args)
-        }
+        Expr::ScalarFunction(fun) => create_function_name(fun.name(), false, &fun.args),
         Expr::WindowFunction(WindowFunction {
             fun,
             args,
diff --git a/datafusion/substrait/src/logical_plan/producer.rs b/datafusion/substrait/src/logical_plan/producer.rs
index 95604e6d2db9..2be3e7b4e884 100644
--- a/datafusion/substrait/src/logical_plan/producer.rs
+++ b/datafusion/substrait/src/logical_plan/producer.rs
@@ -33,8 +33,8 @@ use datafusion::common::{exec_err, internal_err, not_impl_err};
 #[allow(unused_imports)]
 use datafusion::logical_expr::aggregate_function;
 use datafusion::logical_expr::expr::{
-    Alias, BinaryExpr, Case, Cast, GroupingSet, InList,
-    ScalarFunction as DFScalarFunction, ScalarFunctionDefinition, Sort, WindowFunction,
+    Alias, BinaryExpr, Case, Cast, GroupingSet, InList, ScalarFunctionDefinition, Sort,
+    WindowFunction,
 };
 use datafusion::logical_expr::{expr, Between, JoinConstraint, LogicalPlan, Operator};
 use datafusion::prelude::Expr;
@@ -822,9 +822,9 @@ pub fn to_substrait_rex(
                 Ok(substrait_or_list)
             }
         }
-        Expr::ScalarFunction(DFScalarFunction { func_def, args }) => {
+        Expr::ScalarFunction(fun) => {
             let mut arguments: Vec<FunctionArgument> = vec![];
-            for arg in args {
+            for arg in &fun.args {
                 arguments.push(FunctionArgument {
                     arg_type: Some(ArgType::Value(to_substrait_rex(
                         arg,
@@ -836,12 +836,12 @@ pub fn to_substrait_rex(
             }
 
             // function should be resolved during `AnalyzerRule`
-            if let ScalarFunctionDefinition::Name(_) = func_def {
+            if let ScalarFunctionDefinition::Name(_) = fun.func_def {
                 return internal_err!("Function `Expr` with name should be resolved.");
             }
 
             let function_anchor =
-                _register_function(func_def.name().to_string(), extension_info);
+                _register_function(fun.name().to_string(), extension_info);
             Ok(Expression {
                 rex_type: Some(RexType::ScalarFunction(ScalarFunction {
                     function_reference: function_anchor,