peter-toth commented on PR #10396: URL: https://github.com/apache/datafusion/pull/10396#issuecomment-2099072200
> I also filed #10413 to track the bug you found (🦅 👁️ ). However, this PR doesn't seem to fix it yet 🤔 . I pushed a test to show this and also tried it manually: No, this PR doesn't fix that issue at all. That issue is a resolution issue (https://github.com/apache/datafusion/issues/10413) and has nothing to do with CSE. The example I gave in the description doesn't contain any subexpressions to eliminate and `CommonSubexprEliminate` has no effect on the query plan. The reason I mentioned the resolution issue is because of that issue I couldn't add a test case to this PR which would illustrate the issue of colflicting identifiers in `CommonSubexprEliminate` after https://github.com/apache/datafusion/pull/9871. Once https://github.com/apache/datafusion/issues/10413 is solved I can add a test case here. > I believe @MohamedAbdeen21 used #{expr} in https://github.com/apache/datafusion/pull/10333 to follow what is done by DuckDB -- perhaps we could do so too in this PR (I also think #{} is slightly easier to notice visually than {}) I fully aggree that the current alias is very hard to read and this is because the identifiers are used for aliases as well. But there are 2 different things here: 1. The identifier must be unique for each unique expression when it is used as the key of the `ExprStats` map that stores the counts. 2. The aliases we gave to extracted common expressions shouldn't collide. Currently for both 1. and 2. we use the identifier and I'm sure that in 1. we have touse the identifier. In 2. I'm not sure and @MohamedAbdeen21's PR can be a good follow-up improvement. -- 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
