jorgecarleitao commented on a change in pull request #8836:
URL: https://github.com/apache/arrow/pull/8836#discussion_r536874229



##########
File path: rust/datafusion/src/physical_plan/udf.rs
##########
@@ -56,6 +56,12 @@ impl Debug for ScalarUDF {
     }
 }
 
+impl PartialEq for ScalarUDF {
+    fn eq(&self, other: &Self) -> bool {
+        self.name == other.name && self.signature == other.signature

Review comment:
       I am afraid that this may not be sufficient: two UDFs may have the same 
name and signature and represent different semantics. In general UDFs are 
anonymous functions and thus do not have a `partialEq`. If we base equality on 
`name` and two UDFs are assigned the same name, there will be a semantic error 
in this equality.
   
   Note that while this can't happen in SQL, because UDFs are registered via 
`register_udf(name, ...)`, the DataFrame API supports using bypassing the 
context and calling UDFs without registering them on the context.
   
   In the `simple_udf.rs` example, the line 
   
   ```rust
   let expr1 = pow.call(vec![col("a"), col("b")]);
   ```
   
   demonstrates how to use a UDF without registering it on the context, in 
which case its name is only used when printing the plan.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to