diff --git a/datafusion/core/src/datasource/listing/helpers.rs b/datafusion/core/src/datasource/listing/helpers.rs index f9b02f4d0c10..2bb7567e32e5 100644 --- a/datafusion/core/src/datasource/listing/helpers.rs +++ b/datafusion/core/src/datasource/listing/helpers.rs @@ -92,7 +92,7 @@ pub fn expr_applicable_for_cols(col_names: &[String], expr: &Expr) -> bool { Expr::ScalarFunction(scalar_function) => { match &scalar_function.func_def { - ScalarFunctionDefinition::BuiltIn { fun, .. } => { + ScalarFunctionDefinition::BuiltIn(fun) => { match fun.volatility() { Volatility::Immutable => Ok(VisitRecursion::Continue), // TODO: Stable functions could be `applicable`, but that would require access to the context diff --git a/datafusion/expr/src/built_in_function.rs b/datafusion/expr/src/built_in_function.rs index 53f9e850d303..d0ad271d3463 100644 --- a/datafusion/expr/src/built_in_function.rs +++ b/datafusion/expr/src/built_in_function.rs @@ -346,6 +346,12 @@ impl BuiltinScalarFunction { self.signature().type_signature.supports_zero_argument() } + /// Returns the name of this function + pub fn name(&self) -> &str { + // .unwrap is safe here because compiler makes sure the map will have matches for each BuiltinScalarFunction + function_to_name().get(self).unwrap() + } + /// Returns the [Volatility] of the builtin function. pub fn volatility(&self) -> Volatility { match self { @@ -1616,8 +1622,7 @@ impl BuiltinScalarFunction { impl fmt::Display for BuiltinScalarFunction { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - // .unwrap is safe here because compiler makes sure the map will have matches for each BuiltinScalarFunction - write!(f, "{}", function_to_name().get(self).unwrap()) + write!(f, "{}", self.name()) } } diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index b46d204faafb..d0c18bd42b84 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -17,7 +17,6 @@ //! Expr module contains core type definition for `Expr`. -use crate::built_in_function; use crate::expr_fn::binary_expr; use crate::logical_plan::Subquery; use crate::udaf; @@ -26,6 +25,7 @@ use crate::window_frame; use crate::window_function; use crate::Operator; use crate::{aggregate_function, ExprSchemable}; +use crate::{built_in_function, BuiltinScalarFunction}; use arrow::datatypes::DataType; use datafusion_common::tree_node::{Transformed, TreeNode}; use datafusion_common::{internal_err, DFSchema, OwnedTableReference}; @@ -342,10 +342,7 @@ pub enum ScalarFunctionDefinition { /// Resolved to a `BuiltinScalarFunction` /// There is plan to migrate `BuiltinScalarFunction` to UDF-based implementation (issue#8045) /// This variant is planned to be removed in long term - BuiltIn { - fun: built_in_function::BuiltinScalarFunction, - name: Arc, - }, + BuiltIn(BuiltinScalarFunction), /// Resolved to a user defined function UDF(Arc), /// A scalar function constructed with name. This variant can not be executed directly @@ -373,7 +370,7 @@ impl ScalarFunctionDefinition { /// Function's name for display pub fn name(&self) -> &str { match self { - ScalarFunctionDefinition::BuiltIn { name, .. } => name.as_ref(), + ScalarFunctionDefinition::BuiltIn(fun) => fun.name(), ScalarFunctionDefinition::UDF(udf) => udf.name(), ScalarFunctionDefinition::Name(func_name) => func_name.as_ref(), } @@ -384,10 +381,7 @@ impl ScalarFunction { /// Create a new ScalarFunction expression pub fn new(fun: built_in_function::BuiltinScalarFunction, args: Vec) -> Self { Self { - func_def: ScalarFunctionDefinition::BuiltIn { - fun, - name: Arc::from(fun.to_string()), - }, + func_def: ScalarFunctionDefinition::BuiltIn(fun), args, } } diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs index d2c5e5cddbf3..66c9bb30a0a9 100644 --- a/datafusion/expr/src/expr_fn.rs +++ b/datafusion/expr/src/expr_fn.rs @@ -1031,7 +1031,7 @@ mod test { macro_rules! test_unary_scalar_expr { ($ENUM:ident, $FUNC:ident) => {{ if let Expr::ScalarFunction(ScalarFunction { - func_def: ScalarFunctionDefinition::BuiltIn { fun, .. }, + func_def: ScalarFunctionDefinition::BuiltIn(fun), args, }) = $FUNC(col("tableA.a")) { @@ -1052,7 +1052,7 @@ mod test { col(stringify!($arg.to_string())) ),* ); - if let Expr::ScalarFunction(ScalarFunction { func_def: ScalarFunctionDefinition::BuiltIn{fun, ..}, args }) = result { + if let Expr::ScalarFunction(ScalarFunction { func_def: ScalarFunctionDefinition::BuiltIn(fun), args }) = result { let name = built_in_function::BuiltinScalarFunction::$ENUM; assert_eq!(name, fun); assert_eq!(expected.len(), args.len()); @@ -1072,7 +1072,7 @@ mod test { ),* ] ); - if let Expr::ScalarFunction(ScalarFunction { func_def: ScalarFunctionDefinition::BuiltIn{fun, ..}, args }) = result { + if let Expr::ScalarFunction(ScalarFunction { func_def: ScalarFunctionDefinition::BuiltIn(fun), args }) = result { let name = built_in_function::BuiltinScalarFunction::$ENUM; assert_eq!(name, fun); assert_eq!(expected.len(), args.len()); @@ -1212,7 +1212,7 @@ mod test { #[test] fn uuid_function_definitions() { if let Expr::ScalarFunction(ScalarFunction { - func_def: ScalarFunctionDefinition::BuiltIn { fun, .. }, + func_def: ScalarFunctionDefinition::BuiltIn(fun), args, }) = uuid() { @@ -1227,7 +1227,7 @@ mod test { #[test] fn digest_function_definitions() { if let Expr::ScalarFunction(ScalarFunction { - func_def: ScalarFunctionDefinition::BuiltIn { fun, .. }, + func_def: ScalarFunctionDefinition::BuiltIn(fun), args, }) = digest(col("tableA.a"), lit("md5")) { @@ -1242,7 +1242,7 @@ mod test { #[test] fn encode_function_definitions() { if let Expr::ScalarFunction(ScalarFunction { - func_def: ScalarFunctionDefinition::BuiltIn { fun, .. }, + func_def: ScalarFunctionDefinition::BuiltIn(fun), args, }) = encode(col("tableA.a"), lit("base64")) { @@ -1257,7 +1257,7 @@ mod test { #[test] fn decode_function_definitions() { if let Expr::ScalarFunction(ScalarFunction { - func_def: ScalarFunctionDefinition::BuiltIn { fun, .. }, + func_def: ScalarFunctionDefinition::BuiltIn(fun), args, }) = decode(col("tableA.a"), lit("hex")) { diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index d5d9c848b2e9..e13765353d58 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -84,7 +84,7 @@ impl ExprSchemable for Expr { | Expr::TryCast(TryCast { data_type, .. }) => Ok(data_type.clone()), Expr::ScalarFunction(ScalarFunction { func_def, args }) => { match func_def { - ScalarFunctionDefinition::BuiltIn { fun, .. } => { + ScalarFunctionDefinition::BuiltIn(fun) => { let arg_data_types = args .iter() .map(|e| e.get_type(schema)) diff --git a/datafusion/expr/src/tree_node/expr.rs b/datafusion/expr/src/tree_node/expr.rs index 474b5f7689b9..f50939a8e66d 100644 --- a/datafusion/expr/src/tree_node/expr.rs +++ b/datafusion/expr/src/tree_node/expr.rs @@ -277,7 +277,7 @@ impl TreeNode for Expr { nulls_first, )), Expr::ScalarFunction(ScalarFunction { func_def, args }) => match func_def { - ScalarFunctionDefinition::BuiltIn { fun, .. } => Expr::ScalarFunction( + ScalarFunctionDefinition::BuiltIn(fun) => Expr::ScalarFunction( ScalarFunction::new(fun, transform_vec(args, &mut transform)?), ), ScalarFunctionDefinition::UDF(fun) => Expr::ScalarFunction( diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs b/datafusion/optimizer/src/analyzer/type_coercion.rs index eb5d8c53a5e0..e70f4f3674f5 100644 --- a/datafusion/optimizer/src/analyzer/type_coercion.rs +++ b/datafusion/optimizer/src/analyzer/type_coercion.rs @@ -320,7 +320,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter { Ok(Expr::Case(case)) } Expr::ScalarFunction(ScalarFunction { func_def, args }) => match func_def { - ScalarFunctionDefinition::BuiltIn { fun, .. } => { + ScalarFunctionDefinition::BuiltIn(fun) => { let new_args = coerce_arguments_for_signature( args.as_slice(), &self.schema, diff --git a/datafusion/optimizer/src/optimize_projections.rs b/datafusion/optimizer/src/optimize_projections.rs index b6d026279aa6..bbf704a83c55 100644 --- a/datafusion/optimizer/src/optimize_projections.rs +++ b/datafusion/optimizer/src/optimize_projections.rs @@ -510,9 +510,7 @@ fn rewrite_expr(expr: &Expr, input: &Projection) -> Result> { ))) } Expr::ScalarFunction(scalar_fn) => { - let fun = if let ScalarFunctionDefinition::BuiltIn { fun, .. } = - scalar_fn.func_def - { + let fun = if let ScalarFunctionDefinition::BuiltIn(fun) = scalar_fn.func_def { fun } else { return Ok(None); diff --git a/datafusion/optimizer/src/push_down_filter.rs b/datafusion/optimizer/src/push_down_filter.rs index 95eeee931b4f..d772e293b057 100644 --- a/datafusion/optimizer/src/push_down_filter.rs +++ b/datafusion/optimizer/src/push_down_filter.rs @@ -980,7 +980,7 @@ fn is_volatile_expression(e: &Expr) -> bool { e.apply(&mut |expr| { Ok(match expr { Expr::ScalarFunction(f) => match &f.func_def { - ScalarFunctionDefinition::BuiltIn { fun, .. } + ScalarFunctionDefinition::BuiltIn(fun) if fun.volatility() == Volatility::Volatile => { is_volatile = true; diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs index 3310bfed75bf..6a36b218b3b8 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -345,7 +345,7 @@ impl<'a> ConstEvaluator<'a> { | Expr::Wildcard { .. } | Expr::Placeholder(_) => false, Expr::ScalarFunction(ScalarFunction { func_def, .. }) => match func_def { - ScalarFunctionDefinition::BuiltIn { fun, .. } => { + ScalarFunctionDefinition::BuiltIn(fun) => { Self::volatility_ok(fun.volatility()) } ScalarFunctionDefinition::UDF(fun) => { @@ -1203,41 +1203,28 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for Simplifier<'a, S> { // log Expr::ScalarFunction(ScalarFunction { - func_def: - ScalarFunctionDefinition::BuiltIn { - fun: BuiltinScalarFunction::Log, - .. - }, + func_def: ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Log), args, }) => simpl_log(args, <&S>::clone(&info))?, // power Expr::ScalarFunction(ScalarFunction { - func_def: - ScalarFunctionDefinition::BuiltIn { - fun: BuiltinScalarFunction::Power, - .. - }, + func_def: ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Power), args, }) => simpl_power(args, <&S>::clone(&info))?, // concat Expr::ScalarFunction(ScalarFunction { - func_def: - ScalarFunctionDefinition::BuiltIn { - fun: BuiltinScalarFunction::Concat, - .. - }, + func_def: ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Concat), args, }) => simpl_concat(args)?, // concat_ws Expr::ScalarFunction(ScalarFunction { func_def: - ScalarFunctionDefinition::BuiltIn { - fun: BuiltinScalarFunction::ConcatWithSeparator, - .. - }, + ScalarFunctionDefinition::BuiltIn( + BuiltinScalarFunction::ConcatWithSeparator, + ), args, }) => match &args[..] { [delimiter, vals @ ..] => simpl_concat_ws(delimiter, vals)?, diff --git a/datafusion/optimizer/src/simplify_expressions/utils.rs b/datafusion/optimizer/src/simplify_expressions/utils.rs index e69207b6889a..fa91a3ace2a2 100644 --- a/datafusion/optimizer/src/simplify_expressions/utils.rs +++ b/datafusion/optimizer/src/simplify_expressions/utils.rs @@ -365,11 +365,7 @@ pub fn simpl_log(current_args: Vec, info: &dyn SimplifyInfo) -> Result Ok(args[1].clone()), _ => { @@ -409,11 +405,7 @@ pub fn simpl_power(current_args: Vec, info: &dyn SimplifyInfo) -> Result Ok(args[1].clone()), _ => Ok(Expr::ScalarFunction(ScalarFunction::new( diff --git a/datafusion/physical-expr/src/planner.rs b/datafusion/physical-expr/src/planner.rs index 5c5cc8e36fa7..5501647da2c3 100644 --- a/datafusion/physical-expr/src/planner.rs +++ b/datafusion/physical-expr/src/planner.rs @@ -349,7 +349,7 @@ pub fn create_physical_expr( } Expr::ScalarFunction(ScalarFunction { func_def, args }) => match func_def { - ScalarFunctionDefinition::BuiltIn { fun, .. } => { + ScalarFunctionDefinition::BuiltIn(fun) => { let physical_args = args .iter() .map(|e| { diff --git a/datafusion/proto/src/logical_plan/to_proto.rs b/datafusion/proto/src/logical_plan/to_proto.rs index 9be4a532bb5b..fd7b73487740 100644 --- a/datafusion/proto/src/logical_plan/to_proto.rs +++ b/datafusion/proto/src/logical_plan/to_proto.rs @@ -757,7 +757,7 @@ impl TryFrom<&Expr> for protobuf::LogicalExprNode { )) } Expr::ScalarFunction(ScalarFunction { func_def, args }) => match func_def { - ScalarFunctionDefinition::BuiltIn { fun, .. } => { + ScalarFunctionDefinition::BuiltIn(fun) => { let fun: protobuf::ScalarFunction = fun.try_into()?; let args: Vec = args .iter() diff --git a/datafusion/sql/src/expr/value.rs b/datafusion/sql/src/expr/value.rs index f33e9e8ddf78..a3f29da488ba 100644 --- a/datafusion/sql/src/expr/value.rs +++ b/datafusion/sql/src/expr/value.rs @@ -144,7 +144,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { values.push(value); } Expr::ScalarFunction(ScalarFunction { - func_def: ScalarFunctionDefinition::BuiltIn { fun, .. }, + func_def: ScalarFunctionDefinition::BuiltIn(fun), .. }) => { if fun == BuiltinScalarFunction::MakeArray {