2010YOUY01 commented on code in PR #8222:
URL: https://github.com/apache/arrow-datafusion/pull/8222#discussion_r1394827814
##########
datafusion/expr/src/expr_fn.rs:
##########
@@ -993,7 +993,27 @@ pub fn create_udwf(
pub fn call_fn(name: impl AsRef<str>, args: Vec<Expr>) -> Result<Expr> {
match name.as_ref().parse::<BuiltinScalarFunction>() {
Ok(fun) => Ok(Expr::ScalarFunction(ScalarFunction::new(fun, args))),
- Err(e) => Err(e),
+ Err(_) => {
+ // Constructing a `ScalarUDF` with only name and stub
impl/return_type...
+ // This unresolved UDF will be resolved during analyzing using
registered functions
Review Comment:
That's a great point, without enum the implementation seems a bit vulnerable
to bugs
I think we can either make `Expr::ScalarFunction` or `Expr::ScalarUDF` a
enum, since the long-term goal is to remove `BuiltinScalarFunction` they should
be same.
I'll check which way can make future migration easier
```rust
pub enum ScalarFunctionDefinition {
/// Resolved to a user defined function
UDF(ScalarUDF),
/// A scalar function that will be called by name
Name(Arc<str>),
}
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub struct ScalarUDF {
/// The function
pub fun: ScalarFunctionDefinition,
/// List of expressions to feed to the functions as arguments
pub args: Vec<Expr>,
}
```
```rust
pub enum ScalarFunctionDefinition {
/// Resolved to a built in scalar function
/// (will be removed long term)
BuiltIn(built_in_function::BuiltinScalarFunction),
/// Resolved to a user defined function
UDF(ScalarUDF),
/// A scalar function that will be called by name
Name(Arc<str>),
}
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub struct ScalarFunction {
/// The function
pub fun: ScalarFunctionDefinition,
/// List of expressions to feed to the functions as arguments
pub args: Vec<Expr>,
}
```
--
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]
