andygrove commented on code in PR #4744:
URL: https://github.com/apache/datafusion-comet/pull/4744#discussion_r3529707498
##########
native/core/src/execution/planner.rs:
##########
@@ -532,6 +532,19 @@ impl PhysicalPlanner {
_ => func,
}
}
+ ExprStruct::HighOrderFunc(hof) => {
+ self.create_high_order_function_expr(hof, input_schema)
+ }
+ ExprStruct::NamedLambdaVariable(nlv) => {
+ let idx = input_schema.index_of(&nlv.name).map_err(|_| {
Review Comment:
Lambda variables here resolve by name via `input_schema.index_of`, which
returns the first field of that name. That's only safe when the name is unique
across the combined schema. Two cases concern me: a lambda variable whose name
collides with an input column (e.g. `SELECT x, filter(arr, x -> x > 2) FROM
t`), and nested lambdas that reuse a name (`filter(arr, x -> exists(arr2, x ->
...))`). In both, `index_of` returns the wrong field and we'd silently read the
wrong column, whereas Spark disambiguates by `ExprId`. Could we add tests for
both? If they diverge, resolving by position or uniquifying the names in the
serde would close the gap.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
