gstvg commented on code in PR #22853:
URL: https://github.com/apache/datafusion/pull/22853#discussion_r3542553680
##########
datafusion/physical-expr/src/expressions/lambda.rs:
##########
@@ -129,6 +141,7 @@ impl LambdaExpr {
body,
projected_body,
projection,
+ used_params: used_param_names,
Review Comment:
Good point about maps, I was thinking only about array_* funcs where the
value is free and the index relatively cheap
Thinking about it now, we could also backport any non-breaking,
self-contained fix in 54.x, and for the 55 release, break `LambdaArgument::new`
to receive the whole lambda as `Arc<LambdaExpr>` instead of only the body as
`Arc<dyn PhysicalExpr>`, and add `pub fn used_params(&self) ->
&HashSet<String>` to `LambdaExpr`. And in the future if any other field gets
added to `LambdaExpr` , `LambdaArgument::evaluate` can use it without requiring
new methods like `LambdaArgument::new_with_used_params_and_foo`
--
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]
