peter-toth commented on code in PR #10832:
URL: https://github.com/apache/datafusion/pull/10832#discussion_r1632234343
##########
datafusion/optimizer/src/common_subexpr_eliminate.rs:
##########
@@ -782,9 +786,11 @@ impl TreeNodeRewriter for CommonSubexprRewriter<'_> {
return Ok(Transformed::new(expr, false, TreeNodeRecursion::Jump));
}
- let (up_index, expr_id) = &self.id_array[self.down_index];
+ let (up_index, expr_id, aliased) = &self.id_array[self.down_index];
self.down_index += 1;
+ self.aliased |= aliased;
Review Comment:
You can build up any kinds of plans in a test with `LogicalPlanBuilder`
(including non-root aliases). Whether it ever actually happens or not in real
query is a different question... But think of the current
`CommonSubexprEliminate` rule, it did produce such non-root aliases so there
can be other rules doing the same in certain scenarios despite the original
query couldn't cointain such alias.
BTW I'm also ok with checking only the root node if we want to keep this
rule simple and avoid using stacks, but in that case the check shouldn't be in
`f_down()` as it's called on all nodes.
--
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]
