From aafa67620a0b159991c3a3cb587ba8dac4239721 Mon Sep 17 00:00:00 2001 From: Eren Avsarogullari Date: Sat, 13 Apr 2024 23:27:14 -0700 Subject: [PATCH 1/3] Issue-9939 - Move ceil, exp, factorial to datafusion-functions crate --- datafusion/expr/src/built_in_function.rs | 44 +------ datafusion/expr/src/expr_fn.rs | 14 --- datafusion/functions/src/math/factorial.rs | 118 ++++++++++++++++++ datafusion/functions/src/math/mod.rs | 22 ++++ datafusion/physical-expr/src/functions.rs | 9 +- .../physical-expr/src/math_expressions.rs | 105 +--------------- datafusion/proto/proto/datafusion.proto | 6 +- datafusion/proto/src/generated/pbjson.rs | 9 -- datafusion/proto/src/generated/prost.rs | 12 +- .../proto/src/logical_plan/from_proto.rs | 12 +- datafusion/proto/src/logical_plan/to_proto.rs | 3 - datafusion/sql/src/expr/function.rs | 11 -- datafusion/sql/src/expr/mod.rs | 12 +- 13 files changed, 155 insertions(+), 222 deletions(-) create mode 100644 datafusion/functions/src/math/factorial.rs diff --git a/datafusion/expr/src/built_in_function.rs b/datafusion/expr/src/built_in_function.rs index 43cb0c3e0a50..5bfec00ea3b3 100644 --- a/datafusion/expr/src/built_in_function.rs +++ b/datafusion/expr/src/built_in_function.rs @@ -37,14 +37,8 @@ use strum_macros::EnumIter; #[derive(Debug, Clone, PartialEq, Eq, Hash, EnumIter, Copy)] pub enum BuiltinScalarFunction { // math functions - /// ceil - Ceil, /// coalesce Coalesce, - /// exp - Exp, - /// factorial - Factorial, // string functions /// concat Concat, @@ -106,10 +100,7 @@ impl BuiltinScalarFunction { pub fn volatility(&self) -> Volatility { match self { // Immutable scalar builtins - BuiltinScalarFunction::Ceil => Volatility::Immutable, BuiltinScalarFunction::Coalesce => Volatility::Immutable, - BuiltinScalarFunction::Exp => Volatility::Immutable, - BuiltinScalarFunction::Factorial => Volatility::Immutable, BuiltinScalarFunction::Concat => Volatility::Immutable, BuiltinScalarFunction::ConcatWithSeparator => Volatility::Immutable, BuiltinScalarFunction::EndsWith => Volatility::Immutable, @@ -145,15 +136,6 @@ impl BuiltinScalarFunction { utf8_to_str_type(&input_expr_types[0], "initcap") } BuiltinScalarFunction::EndsWith => Ok(Boolean), - - BuiltinScalarFunction::Factorial => Ok(Int64), - - BuiltinScalarFunction::Ceil | BuiltinScalarFunction::Exp => { - match input_expr_types[0] { - Float32 => Ok(Float32), - _ => Ok(Float64), - } - } } } @@ -185,17 +167,6 @@ impl BuiltinScalarFunction { ], self.volatility(), ), - BuiltinScalarFunction::Factorial => { - Signature::uniform(1, vec![Int64], self.volatility()) - } - BuiltinScalarFunction::Ceil | BuiltinScalarFunction::Exp => { - // math expressions expect 1 argument of type f64 or f32 - // priority is given to f64 because e.g. `sqrt(1i32)` is in IR (real numbers) and thus we - // return the best approximation for it (in f64). - // We accept f32 because in this case it is clear that the best approximation - // will be as good as the number of digits in the number - Signature::uniform(1, vec![Float64, Float32], self.volatility()) - } } } @@ -203,25 +174,12 @@ impl BuiltinScalarFunction { /// The list can be extended, only mathematical and datetime functions are /// considered for the initial implementation of this feature. pub fn monotonicity(&self) -> Option { - if matches!( - &self, - BuiltinScalarFunction::Ceil - | BuiltinScalarFunction::Exp - | BuiltinScalarFunction::Factorial - ) { - Some(vec![Some(true)]) - } else { - None - } + None } /// Returns all names that can be used to call this function pub fn aliases(&self) -> &'static [&'static str] { match self { - BuiltinScalarFunction::Ceil => &["ceil"], - BuiltinScalarFunction::Exp => &["exp"], - BuiltinScalarFunction::Factorial => &["factorial"], - // conditional functions BuiltinScalarFunction::Coalesce => &["coalesce"], diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs index 6a28275ebfcf..448b13165935 100644 --- a/datafusion/expr/src/expr_fn.rs +++ b/datafusion/expr/src/expr_fn.rs @@ -525,16 +525,6 @@ macro_rules! nary_scalar_expr { // generate methods for creating the supported unary/binary expressions // math functions -scalar_expr!(Factorial, factorial, num, "factorial"); -scalar_expr!( - Ceil, - ceil, - num, - "nearest integer greater than or equal to argument" -); - -scalar_expr!(Exp, exp, num, "exponential"); - scalar_expr!(InitCap, initcap, string, "converts the first letter of each word in `string` in uppercase and the remaining characters in lowercase"); scalar_expr!(EndsWith, ends_with, string suffix, "whether the `string` ends with the `suffix`"); nary_scalar_expr!(Coalesce, coalesce, "returns `coalesce(args...)`, which evaluates to the value of the first [Expr] which is not NULL"); @@ -913,10 +903,6 @@ mod test { #[test] fn scalar_function_definitions() { - test_unary_scalar_expr!(Factorial, factorial); - test_unary_scalar_expr!(Ceil, ceil); - test_unary_scalar_expr!(Exp, exp); - test_scalar_expr!(InitCap, initcap, string); test_scalar_expr!(EndsWith, ends_with, string, characters); } diff --git a/datafusion/functions/src/math/factorial.rs b/datafusion/functions/src/math/factorial.rs new file mode 100644 index 000000000000..1d3b3fc26ea9 --- /dev/null +++ b/datafusion/functions/src/math/factorial.rs @@ -0,0 +1,118 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use arrow::array::{ArrayRef, Int64Array}; +use std::any::Any; +use std::sync::Arc; + +use arrow::datatypes::DataType; +use arrow::datatypes::DataType::Int64; + +use crate::utils::make_scalar_function; +use datafusion_common::{exec_err, DataFusionError, Result}; +use datafusion_expr::{ColumnarValue, ScalarUDFImpl, Signature, Volatility}; + +#[derive(Debug)] +pub struct FactorialFunc { + signature: Signature, +} + +impl Default for FactorialFunc { + fn default() -> Self { + FactorialFunc::new() + } +} + +impl FactorialFunc { + pub fn new() -> Self { + Self { + signature: Signature::uniform(1, vec![Int64], Volatility::Volatile), + } + } +} + +impl ScalarUDFImpl for FactorialFunc { + fn as_any(&self) -> &dyn Any { + self + } + + fn name(&self) -> &str { + "factorial" + } + + fn signature(&self) -> &Signature { + &self.signature + } + + fn return_type(&self, _arg_types: &[DataType]) -> Result { + Ok(Int64) + } + + fn invoke(&self, args: &[ColumnarValue]) -> Result { + make_scalar_function(factorial, vec![])(args) + } +} + +macro_rules! make_function_scalar_inputs { + ($ARG: expr, $NAME:expr, $ARRAY_TYPE:ident, $FUNC: block) => {{ + let arg = downcast_arg!($ARG, $NAME, $ARRAY_TYPE); + + arg.iter() + .map(|a| match a { + Some(a) => Some($FUNC(a)), + _ => None, + }) + .collect::<$ARRAY_TYPE>() + }}; +} + +/// Factorial SQL function +fn factorial(args: &[ArrayRef]) -> Result { + match args[0].data_type() { + DataType::Int64 => Ok(Arc::new(make_function_scalar_inputs!( + &args[0], + "value", + Int64Array, + { |value: i64| { (1..=value).product() } } + )) as ArrayRef), + other => exec_err!("Unsupported data type {other:?} for function factorial."), + } +} + +#[cfg(test)] +mod test { + use arrow::array::Float64Array; + + use datafusion_common::cast::as_int64_array; + + use super::*; + + #[test] + fn test_factorial_i64() { + let args: Vec = vec![ + Arc::new(Int64Array::from(vec![0, 1, 2, 4])), // input + ]; + + let result = factorial(&args).expect("failed to initialize function factorial"); + let ints = + as_int64_array(&result).expect("failed to initialize function factorial"); + + let expected = Int64Array::from(vec![1, 1, 2, 24]); + + assert_eq!(ints, &expected); + } +} diff --git a/datafusion/functions/src/math/mod.rs b/datafusion/functions/src/math/mod.rs index c83a98cb1913..b6e8d26b6460 100644 --- a/datafusion/functions/src/math/mod.rs +++ b/datafusion/functions/src/math/mod.rs @@ -22,6 +22,7 @@ use std::sync::Arc; pub mod abs; pub mod cot; +pub mod factorial; pub mod gcd; pub mod iszero; pub mod lcm; @@ -44,10 +45,13 @@ make_math_unary_udf!(AtanFunc, ATAN, atan, atan, Some(vec![Some(true)])); make_math_unary_udf!(AtanhFunc, ATANH, atanh, atanh, Some(vec![Some(true)])); make_math_binary_udf!(Atan2, ATAN2, atan2, atan2, Some(vec![Some(true)])); make_math_unary_udf!(CbrtFunc, CBRT, cbrt, cbrt, None); +make_math_unary_udf!(CeilFunc, CEIL, ceil, ceil, Some(vec![Some(true)])); make_math_unary_udf!(CosFunc, COS, cos, cos, None); make_math_unary_udf!(CoshFunc, COSH, cosh, cosh, None); make_udf_function!(cot::CotFunc, COT, cot); make_math_unary_udf!(DegreesFunc, DEGREES, degrees, to_degrees, None); +make_math_unary_udf!(ExpFunc, EXP, exp, exp, Some(vec![Some(true)])); +make_udf_function!(factorial::FactorialFunc, FACTORIAL, factorial); make_math_unary_udf!(FloorFunc, FLOOR, floor, floor, Some(vec![Some(true)])); make_udf_function!(log::LogFunc, LOG, log); make_udf_function!(gcd::GcdFunc, GCD, gcd); @@ -119,6 +123,11 @@ pub mod expr_fn { super::cbrt().call(vec![num]) } + #[doc = "nearest integer greater than or equal to argument"] + pub fn ceil(num: Expr) -> Expr { + super::ceil().call(vec![num]) + } + #[doc = "cosine"] pub fn cos(num: Expr) -> Expr { super::cos().call(vec![num]) @@ -139,6 +148,16 @@ pub mod expr_fn { super::degrees().call(vec![num]) } + #[doc = "exponential"] + pub fn exp(num: Expr) -> Expr { + super::exp().call(vec![num]) + } + + #[doc = "factorial"] + pub fn factorial(num: Expr) -> Expr { + super::factorial().call(vec![num]) + } + #[doc = "nearest integer less than or equal to argument"] pub fn floor(num: Expr) -> Expr { super::floor().call(vec![num]) @@ -262,10 +281,13 @@ pub fn functions() -> Vec> { atan2(), atanh(), cbrt(), + ceil(), cos(), cosh(), cot(), degrees(), + exp(), + factorial(), floor(), gcd(), isnan(), diff --git a/datafusion/physical-expr/src/functions.rs b/datafusion/physical-expr/src/functions.rs index c237e2070675..6756b8622316 100644 --- a/datafusion/physical-expr/src/functions.rs +++ b/datafusion/physical-expr/src/functions.rs @@ -50,8 +50,7 @@ use datafusion_expr::{ use crate::sort_properties::SortProperties; use crate::{ - conditional_expressions, math_expressions, string_expressions, PhysicalExpr, - ScalarFunctionExpr, + conditional_expressions, string_expressions, PhysicalExpr, ScalarFunctionExpr, }; /// Create a physical (function) expression. @@ -178,12 +177,6 @@ pub fn create_physical_fun( fun: &BuiltinScalarFunction, ) -> Result { Ok(match fun { - // math functions - BuiltinScalarFunction::Ceil => Arc::new(math_expressions::ceil), - BuiltinScalarFunction::Exp => Arc::new(math_expressions::exp), - BuiltinScalarFunction::Factorial => { - Arc::new(|args| make_scalar_function_inner(math_expressions::factorial)(args)) - } // string functions BuiltinScalarFunction::Coalesce => Arc::new(conditional_expressions::coalesce), BuiltinScalarFunction::Concat => Arc::new(string_expressions::concat), diff --git a/datafusion/physical-expr/src/math_expressions.rs b/datafusion/physical-expr/src/math_expressions.rs index 004a9abe7f0b..72aac9208b2c 100644 --- a/datafusion/physical-expr/src/math_expressions.rs +++ b/datafusion/physical-expr/src/math_expressions.rs @@ -21,69 +21,12 @@ use std::any::type_name; use std::sync::Arc; use arrow::array::ArrayRef; -use arrow::array::{BooleanArray, Float32Array, Float64Array, Int64Array}; +use arrow::array::{BooleanArray, Float32Array, Float64Array}; use arrow::datatypes::DataType; use arrow_array::Array; -use datafusion_common::{exec_err, ScalarValue}; +use datafusion_common::exec_err; use datafusion_common::{DataFusionError, Result}; -use datafusion_expr::ColumnarValue; - -macro_rules! downcast_compute_op { - ($ARRAY:expr, $NAME:expr, $FUNC:ident, $TYPE:ident) => {{ - let n = $ARRAY.as_any().downcast_ref::<$TYPE>(); - match n { - Some(array) => { - let res: $TYPE = - arrow::compute::kernels::arity::unary(array, |x| x.$FUNC()); - Ok(Arc::new(res)) - } - _ => exec_err!("Invalid data type for {}", $NAME), - } - }}; -} - -macro_rules! unary_primitive_array_op { - ($VALUE:expr, $NAME:expr, $FUNC:ident) => {{ - match ($VALUE) { - ColumnarValue::Array(array) => match array.data_type() { - DataType::Float32 => { - let result = downcast_compute_op!(array, $NAME, $FUNC, Float32Array); - Ok(ColumnarValue::Array(result?)) - } - DataType::Float64 => { - let result = downcast_compute_op!(array, $NAME, $FUNC, Float64Array); - Ok(ColumnarValue::Array(result?)) - } - other => { - exec_err!("Unsupported data type {:?} for function {}", other, $NAME) - } - }, - ColumnarValue::Scalar(a) => match a { - ScalarValue::Float32(a) => Ok(ColumnarValue::Scalar( - ScalarValue::Float32(a.map(|x| x.$FUNC())), - )), - ScalarValue::Float64(a) => Ok(ColumnarValue::Scalar( - ScalarValue::Float64(a.map(|x| x.$FUNC())), - )), - _ => exec_err!( - "Unsupported data type {:?} for function {}", - ($VALUE).data_type(), - $NAME - ), - }, - } - }}; -} - -macro_rules! math_unary_function { - ($NAME:expr, $FUNC:ident) => { - /// mathematical function that accepts f32 or f64 and returns f64 - pub fn $FUNC(args: &[ColumnarValue]) -> Result { - unary_primitive_array_op!(&args[0], $NAME, $FUNC) - } - }; -} macro_rules! downcast_arg { ($ARG:expr, $NAME:expr, $ARRAY_TYPE:ident) => {{ @@ -98,19 +41,6 @@ macro_rules! downcast_arg { }}; } -macro_rules! make_function_scalar_inputs { - ($ARG: expr, $NAME:expr, $ARRAY_TYPE:ident, $FUNC: block) => {{ - let arg = downcast_arg!($ARG, $NAME, $ARRAY_TYPE); - - arg.iter() - .map(|a| match a { - Some(a) => Some($FUNC(a)), - _ => None, - }) - .collect::<$ARRAY_TYPE>() - }}; -} - macro_rules! make_function_scalar_inputs_return_type { ($ARG: expr, $NAME:expr, $ARGS_TYPE:ident, $RETURN_TYPE:ident, $FUNC: block) => {{ let arg = downcast_arg!($ARG, $NAME, $ARGS_TYPE); @@ -124,22 +54,6 @@ macro_rules! make_function_scalar_inputs_return_type { }}; } -math_unary_function!("ceil", ceil); -math_unary_function!("exp", exp); - -/// Factorial SQL function -pub fn factorial(args: &[ArrayRef]) -> Result { - match args[0].data_type() { - DataType::Int64 => Ok(Arc::new(make_function_scalar_inputs!( - &args[0], - "value", - Int64Array, - { |value: i64| { (1..=value).product() } } - )) as ArrayRef), - other => exec_err!("Unsupported data type {other:?} for function factorial."), - } -} - /// Isnan SQL function pub fn isnan(args: &[ArrayRef]) -> Result { match args[0].data_type() { @@ -171,21 +85,6 @@ mod tests { use super::*; - #[test] - fn test_factorial_i64() { - let args: Vec = vec![ - Arc::new(Int64Array::from(vec![0, 1, 2, 4])), // input - ]; - - let result = factorial(&args).expect("failed to initialize function factorial"); - let ints = - as_int64_array(&result).expect("failed to initialize function factorial"); - - let expected = Int64Array::from(vec![1, 1, 2, 24]); - - assert_eq!(ints, &expected); - } - #[test] fn test_isnan_f64() { let args: Vec = vec![Arc::new(Float64Array::from(vec![ diff --git a/datafusion/proto/proto/datafusion.proto b/datafusion/proto/proto/datafusion.proto index e1bcf33b8254..6578c64cff1f 100644 --- a/datafusion/proto/proto/datafusion.proto +++ b/datafusion/proto/proto/datafusion.proto @@ -546,10 +546,10 @@ enum ScalarFunction { // 2 was Asin // 3 was Atan // 4 was Ascii - Ceil = 5; + // 5 was Ceil // 6 was Cos // 7 was Digest - Exp = 8; + // 8 was Exp // 9 was Floor // 10 was Ln // 11 was Log @@ -624,7 +624,7 @@ enum ScalarFunction { // 80 was Pi // 81 was Degrees // 82 was Radians - Factorial = 83; + // 83 was Factorial // 84 was Lcm // 85 was Gcd // 86 was ArrayAppend diff --git a/datafusion/proto/src/generated/pbjson.rs b/datafusion/proto/src/generated/pbjson.rs index 7beaeef0e58b..1546d75f2acd 100644 --- a/datafusion/proto/src/generated/pbjson.rs +++ b/datafusion/proto/src/generated/pbjson.rs @@ -22792,13 +22792,10 @@ impl serde::Serialize for ScalarFunction { { let variant = match self { Self::Unknown => "unknown", - Self::Ceil => "Ceil", - Self::Exp => "Exp", Self::Concat => "Concat", Self::ConcatWithSeparator => "ConcatWithSeparator", Self::InitCap => "InitCap", Self::Coalesce => "Coalesce", - Self::Factorial => "Factorial", Self::EndsWith => "EndsWith", }; serializer.serialize_str(variant) @@ -22812,13 +22809,10 @@ impl<'de> serde::Deserialize<'de> for ScalarFunction { { const FIELDS: &[&str] = &[ "unknown", - "Ceil", - "Exp", "Concat", "ConcatWithSeparator", "InitCap", "Coalesce", - "Factorial", "EndsWith", ]; @@ -22861,13 +22855,10 @@ impl<'de> serde::Deserialize<'de> for ScalarFunction { { match value { "unknown" => Ok(ScalarFunction::Unknown), - "Ceil" => Ok(ScalarFunction::Ceil), - "Exp" => Ok(ScalarFunction::Exp), "Concat" => Ok(ScalarFunction::Concat), "ConcatWithSeparator" => Ok(ScalarFunction::ConcatWithSeparator), "InitCap" => Ok(ScalarFunction::InitCap), "Coalesce" => Ok(ScalarFunction::Coalesce), - "Factorial" => Ok(ScalarFunction::Factorial), "EndsWith" => Ok(ScalarFunction::EndsWith), _ => Err(serde::de::Error::unknown_variant(value, FIELDS)), } diff --git a/datafusion/proto/src/generated/prost.rs b/datafusion/proto/src/generated/prost.rs index d6a27dbc5652..c752743cbdce 100644 --- a/datafusion/proto/src/generated/prost.rs +++ b/datafusion/proto/src/generated/prost.rs @@ -2846,10 +2846,10 @@ pub enum ScalarFunction { /// 2 was Asin /// 3 was Atan /// 4 was Ascii - Ceil = 5, + /// 5 was Ceil /// 6 was Cos /// 7 was Digest - Exp = 8, + /// 8 was Exp /// 9 was Floor /// 10 was Ln /// 11 was Log @@ -2924,7 +2924,7 @@ pub enum ScalarFunction { /// 80 was Pi /// 81 was Degrees /// 82 was Radians - Factorial = 83, + /// 83 was Factorial /// 84 was Lcm /// 85 was Gcd /// 86 was ArrayAppend @@ -2988,13 +2988,10 @@ impl ScalarFunction { pub fn as_str_name(&self) -> &'static str { match self { ScalarFunction::Unknown => "unknown", - ScalarFunction::Ceil => "Ceil", - ScalarFunction::Exp => "Exp", ScalarFunction::Concat => "Concat", ScalarFunction::ConcatWithSeparator => "ConcatWithSeparator", ScalarFunction::InitCap => "InitCap", ScalarFunction::Coalesce => "Coalesce", - ScalarFunction::Factorial => "Factorial", ScalarFunction::EndsWith => "EndsWith", } } @@ -3002,13 +2999,10 @@ impl ScalarFunction { pub fn from_str_name(value: &str) -> ::core::option::Option { match value { "unknown" => Some(Self::Unknown), - "Ceil" => Some(Self::Ceil), - "Exp" => Some(Self::Exp), "Concat" => Some(Self::Concat), "ConcatWithSeparator" => Some(Self::ConcatWithSeparator), "InitCap" => Some(Self::InitCap), "Coalesce" => Some(Self::Coalesce), - "Factorial" => Some(Self::Factorial), "EndsWith" => Some(Self::EndsWith), _ => None, } diff --git a/datafusion/proto/src/logical_plan/from_proto.rs b/datafusion/proto/src/logical_plan/from_proto.rs index 057690aacee6..4e61385bb545 100644 --- a/datafusion/proto/src/logical_plan/from_proto.rs +++ b/datafusion/proto/src/logical_plan/from_proto.rs @@ -37,9 +37,9 @@ use datafusion_expr::expr::Unnest; use datafusion_expr::expr::{Alias, Placeholder}; use datafusion_expr::window_frame::{check_window_frame, regularize_window_order_by}; use datafusion_expr::{ - ceil, coalesce, concat_expr, concat_ws_expr, ends_with, exp, + coalesce, concat_expr, concat_ws_expr, ends_with, expr::{self, InList, Sort, WindowFunction}, - factorial, initcap, + initcap, logical_plan::{PlanType, StringifiedPlan}, AggregateFunction, Between, BinaryExpr, BuiltInWindowFunction, BuiltinScalarFunction, Case, Cast, Expr, GetFieldAccess, GetIndexedField, GroupingSet, @@ -418,9 +418,6 @@ impl From<&protobuf::ScalarFunction> for BuiltinScalarFunction { use protobuf::ScalarFunction; match f { ScalarFunction::Unknown => todo!(), - ScalarFunction::Exp => Self::Exp, - ScalarFunction::Factorial => Self::Factorial, - ScalarFunction::Ceil => Self::Ceil, ScalarFunction::Concat => Self::Concat, ScalarFunction::ConcatWithSeparator => Self::ConcatWithSeparator, ScalarFunction::EndsWith => Self::EndsWith, @@ -1287,11 +1284,6 @@ pub fn parse_expr( match scalar_function { ScalarFunction::Unknown => Err(proto_error("Unknown scalar function")), - ScalarFunction::Exp => Ok(exp(parse_expr(&args[0], registry, codec)?)), - ScalarFunction::Factorial => { - Ok(factorial(parse_expr(&args[0], registry, codec)?)) - } - ScalarFunction::Ceil => Ok(ceil(parse_expr(&args[0], registry, codec)?)), ScalarFunction::InitCap => { Ok(initcap(parse_expr(&args[0], registry, codec)?)) } diff --git a/datafusion/proto/src/logical_plan/to_proto.rs b/datafusion/proto/src/logical_plan/to_proto.rs index 358eea785713..2680bc15e1b4 100644 --- a/datafusion/proto/src/logical_plan/to_proto.rs +++ b/datafusion/proto/src/logical_plan/to_proto.rs @@ -1407,9 +1407,6 @@ impl TryFrom<&BuiltinScalarFunction> for protobuf::ScalarFunction { fn try_from(scalar: &BuiltinScalarFunction) -> Result { let scalar_function = match scalar { - BuiltinScalarFunction::Exp => Self::Exp, - BuiltinScalarFunction::Factorial => Self::Factorial, - BuiltinScalarFunction::Ceil => Self::Ceil, BuiltinScalarFunction::Concat => Self::Concat, BuiltinScalarFunction::ConcatWithSeparator => Self::ConcatWithSeparator, BuiltinScalarFunction::EndsWith => Self::EndsWith, diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index 4bf0906685ca..501c51c4be8a 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -282,17 +282,6 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { Ok(Expr::ScalarFunction(ScalarFunction::new_udf(fun, args))) } - pub(super) fn sql_named_function_to_expr( - &self, - expr: SQLExpr, - fun: BuiltinScalarFunction, - schema: &DFSchema, - planner_context: &mut PlannerContext, - ) -> Result { - let args = vec![self.sql_expr_to_logical_expr(expr, schema, planner_context)?]; - Ok(Expr::ScalarFunction(ScalarFunction::new(fun, args))) - } - pub(super) fn find_window_func( &self, name: &str, diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 7763fa2d8dab..f07377ce50e1 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -28,9 +28,8 @@ use datafusion_expr::expr::AggregateFunctionDefinition; use datafusion_expr::expr::InList; use datafusion_expr::expr::ScalarFunction; use datafusion_expr::{ - col, expr, lit, AggregateFunction, Between, BinaryExpr, BuiltinScalarFunction, Cast, - Expr, ExprSchemable, GetFieldAccess, GetIndexedField, Like, Literal, Operator, - TryCast, + col, expr, lit, AggregateFunction, Between, BinaryExpr, Cast, Expr, ExprSchemable, + GetFieldAccess, GetIndexedField, Like, Literal, Operator, TryCast, }; use crate::planner::{ContextProvider, PlannerContext, SqlToRel}; @@ -522,12 +521,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { SQLExpr::Ceil { expr, field: _field, - } => self.sql_named_function_to_expr( - *expr, - BuiltinScalarFunction::Ceil, - schema, - planner_context, - ), + } => self.sql_fn_name_to_expr(*expr, "ceil", schema, planner_context), SQLExpr::Overlay { expr, overlay_what, From a3b98f2051c93e2d48a9642af480e1e768415b27 Mon Sep 17 00:00:00 2001 From: Eren Avsarogullari Date: Sun, 14 Apr 2024 22:41:45 -0700 Subject: [PATCH 2/3] Issue-9939 - Fix build failures --- datafusion/expr/src/expr_fn.rs | 16 ------- datafusion/functions/src/math/factorial.rs | 1 - .../physical-expr/src/equivalence/ordering.rs | 11 +++-- datafusion/physical-expr/src/functions.rs | 45 +------------------ .../physical-expr/src/math_expressions.rs | 2 +- 5 files changed, 7 insertions(+), 68 deletions(-) diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs index 448b13165935..f7900f6b197d 100644 --- a/datafusion/expr/src/expr_fn.rs +++ b/datafusion/expr/src/expr_fn.rs @@ -867,22 +867,6 @@ mod test { ); } - macro_rules! test_unary_scalar_expr { - ($ENUM:ident, $FUNC:ident) => {{ - if let Expr::ScalarFunction(ScalarFunction { - func_def: ScalarFunctionDefinition::BuiltIn(fun), - args, - }) = $FUNC(col("tableA.a")) - { - let name = built_in_function::BuiltinScalarFunction::$ENUM; - assert_eq!(name, fun); - assert_eq!(1, args.len()); - } else { - assert!(false, "unexpected"); - } - }}; - } - macro_rules! test_scalar_expr { ($ENUM:ident, $FUNC:ident, $($arg:ident),*) => { let expected = [$(stringify!($arg)),*]; diff --git a/datafusion/functions/src/math/factorial.rs b/datafusion/functions/src/math/factorial.rs index 1d3b3fc26ea9..04f1c891047e 100644 --- a/datafusion/functions/src/math/factorial.rs +++ b/datafusion/functions/src/math/factorial.rs @@ -95,7 +95,6 @@ fn factorial(args: &[ArrayRef]) -> Result { #[cfg(test)] mod test { - use arrow::array::Float64Array; use datafusion_common::cast::as_int64_array; diff --git a/datafusion/physical-expr/src/equivalence/ordering.rs b/datafusion/physical-expr/src/equivalence/ordering.rs index 688cdf798bdd..ed4600f2d95e 100644 --- a/datafusion/physical-expr/src/equivalence/ordering.rs +++ b/datafusion/physical-expr/src/equivalence/ordering.rs @@ -228,8 +228,7 @@ mod tests { use itertools::Itertools; use datafusion_common::{DFSchema, Result}; - use datafusion_expr::execution_props::ExecutionProps; - use datafusion_expr::{BuiltinScalarFunction, Operator, ScalarUDF}; + use datafusion_expr::{Operator, ScalarUDF}; use crate::equivalence::tests::{ convert_to_orderings, convert_to_sort_exprs, create_random_schema, @@ -241,7 +240,6 @@ mod tests { }; use crate::expressions::Column; use crate::expressions::{col, BinaryExpr}; - use crate::functions::create_physical_expr; use crate::utils::tests::TestScalarUDF; use crate::{PhysicalExpr, PhysicalSortExpr}; @@ -301,11 +299,12 @@ mod tests { &[], &DFSchema::empty(), )?; - let exp_a = &create_physical_expr( - &BuiltinScalarFunction::Exp, + let exp_a = &crate::udf::create_physical_expr( + &test_fun, &[col("a", &test_schema)?], &test_schema, - &ExecutionProps::default(), + &[], + &DFSchema::empty(), )?; let a_plus_b = Arc::new(BinaryExpr::new( col_a.clone(), diff --git a/datafusion/physical-expr/src/functions.rs b/datafusion/physical-expr/src/functions.rs index 6756b8622316..6efbc4179ff4 100644 --- a/datafusion/physical-expr/src/functions.rs +++ b/datafusion/physical-expr/src/functions.rs @@ -272,10 +272,7 @@ fn func_order_in_one_dimension( #[cfg(test)] mod tests { use arrow::{ - array::{ - Array, ArrayRef, BooleanArray, Float32Array, Float64Array, Int32Array, - StringArray, UInt64Array, - }, + array::{Array, ArrayRef, BooleanArray, Int32Array, StringArray, UInt64Array}, datatypes::Field, record_batch::RecordBatch, }; @@ -403,46 +400,6 @@ mod tests { Utf8, StringArray ); - test_function!( - Exp, - &[lit(ScalarValue::Int32(Some(1)))], - Ok(Some((1.0_f64).exp())), - f64, - Float64, - Float64Array - ); - test_function!( - Exp, - &[lit(ScalarValue::UInt32(Some(1)))], - Ok(Some((1.0_f64).exp())), - f64, - Float64, - Float64Array - ); - test_function!( - Exp, - &[lit(ScalarValue::UInt64(Some(1)))], - Ok(Some((1.0_f64).exp())), - f64, - Float64, - Float64Array - ); - test_function!( - Exp, - &[lit(ScalarValue::Float64(Some(1.0)))], - Ok(Some((1.0_f64).exp())), - f64, - Float64, - Float64Array - ); - test_function!( - Exp, - &[lit(ScalarValue::Float32(Some(1.0)))], - Ok(Some((1.0_f32).exp())), - f32, - Float32, - Float32Array - ); test_function!( InitCap, &[lit("hi THOMAS")], diff --git a/datafusion/physical-expr/src/math_expressions.rs b/datafusion/physical-expr/src/math_expressions.rs index 72aac9208b2c..cee1b8c787e2 100644 --- a/datafusion/physical-expr/src/math_expressions.rs +++ b/datafusion/physical-expr/src/math_expressions.rs @@ -81,7 +81,7 @@ pub fn isnan(args: &[ArrayRef]) -> Result { mod tests { use arrow::array::Float64Array; - use datafusion_common::cast::{as_boolean_array, as_int64_array}; + use datafusion_common::cast::as_boolean_array; use super::*; From f1a44e95f975e762bda9e38e12227ecbaa762d90 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Mon, 15 Apr 2024 06:20:35 -0400 Subject: [PATCH 3/3] Update Volatility --- datafusion/functions/src/math/factorial.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datafusion/functions/src/math/factorial.rs b/datafusion/functions/src/math/factorial.rs index 04f1c891047e..dc481da79069 100644 --- a/datafusion/functions/src/math/factorial.rs +++ b/datafusion/functions/src/math/factorial.rs @@ -40,7 +40,7 @@ impl Default for FactorialFunc { impl FactorialFunc { pub fn new() -> Self { Self { - signature: Signature::uniform(1, vec![Int64], Volatility::Volatile), + signature: Signature::uniform(1, vec![Int64], Volatility::Immutable), } } }