peter-toth commented on PR #10868: URL: https://github.com/apache/datafusion/pull/10868#issuecomment-2171754117
> Hey @peter-toth sorry for the late reply. > > Looks good, and I like the Alias generator. But I have a couple of comments: > > 1. Thought we agreed on the `#` prefix. Is there a reason for choosing the `__cse_` prefix? DuckDB uses `#XXX`, SQLServer uses `ExprXXX` (can't verify this atm), Spark uses `_expr#XXX`, all of which look cleaner than `__cse_XXX` > 2. Tiny nit, but can you use `into_values` instead of using `into_iter` then ignoring the key? > > I don't mind either options, a PR against this one keeps the entire review history and can make reviews easier, but a new PR can be easier to maintain as you'll be able to directly push commits. No problem, I've opened https://github.com/apache/datafusion/pull/10939. Yes and actually I'm fine with that as well, but I found a few `AliasGenerator` usecases in the existing code and wanted to follow the conventions of https://github.com/apache/datafusion/blob/main/datafusion/optimizer/src/decorrelate_predicate_subquery.rs#L249 and https://github.com/apache/datafusion/blob/main/datafusion/optimizer/src/scalar_subquery_to_join.rs#L245 with the `__cse` prefix. I've also changed `into_iter()` to `into_values ()` in https://github.com/apache/datafusion/pull/10939/commits/6b1d1e34abd4c9da4a550db99471c09f5add09ff. -- 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
