alamb commented on code in PR #8985:
URL: https://github.com/apache/arrow-datafusion/pull/8985#discussion_r1486884992


##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -37,19 +37,19 @@ use std::sync::Arc;
 /// trait to allow expr to typable with respect to a schema
 pub trait ExprSchemable {
     /// given a schema, return the type of the expr
-    fn get_type<S: ExprSchema>(&self, schema: &S) -> Result<DataType>;
+    fn get_type(&self, schema: &dyn ExprSchema) -> Result<DataType>;

Review Comment:
   I had to change the traits to use `dyn` dispatch rather than generics so 
that the UDF could use the same object (and e.g. not have to create its own 
copy of these methods for Expr)
   
   I expect this to have 0 performance impact, but I will run the planning 
benchmarks to be sure if this acceptable



##########
datafusion/core/tests/user_defined/user_defined_scalar_functions.rs:
##########
@@ -494,6 +495,127 @@ async fn test_user_defined_functions_zero_argument() -> 
Result<()> {
     Ok(())
 }
 
+#[derive(Debug)]
+struct TakeUDF {

Review Comment:
   Here is an example of the feature working



##########
datafusion/expr/src/udf.rs:
##########
@@ -249,6 +261,22 @@ pub trait ScalarUDFImpl: Debug + Send + Sync {
     /// the arguments
     fn return_type(&self, arg_types: &[DataType]) -> Result<DataType>;
 
+    /// What [`DataType`] will be returned by this function, given the types of
+    /// the expr arguments
+    fn return_type_from_exprs(

Review Comment:
   Update is I changed the signature to take `&dyn ExprSchema` which seems to 
have worked just fine



-- 
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.

To unsubscribe, e-mail: [email protected]

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

Reply via email to