Fix incorrect Schema over aggregate function, Remove unnecessary exprlist_to_fields_aggregate

[jonahgao]

Which issue does this PR close?

Closes #10346.

Rationale for this change

exprlist_to_fields_aggregate was introduced by #2486.
It was necessary for the test aggregate_with_rollup_with_grouping.

datafusion/datafusion/core/src/sql/planner.rs

Lines 4698 to 4705 in 5335f80

	async fn aggregate_with_rollup_with_grouping() {
	let sql = "SELECT id, state, age, grouping(state), grouping(age), grouping(state) + grouping(age), COUNT(*) \
	FROM person GROUP BY id, ROLLUP (state, age)";
	let expected = "Projection: #person.id, #person.state, #person.age, #GROUPING(person.state), #GROUPING(person.age), #GROUPING(person.state) + #GROUPING(person.age), #COUNT(UInt8(1))\
	\n Aggregate: groupBy=[[#person.id, ROLLUP (#person.state, #person.age)]], aggr=[[GROUPING(#person.state), GROUPING(#person.age), COUNT(UInt8(1))]]\
	\n TableScan: person projection=None";
	quick_test(sql, expected);
	}

In this test, the grouped-by expression person.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, person.state is already included in the aggregate's output schema, which is achieved through the function grouping_set_to_exprlist. It adds all the grouped-by expressions.

datafusion/datafusion/expr/src/logical_plan/plan.rs

Line 2273 in c8b8c74

let grouping_expr: Vec<Expr> = grouping_set_to_exprlist(group_expr.as_slice())?;

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

[alamb]

It is a very nice PR that fixes a bug by deleting code 👨‍🍳 👌

Thank you very much @jonahgao 🙏

[alamb]

FYI @timsaucer

[alamb]

Thanks again @jonahgao