alamb commented on code in PR #9522:
URL: https://github.com/apache/arrow-datafusion/pull/9522#discussion_r1518813170
##########
datafusion/physical-expr/src/udf.rs:
##########
@@ -28,8 +29,22 @@ use std::sync::Arc;
pub fn create_physical_expr(
fun: &ScalarUDF,
input_phy_exprs: &[Arc<dyn PhysicalExpr>],
- return_type: DataType,
+ input_schema: &Schema,
+ args: &[Expr],
+ input_dfschema: &DFSchema,
) -> Result<Arc<dyn PhysicalExpr>> {
+ let input_expr_types = input_phy_exprs
Review Comment:
I think the idea was at this point the type checking had already been done,
so there is no reason to check the types again.
However, I looked at how non udf functions work, and they also seem to check
the data types again, so the change in this PR is consistent:
https://github.com/apache/arrow-datafusion/blob/6710e6db38d3f5292e6d9506c70c153c2e76f339/datafusion/physical-expr/src/functions.rs#L62-L84
##########
datafusion/expr/src/udf.rs:
##########
@@ -305,14 +305,11 @@ pub trait ScalarUDFImpl: Debug + Send + Sync {
/// value for `('foo' | 'bar')` as it does for ('foobar').
fn return_type_from_exprs(
&self,
- args: &[Expr],
- schema: &dyn ExprSchema,
+ _args: &[Expr],
+ _schema: &dyn ExprSchema,
+ arg_types: &[DataType],
Review Comment:
I reviewed the documentation of this API, and I think in general it is clear
and no changes are needed
##########
datafusion/core/tests/user_defined/user_defined_scalar_functions.rs:
##########
@@ -655,6 +655,7 @@ impl ScalarUDFImpl for TakeUDF {
&self,
arg_exprs: &[Expr],
schema: &dyn ExprSchema,
+ _arg_data_types: &[DataType],
Review Comment:
(note this is a test)
--
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]
