andygrove commented on code in PR #4744:
URL: https://github.com/apache/datafusion-comet/pull/4744#discussion_r3529972081
##########
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:
Correction to my comment above: I checked this out locally and the
single-level column-collision example I gave (`filter(arr, x -> x > 2)` with a
column also named `x`) actually works fine. DataFusion's
`LambdaArgument`/`merge_captures_with_variables` machinery handles the flat
case correctly, so please disregard that example.
The real problem is broader though: **any higher-order function nested
inside another lambda's body resolves lambda variables to the wrong index.** In
`create_lambda_expr` the body schema is `input_schema.fields() ++ this lambda's
args`, but for an inner lambda `input_schema` already contains the outer
lambda's appended arg, so the index no longer lines up with what `LambdaExpr`'s
projection remapping expects. It shows up two ways.
Distinct variable names produce a hard native crash:
```sql
SELECT filter(outer_arr, x -> size(filter(inner_arr, y -> y > 15)) > 0) FROM
t
-- CometNativeException: Field of physical LambdaVariable with index 0
doesn't match
-- batch field during evaluation Field { "y": Int32 } != Field { "x": Int32 }
```
Reusing the name silently returns wrong results:
```sql
SELECT filter(outer_arr, x -> size(filter(inner_arr, x -> x > 15)) > 0) FROM
t
-- Spark: [[1, 2, 3]] Comet: [[]]
```
With `t` as `VALUES (array(1,2,3), array(10,20,30))`. Both go native
(`CometProject` in the plan). The existing `filter(filter(arr, x -> ...), y ->
...)` test passes because the inner filter is a value argument, not nested
inside a lambda body, so it never actually nests.
This might be worth an explicit unsupported-shape guard (fall back to
codegen/Spark when a lambda body itself contains a higher-order function) until
nested lambda scoping is handled, so it doesn't crash or silently miscompute.
Happy to share the SQL-file repros.
--
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]
