yyy1000 commented on PR #9522:
URL:
https://github.com/apache/arrow-datafusion/pull/9522#issuecomment-1987093523
> By default, they should get data types with these lines, so we still need
them even all the functions are udf-based.
> If the udf functions need user-defined return type, they can implement
their own `return_type`.
Yeah, I understand that. What I mean is the default implementation for
`return_type_from_exprs` also get data types with these lines. Maybe we can
delete them (if all the functions are udf-based) and the
`return_type_from_exprs` would keep the same as before, so the args_type are
only computed once?
```rust
Expr::ScalarFunction(ScalarFunction { func_def, args }) => {
match func_def {
ScalarFunctionDefinition::UDF(fun) => {
// Maybe we can move the check into
return_type_from_exprs?
// verify that function is invoked with correct
number and type of arguments as defined in `TypeSignature`
// data_types(&arg_data_types,
fun.signature()).map_err(|_| {
// plan_datafusion_err!(
// "{}",
// utils::generate_signature_error_msg(
// fun.name(),
// fun.signature().clone(),
// &arg_data_types,
// )
// )
// })?;
// perform additional function arguments validation
(due to limited
// expressiveness of `TypeSignature`), then infer
return type
Ok(fun.return_type_from_exprs(args, schema)?)
}
ScalarFunctionDefinition::Name(_) => {
internal_err!("Function `Expr` with name should be
resolved.")
}
}
}
```
> Actually, I think we don't even need return_type_from_exprs, we should
combine it with return_type with args, schema, arg_types.
That sounds good. :) I'd like to see what it will be like
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
