peter-toth commented on code in PR #10868:
URL: https://github.com/apache/datafusion/pull/10868#discussion_r1634727867
##########
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:
Generating unique aliases with some kind of index is a great idea, but I'm
not sure this implementation is robust enough. The rule
`CommonSubexprEliminate` can run many times during optimization so starting
alias index always from 0 might result colliding column names.
I wonder if we could check the schema for possible duplicates before
assigning an alias? This idea has actually come up before:
https://github.com/apache/datafusion/pull/10473#discussion_r1604645685, but
would be better to implement in a separate PR like this one.
Also, do you think we can assign aliases with "cse_" or "common_" prefix?
IMO we don't need a special `#` prefix for that.
--
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
