Derive ScalarUDFImpl equality, hash from Eq, Hash traits (#17164)

* Remove redundant JsonGetStr::aliases field

* Derive `ScalarUDFImpl` equality, hash from `Eq`, `Hash` traits

Follows similar change for `WindowUDFImpl` and `AggregateUDFImpl`, i.e.
the 8494a39 and
b8bf7c5 commits.

Previously, the `ScalarUDFImpl` trait contained `equals` and
`hash_value` methods with contracts following the `Eq` and `Hash`
traits.  However, the existence of default implementations of these
methods made it error-prone, with many functions (scalar, aggregate,
window) missing to customize the equals even though they ought to.
There is no fix to this that's not an API breaking change, so a breaking
change is warranted.

Removing the default implementations would be enough of a solution, but
at the cost of a lot of boilerplate needed in implementations.

Instead, this removes the methods from the trait, and reuses `DynEq`,
`DynHash` traits used previously only for physical expressions. This
allows for functions to provide their implementations using no more than
`#[derive(PartialEq, Eq, Hash)]` in a typical case.

Author: findepi

datafusion-examples/examples/advanced_udf.rs

@@ -39,7 +39,7 @@ use datafusion::prelude::*;
 /// the power of the second argument `a^b`.
 ///
 /// To do so, we must implement the `ScalarUDFImpl` trait.
-#[derive(Debug, Clone)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 struct PowUdf {
     signature: Signature,
     aliases: Vec<String>,

datafusion-examples/examples/async_udf.rs

@@ -133,7 +133,7 @@ fn animal() -> Result<RecordBatch> {
 ///
 /// Since this is a simplified example, it does not call an LLM service, but
 /// could be extended to do so in a real-world scenario.
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 struct AskLLM {
     signature: Signature,
 }

datafusion-examples/examples/function_factory.rs

@@ -28,7 +28,7 @@ use datafusion::logical_expr::{
     ColumnarValue, CreateFunction, Expr, ScalarFunctionArgs, ScalarUDF, ScalarUDFImpl,
     Signature, Volatility,
 };
-use std::hash::{DefaultHasher, Hash, Hasher};
+use std::hash::Hash;
 use std::result::Result as RResult;
 use std::sync::Arc;
 
@@ -107,7 +107,7 @@ impl FunctionFactory for CustomFunctionFactory {
 }
 
 /// this function represents the newly created execution engine.
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 struct ScalarFunctionWrapper {
     /// The text of the function body, `$1 + f1($2)` in our example
     name: String,
@@ -154,38 +154,6 @@ impl ScalarUDFImpl for ScalarFunctionWrapper {
     fn output_ordering(&self, _input: &[ExprProperties]) -> Result<SortProperties> {
         Ok(SortProperties::Unordered)
     }
-
-    fn equals(&self, other: &dyn ScalarUDFImpl) -> bool {
-        let Some(other) = other.as_any().downcast_ref::<Self>() else {
-            return false;
-        };
-        let Self {
-            name,
-            expr,
-            signature,
-            return_type,
-        } = self;
-        name == &other.name
-            && expr == &other.expr
-            && signature == &other.signature
-            && return_type == &other.return_type
-    }
-
-    fn hash_value(&self) -> u64 {
-        let Self {
-            name,
-            expr,
-            signature,
-            return_type,
-        } = self;
-        let mut hasher = DefaultHasher::new();
-        std::any::type_name::<Self>().hash(&mut hasher);
-        name.hash(&mut hasher);
-        expr.hash(&mut hasher);
-        signature.hash(&mut hasher);
-        return_type.hash(&mut hasher);
-        hasher.finish()
-    }
 }
 
 impl ScalarFunctionWrapper {

datafusion-examples/examples/json_shredding.rs

@@ -282,17 +282,15 @@ impl TableProvider for ExampleTableProvider {
 }
 
 /// Scalar UDF that uses serde_json to access json fields
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 pub struct JsonGetStr {
     signature: Signature,
-    aliases: [String; 1],
 }
 
 impl Default for JsonGetStr {
     fn default() -> Self {
         Self {
             signature: Signature::variadic_any(Volatility::Immutable),
-            aliases: ["json_get_str".to_string()],
         }
     }
 }
@@ -303,7 +301,7 @@ impl ScalarUDFImpl for JsonGetStr {
     }
 
     fn name(&self) -> &str {
-        self.aliases[0].as_str()
+        "json_get_str"
     }
 
     fn signature(&self) -> &Signature {
@@ -355,10 +353,6 @@ impl ScalarUDFImpl for JsonGetStr {
             .collect::<StringArray>();
         Ok(ColumnarValue::Array(Arc::new(values)))
     }
-
-    fn aliases(&self) -> &[String] {
-        &self.aliases
-    }
 }
 
 /// Factory for creating ShreddedJsonRewriter instances

datafusion-examples/examples/optimizer_rule.rs

@@ -175,7 +175,7 @@ fn is_lit_or_col(expr: &Expr) -> bool {
 }
 
 /// A simple user defined filter function
-#[derive(Debug, Clone)]
+#[derive(Debug, PartialEq, Eq, Hash, Clone)]
 struct MyEq {
     signature: Signature,
 }

datafusion/core/tests/fuzz_cases/equivalence/utils.rs

@@ -512,7 +512,7 @@ fn get_sort_columns(
         .collect::<Result<Vec<_>>>()
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 pub struct TestScalarUDF {
     pub(crate) signature: Signature,
 }

datafusion/core/tests/physical_optimizer/projection_pushdown.rs

@@ -63,7 +63,7 @@ use insta::assert_snapshot;
 use itertools::Itertools;
 
 /// Mocked UDF
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 struct DummyUDF {
     signature: Signature,
 }

datafusion/core/tests/user_defined/user_defined_scalar_functions.rs

@@ -43,9 +43,9 @@ use datafusion_common::{
 use datafusion_expr::expr::FieldMetadata;
 use datafusion_expr::simplify::{ExprSimplifyResult, SimplifyInfo};
 use datafusion_expr::{
-    lit_with_metadata, udf_equals_hash, Accumulator, ColumnarValue, CreateFunction,
-    CreateFunctionBody, LogicalPlanBuilder, OperateFunctionArg, ReturnFieldArgs,
-    ScalarFunctionArgs, ScalarUDF, ScalarUDFImpl, Signature, Volatility,
+    lit_with_metadata, Accumulator, ColumnarValue, CreateFunction, CreateFunctionBody,
+    LogicalPlanBuilder, OperateFunctionArg, ReturnFieldArgs, ScalarFunctionArgs,
+    ScalarUDF, ScalarUDFImpl, Signature, Volatility,
 };
 use datafusion_functions_nested::range::range_udf;
 use parking_lot::Mutex;
@@ -218,8 +218,6 @@ impl ScalarUDFImpl for Simple0ArgsScalarUDF {
     fn invoke_with_args(&self, _args: ScalarFunctionArgs) -> Result<ColumnarValue> {
         Ok(ColumnarValue::Scalar(ScalarValue::Int32(Some(100))))
     }
-
-    udf_equals_hash!(ScalarUDFImpl);
 }
 
 #[tokio::test]
@@ -560,8 +558,6 @@ impl ScalarUDFImpl for AddIndexToStringVolatileScalarUDF {
         };
         Ok(ColumnarValue::Array(Arc::new(StringArray::from(answer))))
     }
-
-    udf_equals_hash!(ScalarUDFImpl);
 }
 
 #[tokio::test]
@@ -665,7 +661,7 @@ async fn volatile_scalar_udf_with_params() -> Result<()> {
     Ok(())
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 struct CastToI64UDF {
     signature: Signature,
 }
@@ -787,7 +783,7 @@ async fn deregister_udf() -> Result<()> {
     Ok(())
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 struct TakeUDF {
     signature: Signature,
 }
@@ -979,8 +975,6 @@ impl ScalarUDFImpl for ScalarFunctionWrapper {
 
         Ok(ExprSimplifyResult::Simplified(replacement))
     }
-
-    udf_equals_hash!(ScalarUDFImpl);
 }
 
 impl ScalarFunctionWrapper {
@@ -1282,8 +1276,6 @@ impl ScalarUDFImpl for MyRegexUdf {
             _ => exec_err!("regex_udf only accepts a Utf8 arguments"),
         }
     }
-
-    udf_equals_hash!(ScalarUDFImpl);
 }
 
 #[tokio::test]
@@ -1471,8 +1463,6 @@ impl ScalarUDFImpl for MetadataBasedUdf {
             }
         }
     }
-
-    udf_equals_hash!(ScalarUDFImpl);
 }
 
 #[tokio::test]
@@ -1611,7 +1601,7 @@ async fn test_metadata_based_udf_with_literal() -> Result<()> {
 /// sides. For the input, we will handle the data differently if there is
 /// the canonical extension type Bool8. For the output we will add a
 /// user defined extension type.
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Hash)]
 struct ExtensionBasedUdf {
     name: String,
     signature: Signature,
@@ -1790,7 +1780,7 @@ async fn test_extension_based_udf() -> Result<()> {
 
 #[tokio::test]
 async fn test_config_options_work_for_scalar_func() -> Result<()> {
-    #[derive(Debug)]
+    #[derive(Debug, PartialEq, Eq, Hash)]
     struct TestScalarUDF {
         signature: Signature,
     }

datafusion/expr/src/async_udf.rs

@@ -16,9 +16,7 @@
 // under the License.
 
 use crate::ptr_eq::{arc_ptr_eq, arc_ptr_hash};
-use crate::{
-    udf_equals_hash, ReturnFieldArgs, ScalarFunctionArgs, ScalarUDF, ScalarUDFImpl,
-};
+use crate::{ReturnFieldArgs, ScalarFunctionArgs, ScalarUDF, ScalarUDFImpl};
 use arrow::datatypes::{DataType, FieldRef};
 use async_trait::async_trait;
 use datafusion_common::error::Result;
@@ -127,8 +125,6 @@ impl ScalarUDFImpl for AsyncScalarUDF {
     fn invoke_with_args(&self, _args: ScalarFunctionArgs) -> Result<ColumnarValue> {
         internal_err!("async functions should not be called directly")
     }
-
-    udf_equals_hash!(ScalarUDFImpl);
 }
 
 impl Display for AsyncScalarUDF {

datafusion/expr/src/expr.rs

@@ -3648,7 +3648,7 @@ mod test {
     #[test]
     fn test_is_volatile_scalar_func() {
         // UDF
-        #[derive(Debug)]
+        #[derive(Debug, PartialEq, Eq, Hash)]
         struct TestScalarUDF {
             signature: Signature,
         }