JacobOgle commented on code in PR #8100:
URL: https://github.com/apache/arrow-datafusion/pull/8100#discussion_r1388697103
##########
datafusion/proto/src/logical_plan/from_proto.rs:
##########
@@ -1645,6 +1646,13 @@ pub fn parse_expr(
)),
ScalarFunction::Isnan => Ok(isnan(parse_expr(&args[0],
registry)?)),
ScalarFunction::Iszero => Ok(iszero(parse_expr(&args[0],
registry)?)),
+ ScalarFunction::ArrowTypeof => {
+ Ok(arrow_typeof(parse_expr(&args[0], registry)?))
+ }
+ ScalarFunction::ToTimestamp => {
+ Ok(to_timestamp_seconds(parse_expr(&args[0], registry)?))
+ }
+ ScalarFunction::Flatten => Ok(flatten(parse_expr(&args[0],
registry)?)),
_ => Err(proto_error(
"Protobuf deserialization error: Unsupported scalar
function",
)),
Review Comment:
@alamb I was able to get the remaining cases covered with the exception to
`datafusion::ScalarFunction::StructFun`.
Browsing the code I am not seeing a supported function for this enum variant
in `expr_fn.rs`. The scalar_expr macro maps to enum variants in
`built_in_functions.rs` but has no match for StructFun. Should this be mapped
to the Struct variant in the macro?
Apologies for the confusion, but appreciate any guidance.
```
macro_rules! scalar_expr {
($ENUM:ident, $FUNC:ident, $($arg:ident)*, $DOC:expr) => {
#[doc = $DOC ]
pub fn $FUNC($($arg: Expr),*) -> Expr {
Expr::ScalarFunction(ScalarFunction::new(
built_in_function::BuiltinScalarFunction::$ENUM,
vec![$($arg),*],
))
}
};
}
```
but only has
```
...
// struct functions
/// struct
Struct,
...
```
in the `BuiltinScalarFunction::$ENUM`
--
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]
