This is an automated email from the ASF dual-hosted git repository.

jayzhan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new 85f7a8e88e Move abs to datafusion_functions (#9313)
85f7a8e88e is described below

commit 85f7a8e88e3596b3ec900c43fabb8f7f42bbea5c
Author: Junhao Liu <junhaoliu2...@gmail.com>
AuthorDate: Mon Feb 26 18:24:16 2024 -0600

    Move abs to datafusion_functions (#9313)
    
    * feat: move abs to datafusion_functions
    
    * fix proto
    
    * fix proto
    
    * fix CI vendored code
    
    * Fix proto
    
    * add support type
    
    * fix signature
    
    * fix typo
    
    * fix test cases
    
    * disable a test case
    
    * remove old code from math_expressions
    
    * feat: add test
    
    * fix clippy
    
    * use unknown for proto
    
    * fix unknown proto
---
 datafusion/expr/src/built_in_function.rs         |   7 -
 datafusion/expr/src/expr.rs                      |   5 -
 datafusion/expr/src/expr_fn.rs                   |   2 -
 datafusion/functions/src/math/abs.rs             | 177 +++++++++++++++++++++++
 datafusion/functions/src/math/mod.rs             |   8 +-
 datafusion/physical-expr/src/functions.rs        |   4 -
 datafusion/physical-expr/src/math_expressions.rs |  93 +-----------
 datafusion/proto/proto/datafusion.proto          |   4 +-
 datafusion/proto/src/generated/pbjson.rs         |   6 +-
 datafusion/proto/src/generated/prost.rs          |   8 +-
 datafusion/proto/src/logical_plan/from_proto.rs  |   6 +-
 datafusion/proto/src/logical_plan/to_proto.rs    |   1 -
 12 files changed, 198 insertions(+), 123 deletions(-)

diff --git a/datafusion/expr/src/built_in_function.rs 
b/datafusion/expr/src/built_in_function.rs
index 8b4e65121c..cf1e73f780 100644
--- a/datafusion/expr/src/built_in_function.rs
+++ b/datafusion/expr/src/built_in_function.rs
@@ -42,8 +42,6 @@ use strum_macros::EnumIter;
 #[derive(Debug, Clone, PartialEq, Eq, Hash, EnumIter, Copy)]
 pub enum BuiltinScalarFunction {
     // math functions
-    /// abs
-    Abs,
     /// acos
     Acos,
     /// asin
@@ -364,7 +362,6 @@ impl BuiltinScalarFunction {
     pub fn volatility(&self) -> Volatility {
         match self {
             // Immutable scalar builtins
-            BuiltinScalarFunction::Abs => Volatility::Immutable,
             BuiltinScalarFunction::Acos => Volatility::Immutable,
             BuiltinScalarFunction::Asin => Volatility::Immutable,
             BuiltinScalarFunction::Atan => Volatility::Immutable,
@@ -868,8 +865,6 @@ impl BuiltinScalarFunction {
 
             BuiltinScalarFunction::ArrowTypeof => Ok(Utf8),
 
-            BuiltinScalarFunction::Abs => Ok(input_expr_types[0].clone()),
-
             BuiltinScalarFunction::OverLay => {
                 utf8_to_str_type(&input_expr_types[0], "overlay")
             }
@@ -1338,7 +1333,6 @@ impl BuiltinScalarFunction {
                 Signature::uniform(2, vec![Int64], self.volatility())
             }
             BuiltinScalarFunction::ArrowTypeof => Signature::any(1, 
self.volatility()),
-            BuiltinScalarFunction::Abs => Signature::any(1, self.volatility()),
             BuiltinScalarFunction::OverLay => Signature::one_of(
                 vec![
                     Exact(vec![Utf8, Utf8, Int64, Int64]),
@@ -1444,7 +1438,6 @@ impl BuiltinScalarFunction {
     /// Returns all names that can be used to call this function
     pub fn aliases(&self) -> &'static [&'static str] {
         match self {
-            BuiltinScalarFunction::Abs => &["abs"],
             BuiltinScalarFunction::Acos => &["acos"],
             BuiltinScalarFunction::Acosh => &["acosh"],
             BuiltinScalarFunction::Asin => &["asin"],
diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs
index f40ccb6cdb..c3d9269d15 100644
--- a/datafusion/expr/src/expr.rs
+++ b/datafusion/expr/src/expr.rs
@@ -2033,11 +2033,6 @@ mod test {
                 .is_volatile()
                 .unwrap()
         );
-        assert!(
-            !ScalarFunctionDefinition::BuiltIn(BuiltinScalarFunction::Abs)
-                .is_volatile()
-                .unwrap()
-        );
 
         // UDF
         #[derive(Debug)]
diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs
index 4aa270e6dd..55bd40a189 100644
--- a/datafusion/expr/src/expr_fn.rs
+++ b/datafusion/expr/src/expr_fn.rs
@@ -557,7 +557,6 @@ nary_scalar_expr!(
     trunc,
     "truncate toward zero, with optional precision"
 );
-scalar_expr!(Abs, abs, num, "absolute value");
 scalar_expr!(Signum, signum, num, "sign of the argument (-1, 0, +1) ");
 scalar_expr!(Exp, exp, num, "exponential");
 scalar_expr!(Gcd, gcd, arg_1 arg_2, "greatest common divisor");
@@ -1354,7 +1353,6 @@ mod test {
         test_nary_scalar_expr!(Round, round, input, decimal_places);
         test_nary_scalar_expr!(Trunc, trunc, num);
         test_nary_scalar_expr!(Trunc, trunc, num, precision);
-        test_unary_scalar_expr!(Abs, abs);
         test_unary_scalar_expr!(Signum, signum);
         test_unary_scalar_expr!(Exp, exp);
         test_unary_scalar_expr!(Log2, log2);
diff --git a/datafusion/functions/src/math/abs.rs 
b/datafusion/functions/src/math/abs.rs
new file mode 100644
index 0000000000..21ca37fb8e
--- /dev/null
+++ b/datafusion/functions/src/math/abs.rs
@@ -0,0 +1,177 @@
+// 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.
+
+//! math expressions
+
+use arrow::array::Decimal128Array;
+use arrow::array::Decimal256Array;
+use arrow::array::Int16Array;
+use arrow::array::Int32Array;
+use arrow::array::Int64Array;
+use arrow::array::Int8Array;
+use arrow::datatypes::DataType;
+use datafusion_common::not_impl_err;
+use datafusion_common::plan_datafusion_err;
+use datafusion_common::{internal_err, Result, DataFusionError};
+use datafusion_expr::utils;
+use datafusion_expr::ColumnarValue;
+
+use datafusion_expr::{ScalarUDFImpl, Signature, Volatility};
+use std::any::Any;
+use std::sync::Arc;
+use arrow::array::{ArrayRef, Float32Array, Float64Array};
+use arrow::error::ArrowError;
+
+type MathArrayFunction = fn(&Vec<ArrayRef>) -> Result<ArrayRef>;
+
+macro_rules! make_abs_function {
+    ($ARRAY_TYPE:ident) => {{
+        |args: &Vec<ArrayRef>| {
+            let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+            let res: $ARRAY_TYPE = array.unary(|x| x.abs());
+            Ok(Arc::new(res) as ArrayRef)
+        }
+    }};
+}
+
+macro_rules! make_try_abs_function {
+    ($ARRAY_TYPE:ident) => {{
+        |args: &Vec<ArrayRef>| {
+            let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+            let res: $ARRAY_TYPE = array.try_unary(|x| {
+                x.checked_abs().ok_or_else(|| {
+                    ArrowError::ComputeError(format!(
+                        "{} overflow on abs({})",
+                        stringify!($ARRAY_TYPE),
+                        x
+                    ))
+                })
+            })?;
+            Ok(Arc::new(res) as ArrayRef)
+        }
+    }};
+}
+
+macro_rules! make_decimal_abs_function {
+    ($ARRAY_TYPE:ident) => {{
+        |args: &Vec<ArrayRef>| {
+            let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
+            let res: $ARRAY_TYPE = array
+                .unary(|x| x.wrapping_abs())
+                .with_data_type(args[0].data_type().clone());
+            Ok(Arc::new(res) as ArrayRef)
+        }
+    }};
+}
+
+/// Abs SQL function
+/// Return different implementations based on input datatype to reduce 
branches during execution
+fn create_abs_function(
+    input_data_type: &DataType,
+) -> Result<MathArrayFunction> {
+    match input_data_type {
+        DataType::Float32 => Ok(make_abs_function!(Float32Array)),
+        DataType::Float64 => Ok(make_abs_function!(Float64Array)),
+
+        // Types that may overflow, such as abs(-128_i8).
+        DataType::Int8 => Ok(make_try_abs_function!(Int8Array)),
+        DataType::Int16 => Ok(make_try_abs_function!(Int16Array)),
+        DataType::Int32 => Ok(make_try_abs_function!(Int32Array)),
+        DataType::Int64 => Ok(make_try_abs_function!(Int64Array)),
+
+        // Types of results are the same as the input.
+        DataType::Null
+        | DataType::UInt8
+        | DataType::UInt16
+        | DataType::UInt32
+        | DataType::UInt64 => Ok(|args: &Vec<ArrayRef>| Ok(args[0].clone())),
+
+        // Decimal types
+        DataType::Decimal128(_, _) => 
Ok(make_decimal_abs_function!(Decimal128Array)),
+        DataType::Decimal256(_, _) => 
Ok(make_decimal_abs_function!(Decimal256Array)),
+
+        other => not_impl_err!("Unsupported data type {other:?} for function 
abs"),
+    }
+}
+#[derive(Debug)]
+pub(super) struct AbsFunc {
+    signature: Signature,
+}
+
+impl AbsFunc {
+    pub fn new() -> Self {
+        Self {
+            signature: Signature::any(1, Volatility::Immutable)
+        }
+    }
+}
+
+impl ScalarUDFImpl for AbsFunc {
+    fn as_any(&self) -> &dyn Any {
+        self
+    }
+    fn name(&self) -> &str {
+        "abs"
+    }
+
+    fn signature(&self) -> &Signature {
+        &self.signature
+    }
+
+    fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
+        if arg_types.len() != 1 {
+            return Err(plan_datafusion_err!(
+                "{}",
+                utils::generate_signature_error_msg(
+                    self.name(),
+                    self.signature().clone(),
+                    arg_types,
+                )
+            ));
+        }
+        match arg_types[0] {
+            DataType::Float32 => Ok(DataType::Float32),
+            DataType::Float64 => Ok(DataType::Float64),
+            DataType::Int8 => Ok(DataType::Int8),
+            DataType::Int16 => Ok(DataType::Int16),
+            DataType::Int32 => Ok(DataType::Int32),
+            DataType::Int64 => Ok(DataType::Int64),
+            DataType::Null => Ok(DataType::Null),
+            DataType::UInt8 => Ok(DataType::UInt8),
+            DataType::UInt16 => Ok(DataType::UInt16),
+            DataType::UInt32 => Ok(DataType::UInt32),
+            DataType::UInt64 => Ok(DataType::UInt64),
+            DataType::Decimal128(precision, scale) => 
Ok(DataType::Decimal128(precision, scale)),
+            DataType::Decimal256(precision, scale) => 
Ok(DataType::Decimal256(precision, scale)),
+            _ => not_impl_err!("Unsupported data type {} for function abs", 
arg_types[0].to_string()),
+        }
+    }
+
+    fn invoke(&self, args: &[ColumnarValue]) -> Result<ColumnarValue> {
+        let args = ColumnarValue::values_to_arrays(args)?;
+
+        if args.len() != 1 {
+            return internal_err!("abs function requires 1 argument, got {}", 
args.len());
+        }
+    
+        let input_data_type = args[0].data_type();
+        let abs_fun = create_abs_function(input_data_type)?;
+    
+        let arr = abs_fun(&args)?;
+        Ok(ColumnarValue::Array(arr))
+    }
+}
diff --git a/datafusion/functions/src/math/mod.rs 
b/datafusion/functions/src/math/mod.rs
index 873625948a..9d13103ef2 100644
--- a/datafusion/functions/src/math/mod.rs
+++ b/datafusion/functions/src/math/mod.rs
@@ -18,12 +18,14 @@
 //! "math" DataFusion functions
 
 mod nans;
+mod abs;
 
 // create  UDFs
 make_udf_function!(nans::IsNanFunc, ISNAN, isnan);
+make_udf_function!(abs::AbsFunc, ABS, abs);
 
 // Export the functions out of this package, both as expr_fn as well as a list 
of functions
 export_functions!(
-    (isnan, num, "returns true if a given number is +NaN or -NaN otherwise 
returns false")
-);
-
+    (isnan, num, "returns true if a given number is +NaN or -NaN otherwise 
returns false"),
+    (abs, num, "returns the absolute value of a given number")
+);
\ No newline at end of file
diff --git a/datafusion/physical-expr/src/functions.rs 
b/datafusion/physical-expr/src/functions.rs
index 8446a65d72..0dc3f96dc1 100644
--- a/datafusion/physical-expr/src/functions.rs
+++ b/datafusion/physical-expr/src/functions.rs
@@ -260,9 +260,6 @@ pub fn create_physical_fun(
 ) -> Result<ScalarFunctionImplementation> {
     Ok(match fun {
         // math functions
-        BuiltinScalarFunction::Abs => Arc::new(|args| {
-            make_scalar_function_inner(math_expressions::abs_invoke)(args)
-        }),
         BuiltinScalarFunction::Acos => Arc::new(math_expressions::acos),
         BuiltinScalarFunction::Asin => Arc::new(math_expressions::asin),
         BuiltinScalarFunction::Atan => Arc::new(math_expressions::atan),
@@ -3075,7 +3072,6 @@ mod tests {
         let funs = [
             BuiltinScalarFunction::Concat,
             BuiltinScalarFunction::ToTimestamp,
-            BuiltinScalarFunction::Abs,
             BuiltinScalarFunction::Repeat,
         ];
 
diff --git a/datafusion/physical-expr/src/math_expressions.rs 
b/datafusion/physical-expr/src/math_expressions.rs
index af66862aec..b622aee8e2 100644
--- a/datafusion/physical-expr/src/math_expressions.rs
+++ b/datafusion/physical-expr/src/math_expressions.rs
@@ -18,15 +18,11 @@
 //! Math expressions
 
 use arrow::array::ArrayRef;
-use arrow::array::{
-    BooleanArray, Decimal128Array, Decimal256Array, Float32Array, Float64Array,
-    Int16Array, Int32Array, Int64Array, Int8Array,
-};
+use arrow::array::{BooleanArray, Float32Array, Float64Array, Int64Array};
 use arrow::datatypes::DataType;
-use arrow::error::ArrowError;
+use datafusion_common::internal_err;
 use datafusion_common::ScalarValue;
 use datafusion_common::ScalarValue::{Float32, Int64};
-use datafusion_common::{internal_err, not_impl_err};
 use datafusion_common::{DataFusionError, Result};
 use datafusion_expr::ColumnarValue;
 use rand::{thread_rng, Rng};
@@ -35,8 +31,6 @@ use std::iter;
 use std::mem::swap;
 use std::sync::Arc;
 
-type MathArrayFunction = fn(&[ArrayRef]) -> Result<ArrayRef>;
-
 macro_rules! downcast_compute_op {
     ($ARRAY:expr, $NAME:expr, $FUNC:ident, $TYPE:ident) => {{
         let n = $ARRAY.as_any().downcast_ref::<$TYPE>();
@@ -176,7 +170,6 @@ math_unary_function!("acosh", acosh);
 math_unary_function!("atanh", atanh);
 math_unary_function!("floor", floor);
 math_unary_function!("ceil", ceil);
-math_unary_function!("abs", abs);
 math_unary_function!("signum", signum);
 math_unary_function!("exp", exp);
 math_unary_function!("ln", ln);
@@ -673,88 +666,6 @@ fn compute_truncate64(x: f64, y: i64) -> f64 {
     (x * factor).round() / factor
 }
 
-macro_rules! make_abs_function {
-    ($ARRAY_TYPE:ident) => {{
-        |args: &[ArrayRef]| {
-            let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
-            let res: $ARRAY_TYPE = array.unary(|x| x.abs());
-            Ok(Arc::new(res) as ArrayRef)
-        }
-    }};
-}
-
-macro_rules! make_try_abs_function {
-    ($ARRAY_TYPE:ident) => {{
-        |args: &[ArrayRef]| {
-            let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
-            let res: $ARRAY_TYPE = array.try_unary(|x| {
-                x.checked_abs().ok_or_else(|| {
-                    ArrowError::ComputeError(format!(
-                        "{} overflow on abs({})",
-                        stringify!($ARRAY_TYPE),
-                        x
-                    ))
-                })
-            })?;
-            Ok(Arc::new(res) as ArrayRef)
-        }
-    }};
-}
-
-macro_rules! make_decimal_abs_function {
-    ($ARRAY_TYPE:ident) => {{
-        |args: &[ArrayRef]| {
-            let array = downcast_arg!(&args[0], "abs arg", $ARRAY_TYPE);
-            let res: $ARRAY_TYPE = array
-                .unary(|x| x.wrapping_abs())
-                .with_data_type(args[0].data_type().clone());
-            Ok(Arc::new(res) as ArrayRef)
-        }
-    }};
-}
-
-/// Abs SQL function
-/// Return different implementations based on input datatype to reduce 
branches during execution
-pub(super) fn create_abs_function(
-    input_data_type: &DataType,
-) -> Result<MathArrayFunction> {
-    match input_data_type {
-        DataType::Float32 => Ok(make_abs_function!(Float32Array)),
-        DataType::Float64 => Ok(make_abs_function!(Float64Array)),
-
-        // Types that may overflow, such as abs(-128_i8).
-        DataType::Int8 => Ok(make_try_abs_function!(Int8Array)),
-        DataType::Int16 => Ok(make_try_abs_function!(Int16Array)),
-        DataType::Int32 => Ok(make_try_abs_function!(Int32Array)),
-        DataType::Int64 => Ok(make_try_abs_function!(Int64Array)),
-
-        // Types of results are the same as the input.
-        DataType::Null
-        | DataType::UInt8
-        | DataType::UInt16
-        | DataType::UInt32
-        | DataType::UInt64 => Ok(|args: &[ArrayRef]| Ok(args[0].clone())),
-
-        // Decimal types
-        DataType::Decimal128(_, _) => 
Ok(make_decimal_abs_function!(Decimal128Array)),
-        DataType::Decimal256(_, _) => 
Ok(make_decimal_abs_function!(Decimal256Array)),
-
-        other => not_impl_err!("Unsupported data type {other:?} for function 
abs"),
-    }
-}
-
-/// abs() SQL function implementation
-pub fn abs_invoke(args: &[ArrayRef]) -> Result<ArrayRef> {
-    if args.len() != 1 {
-        return internal_err!("abs function requires 1 argument, got {}", 
args.len());
-    }
-
-    let input_data_type = args[0].data_type();
-    let abs_fun = create_abs_function(input_data_type)?;
-
-    abs_fun(args)
-}
-
 #[cfg(test)]
 mod tests {
 
diff --git a/datafusion/proto/proto/datafusion.proto 
b/datafusion/proto/proto/datafusion.proto
index 7673ce86ae..d91373f8f8 100644
--- a/datafusion/proto/proto/datafusion.proto
+++ b/datafusion/proto/proto/datafusion.proto
@@ -545,7 +545,9 @@ message InListNode {
 }
 
 enum ScalarFunction {
-  Abs = 0;
+  //  0 was Abs before
+  //  The first enum value must be zero for open enums
+  unknown = 0;
   Acos = 1;
   Asin = 2;
   Atan = 3;
diff --git a/datafusion/proto/src/generated/pbjson.rs 
b/datafusion/proto/src/generated/pbjson.rs
index 65483f9ac4..964b889018 100644
--- a/datafusion/proto/src/generated/pbjson.rs
+++ b/datafusion/proto/src/generated/pbjson.rs
@@ -22321,7 +22321,7 @@ impl serde::Serialize for ScalarFunction {
         S: serde::Serializer,
     {
         let variant = match self {
-            Self::Abs => "Abs",
+            Self::Unknown => "unknown",
             Self::Acos => "Acos",
             Self::Asin => "Asin",
             Self::Atan => "Atan",
@@ -22464,7 +22464,7 @@ impl<'de> serde::Deserialize<'de> for ScalarFunction {
         D: serde::Deserializer<'de>,
     {
         const FIELDS: &[&str] = &[
-            "Abs",
+            "unknown",
             "Acos",
             "Asin",
             "Atan",
@@ -22636,7 +22636,7 @@ impl<'de> serde::Deserialize<'de> for ScalarFunction {
                 E: serde::de::Error,
             {
                 match value {
-                    "Abs" => Ok(ScalarFunction::Abs),
+                    "unknown" => Ok(ScalarFunction::Unknown),
                     "Acos" => Ok(ScalarFunction::Acos),
                     "Asin" => Ok(ScalarFunction::Asin),
                     "Atan" => Ok(ScalarFunction::Atan),
diff --git a/datafusion/proto/src/generated/prost.rs 
b/datafusion/proto/src/generated/prost.rs
index a567269e33..292aef4402 100644
--- a/datafusion/proto/src/generated/prost.rs
+++ b/datafusion/proto/src/generated/prost.rs
@@ -2633,7 +2633,9 @@ impl JoinConstraint {
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, 
::prost::Enumeration)]
 #[repr(i32)]
 pub enum ScalarFunction {
-    Abs = 0,
+    ///   0 was Abs before
+    ///   The first enum value must be zero for open enums
+    Unknown = 0,
     Acos = 1,
     Asin = 2,
     Atan = 3,
@@ -2776,7 +2778,7 @@ impl ScalarFunction {
     /// (if the ProtoBuf definition does not change) and safe for programmatic 
use.
     pub fn as_str_name(&self) -> &'static str {
         match self {
-            ScalarFunction::Abs => "Abs",
+            ScalarFunction::Unknown => "unknown",
             ScalarFunction::Acos => "Acos",
             ScalarFunction::Asin => "Asin",
             ScalarFunction::Atan => "Atan",
@@ -2913,7 +2915,7 @@ impl ScalarFunction {
     /// Creates an enum from field names used in the ProtoBuf definition.
     pub fn from_str_name(value: &str) -> ::core::option::Option<Self> {
         match value {
-            "Abs" => Some(Self::Abs),
+            "unknown" => Some(Self::Unknown),
             "Acos" => Some(Self::Acos),
             "Asin" => Some(Self::Asin),
             "Atan" => Some(Self::Atan),
diff --git a/datafusion/proto/src/logical_plan/from_proto.rs 
b/datafusion/proto/src/logical_plan/from_proto.rs
index 2554018a92..69114fd745 100644
--- a/datafusion/proto/src/logical_plan/from_proto.rs
+++ b/datafusion/proto/src/logical_plan/from_proto.rs
@@ -47,7 +47,7 @@ use datafusion_common::{
 use datafusion_expr::expr::Unnest;
 use datafusion_expr::window_frame::{check_window_frame, 
regularize_window_order_by};
 use datafusion_expr::{
-    abs, acos, acosh, array, array_append, array_concat, array_dims, 
array_distinct,
+    acos, acosh, array, array_append, array_concat, array_dims, array_distinct,
     array_element, array_empty, array_except, array_has, array_has_all, 
array_has_any,
     array_intersect, array_length, array_ndims, array_pop_back, 
array_pop_front,
     array_position, array_positions, array_prepend, array_remove, 
array_remove_all,
@@ -442,6 +442,7 @@ impl From<&protobuf::ScalarFunction> for 
BuiltinScalarFunction {
     fn from(f: &protobuf::ScalarFunction) -> Self {
         use protobuf::ScalarFunction;
         match f {
+            ScalarFunction::Unknown => todo!(),
             ScalarFunction::Sqrt => Self::Sqrt,
             ScalarFunction::Cbrt => Self::Cbrt,
             ScalarFunction::Sin => Self::Sin,
@@ -470,7 +471,6 @@ impl From<&protobuf::ScalarFunction> for 
BuiltinScalarFunction {
             ScalarFunction::Ceil => Self::Ceil,
             ScalarFunction::Round => Self::Round,
             ScalarFunction::Trunc => Self::Trunc,
-            ScalarFunction::Abs => Self::Abs,
             ScalarFunction::OctetLength => Self::OctetLength,
             ScalarFunction::Concat => Self::Concat,
             ScalarFunction::Lower => Self::Lower,
@@ -1360,6 +1360,7 @@ pub fn parse_expr(
             let args = &expr.args;
 
             match scalar_function {
+                ScalarFunction::Unknown => Err(proto_error("Unknown scalar 
function")),
                 ScalarFunction::Asin => Ok(asin(parse_expr(&args[0], 
registry)?)),
                 ScalarFunction::Acos => Ok(acos(parse_expr(&args[0], 
registry)?)),
                 ScalarFunction::Asinh => Ok(asinh(parse_expr(&args[0], 
registry)?)),
@@ -1537,7 +1538,6 @@ pub fn parse_expr(
                         .map(|expr| parse_expr(expr, registry))
                         .collect::<Result<Vec<_>, _>>()?,
                 )),
-                ScalarFunction::Abs => Ok(abs(parse_expr(&args[0], 
registry)?)),
                 ScalarFunction::Signum => Ok(signum(parse_expr(&args[0], 
registry)?)),
                 ScalarFunction::OctetLength => {
                     Ok(octet_length(parse_expr(&args[0], registry)?))
diff --git a/datafusion/proto/src/logical_plan/to_proto.rs 
b/datafusion/proto/src/logical_plan/to_proto.rs
index ccadbb217a..9603df209c 100644
--- a/datafusion/proto/src/logical_plan/to_proto.rs
+++ b/datafusion/proto/src/logical_plan/to_proto.rs
@@ -1450,7 +1450,6 @@ impl TryFrom<&BuiltinScalarFunction> for 
protobuf::ScalarFunction {
             BuiltinScalarFunction::Ceil => Self::Ceil,
             BuiltinScalarFunction::Round => Self::Round,
             BuiltinScalarFunction::Trunc => Self::Trunc,
-            BuiltinScalarFunction::Abs => Self::Abs,
             BuiltinScalarFunction::OctetLength => Self::OctetLength,
             BuiltinScalarFunction::Concat => Self::Concat,
             BuiltinScalarFunction::Lower => Self::Lower,

Reply via email to