alamb commented on code in PR #9546:
URL: https://github.com/apache/arrow-datafusion/pull/9546#discussion_r1519548725
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -583,8 +586,10 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
self.sql_expr_to_logical_expr(value, input_schema,
planner_context)
})
.collect::<Result<Vec<_>>>()?;
- Ok(Expr::ScalarFunction(ScalarFunction::new(
- BuiltinScalarFunction::Struct,
+ Ok(Expr::ScalarFunction(ScalarFunction::new_udf(
+ Arc::new(ScalarUDF::new_from_impl(
+ datafusion_functions::core::r#struct::StructFunc::new(),
+ )),
Review Comment:
One of the goals of pulling datafusion functions out of the core is so that
the built in functions aren't treated specially. Thus having the sql planner
directly instantiate the function goes against that goal.
Can you please change this to call the `struct()` function by name
(`get_function_meta`) instead of adding a dependency
For example
https://github.com/apache/arrow-datafusion/blob/4cd3c433004a7a6825643d6b3911db720efe5f76/datafusion/sql/src/expr/mod.rs#L173-L180
--
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]
