[jonasHanhan]

Which issue does this PR close?

• Closes Substrait producer: GROUP BY k, k gives a plan whose projection reads the aggregate's columns shifted by one #27058.

Rationale for this change

GROUP BY k, k is legal SQL and does not change the result, but producing Substrait from that plan and consuming it again swapped the grouping and measure columns.

DataFusion's Aggregate schema keeps each grouping key once (grouping_set_to_exprlist). The producer already shared identical expressions in grouping_expressions, but it still wrote a repeated expression_reference for each occurrence ([0, 0]). The consumer expands each reference into its own output column, so the AggregateRel effectively gained an extra column. Projections built from the DataFusion plan still indexed fields as if that column were not there, and a round trip returned (sum, k) under the names (k, s).

What changes are included in this PR?

In parse_flat_grouping_exprs, skip grouping expressions that are already present in the current grouping. The AggregateRel then declares and references the repeated key once, matching the DataFusion Aggregate schema that projections are built against.

What is the testing strategy for this PR?

Added aggregate_duplicate_group_by_key_roundtrip in roundtrip_logical_plan.rs. It uses an unoptimized plan so eliminate_duplicated_expr does not hide the bug, checks that the producer emits a single grouping expression reference, and asserts that collecting the plan before and after a Substrait round trip returns the same batches. It also covers the optimized path where the duplicate key is already removed.

Locally: cargo test -p datafusion-substrait passes, and cargo clippy -p datafusion-substrait --all-targets -- -D warnings and cargo fmt are clean.

Are there any user-facing changes?

Substrait plans produced from logical plans that still contain repeated GROUP BY keys (for example unoptimized plans) now round-trip with the correct column order. No API changes.

[bvolpato]

LGTM. The per-group deduplication matches the ordering used by Aggregate's schema, and the regression checks the unoptimized plan where the duplicate key is still present. I ran the Substrait crate tests at this head, including the round-trip regression.

Could you also enable Actions on your fork and push again? Rust (fork) currently reports no Rust workflow run for this commit, so CI still needs to run before merge. Thanks for fixing this.

[jonasHanhan]

Thanks for the review! I'll enable Actions on the fork and push again so Rust (fork) can run.