alamb commented on code in PR #9954:
URL: https://github.com/apache/arrow-datafusion/pull/9954#discussion_r1559446853
##########
datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs:
##########
@@ -109,18 +127,22 @@ impl SimplifyExpressions {
simplifier
};
- let exprs = plan
- .expressions()
Review Comment:
expressions() cloned all the expressions
##########
datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs:
##########
@@ -109,18 +127,22 @@ impl SimplifyExpressions {
simplifier
};
- let exprs = plan
- .expressions()
- .into_iter()
- .map(|e| {
+ // the output schema of a filter or join is the input schema. Thus they
+ // can't handle aliased expressions
+ let use_alias = !matches!(plan, LogicalPlan::Filter(_) |
LogicalPlan::Join(_));
+ plan.map_expressions(|e| {
+ let new_e = if use_alias {
// TODO: unify with `rewrite_preserving_name`
let original_name = e.name_for_alias()?;
- let new_e = simplifier.simplify(e)?;
- new_e.alias_if_changed(original_name)
- })
- .collect::<Result<Vec<_>>>()?;
+ simplifier.simplify(e)?.alias_if_changed(original_name)
+ } else {
+ simplifier.simplify(e)
+ }?;
- plan.with_new_exprs(exprs, new_inputs)
Review Comment:
The whole point of this PR is to remove the calls to `expressions` and
`new_with_exprs` (which requires cloning all the expressions)
##########
datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs:
##########
@@ -109,18 +127,22 @@ impl SimplifyExpressions {
simplifier
};
- let exprs = plan
- .expressions()
Review Comment:
Calling `expressions()` clones all the expressions
--
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]
