MohamedAbdeen21 commented on code in PR #10868:
URL: https://github.com/apache/datafusion/pull/10868#discussion_r1635179893
##########
datafusion/optimizer/src/common_subexpr_eliminate.rs:
##########
@@ -515,14 +522,15 @@ fn build_common_expr_project_plan(
let mut fields_set = BTreeSet::new();
let mut project_exprs = common_exprs
.into_iter()
- .map(|(expr_id, expr)| {
+ .enumerate()
+ .map(|(index, (expr_id, expr))| {
Review Comment:
We shouldn't worry about that scenario because we make sure that all
expressions are properly aliased before writing them. The closest example I can
find of a similar scenario is
[this](https://github.com/apache/datafusion/pull/10868/files#diff-351499880963d6a383c92e156e75019cd9ce33107724a9635853d7d4cd1898d0R1114)
or maybe even
[this](https://github.com/apache/datafusion/pull/10868/files#diff-351499880963d6a383c92e156e75019cd9ce33107724a9635853d7d4cd1898d0R1136).
You'll see that the CSE is aliased to its original name and the number/alias
can be safely given to other expressions.
So for the example you gave it would be the "constant folding"
responsibility to alias `#0` in the intermediate projection to something else.
I can try passing `common_exprs` between different optimization calls if
that will make you feel safer about the change, but that may be challenging
given DataFusion's language of choice (aka our beloved Rust).
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
