alamb commented on code in PR #8258:
URL: https://github.com/apache/arrow-datafusion/pull/8258#discussion_r1399293377
##########
datafusion/expr/src/expr.rs:
##########
@@ -1198,11 +1213,8 @@ impl fmt::Display for Expr {
write!(f, " NULLS LAST")
}
}
- Expr::ScalarFunction(func) => {
- fmt_function(f, &func.fun.to_string(), false, &func.args, true)
- }
- Expr::ScalarUDF(ScalarUDF { fun, args }) => {
- fmt_function(f, fun.name(), false, args, true)
+ Expr::ScalarFunction(ScalarFunction { func_def, args }) => {
+ fmt_function(f, &func_def.name(), false, args, true)
Review Comment:
for example, this callsite doesn't need an owned `String`, `&str` would work
well
##########
datafusion/expr/src/expr.rs:
##########
@@ -338,37 +336,55 @@ impl Between {
}
}
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+/// Defines which implementation of a function for DataFusion to call.
+pub enum ScalarFunctionDefinition {
+ /// Resolved to a `BuiltinScalarFunction`
+ /// There is plan to migrate `BuiltinScalarFunction` to UDF-based
implementation (issue#8045)
+ /// This variant is planned to be removed in long term
+ BuiltIn(built_in_function::BuiltinScalarFunction),
+ /// Resolved to a user defined function
+ UDF(Arc<crate::ScalarUDF>),
+ /// A scalar function constructed with name. This variant can not be
executed directly
+ /// and instead must be resolved to one of the other variants prior to
physical planning.
+ Name(Arc<str>),
+}
+
/// ScalarFunction expression invokes a built-in scalar function
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub struct ScalarFunction {
/// The function
- pub fun: built_in_function::BuiltinScalarFunction,
+ pub func_def: ScalarFunctionDefinition,
/// List of expressions to feed to the functions as arguments
pub args: Vec<Expr>,
}
+impl ScalarFunctionDefinition {
+ /// Function's name for display
+ pub fn name(&self) -> String {
Review Comment:
I think it would be better to allow the callsites to decide if they needed
to make the string copy -- so passing back `&str` I think would make for a
better API:
```suggestion
pub fn name(&self) -> &str {
```
--
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]
