jonahgao opened a new pull request, #10408: URL: https://github.com/apache/datafusion/pull/10408
## Which issue does this PR close? Closes #10346. ## Rationale for this change [exprlist_to_fields_aggregate](https://github.com/apache/datafusion/blob/a0fccbf886346fde5dfbda136149ec98bbd6e952/datafusion/expr/src/utils.rs#L737) was introduced by https://github.com/apache/datafusion/pull/2486. It was necessary for the test `aggregate_with_rollup_with_grouping`. https://github.com/apache/datafusion/blob/5335f8055636a0e11f8bcda8a3084edba825f79e/datafusion/core/src/sql/planner.rs#L4698-L4705 In this test, the grouped-by expression `persion.state` was not included in the aggregate's output schema, so we need to further check the aggregate's input, as mentioned in the comments. > // when dealing with aggregate plans we cannot simply look in the aggregate output schema // because it will contain columns representing complex expressions (such a column named // `#GROUPING(person.state)` so in order to resolve `person.state` in this case we need to // look at the input to the aggregate instead. But on the current main branch, `persion.state` is already included in the aggregate's output schema, which is achieved through the function [grouping_set_to_exprlist](https://github.com/apache/datafusion/blob/c8b8c74904972c57f559a167dd0ccd52c8f91076/datafusion/expr/src/utils.rs#L251). It adds all the grouped-by expressions. https://github.com/apache/datafusion/blob/c8b8c74904972c57f559a167dd0ccd52c8f91076/datafusion/expr/src/logical_plan/plan.rs#L2273 Therefore, `exprlist_to_fields_aggregate` should become unnecessary. Besides, it takes columns from the aggregate's input and might override a column with the same name in the output, leading to the bug in #10346.. ## What changes are included in this PR? Remove the unnecessary `exprlist_to_fields_aggregate` and fix a bug. ## Are these changes tested? Yes ## Are there any user-facing changes? No -- 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
