cj-zhukov commented on PR #21021:
URL: https://github.com/apache/datafusion/pull/21021#issuecomment-4174798208
I’ve pushed updates addressing the previous review comments:
- Fixed unique alias generation
- Added new tests to improve coverage
- Added an example with aggregates in the `select` doc comment
- Added new aggregation examples in `datafusion-examples` that may be useful
for new users
- Updated existing tests to align with the new logic:
`test_aggregate_subexpr` and `test_aggregate_with_union`
Note on `roundtrip_expr_api`:
I updated the test to follow the new approach, but there is still one
unresolved issue with:
`grouping(lit(1))`
This currently fails with:
`Error: Context("resolve_grouping_function", Plan("Argument Int32(1) to
grouping function is not in grouping columns "))`
For now, I’ve commented it out and left a `TODO` to revisit.
@martin-g I’ve addressed all the points from the previous review. The PR
should now be ready for the next round of review.
--
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]