alamb commented on PR #10396: URL: https://github.com/apache/datafusion/pull/10396#issuecomment-2101053281
> @alamb, IMO if this PR can be merged then the next steps should be: > 1. Fix [Incorrect results with expression resolution #10413](https://github.com/apache/datafusion/issues/10413) as that is correctness bug. Agree -- this is now tracked as its own issue and we can deal with it separately > 2. Continue [Rewrite `CommonSubexprEliminate` to avoid copies using TreeNode #10067](https://github.com/apache/datafusion/pull/10067) efforts to refactor the `CommonSubexprEliminate` rule to `rewrite` as that could improve a lot on [Stop copying `Expr`s and LogicalPlans so much during Common Subexpression Elimination #9873](https://github.com/apache/datafusion/issues/9873). I will do this > 3. Rebase @MohamedAbdeen21's [make common expression alias human-readable #10333](https://github.com/apache/datafusion/pull/10333) on the top of this PR as probably we don't need to use the current string identifiers in aliases and we could improve readablity. Sounds like @MohamedAbdeen21 is going to do this maybe this weekend > 4. Revisit the identifiers as using these string identifiers as the keys of `ExprStats` was not the best choice. (Please note that this was not my choice but this is how CSE has been working since the feature was added initially.) See my comment on this in the PR description. 👍 > I'm happy to take 4 as I already worked on it a bit, but unfortunately I have very little time to work on this project lately so I can't take 1. and 2. That would be amazing -- thank you @peter-toth -- I filed https://github.com/apache/datafusion/issues/10426 to track -- 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]
