gstvg commented on code in PR #18921:
URL: https://github.com/apache/datafusion/pull/18921#discussion_r2976104051
##########
datafusion/physical-expr/src/expressions/lambda_variable.rs:
##########
Review Comment:
To make this PR smaller and easier to reason about, I used the name for now.
Could have added a comment, though.
Is possible to resolve by index, we just need to decide how to it supporting
the following `expr_api` usage:
```rust
// v -> v + v
lambda(
["v"],
lambda_var("v") + col("v")
)
```
Resolving by name as today, the code above works without names conflicts. So
that it keeps working by index we can either:
1. During lambda function physical planning, we create a new schema for each
lambda argument that includes it's parameters and use it to plan the lambda
body. For each lambda parameter on that schema, add a metadata entry to them so
that when theirs variables are being planned, we search for the index of field
with both the variable name *and* the metadata entry, but it feels a bit loose?
2. Use different schema for columns and parameters, and move it around
either via a new argument on `create_physical_expr` (breaking) or embed it on
`ExecutionProps`, which is still breaking but less, but also doesn't feel like
a good fit for `ExecutionProps`
3. Revert to the old approach where `LambdaVariable` holds a
`Option<ArrayRef>` that should be binded for each evaluation, and won't depend
on the planning schema nor the evaluation batch
4. Resolve by the index but don't compute them during the regular physical
planning, via `create_physical_expr`, but right after it with a tree node
rewriter that computes the variables indexes without depending on the planning
schema.
Note that this does not apply to sql planning as there lambda variables have
higher precedence and columns get shadowed by them.
--
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]
