fix(physical-optimizer): decline eager aggregation for grouping-set aggregates

[claudespice]

Summary

EagerAggregation produces a physically invalid plan for a GROUPING SETS / ROLLUP / CUBE aggregate over a join. Spice hits it on TPC-DS q36 (which groups by ROLLUP(i_category, i_class) and reads the aggregate outputs from a window function), because Spice enables this rule by default:

tpcds_q36: Query execution failed: code: 'Internal error', message: "Internal error: Assertion failed:
col.name() == matching_name: Input field name sum(store_sales.ss_ext_sales_price) does not match with
the projection expression sum(store_sales.ss_net_profit)."

Root cause

The rewrite rebuilds the top grouping from the flat expression list only:

let new_group_by = PhysicalGroupBy::new_single(new_group_exprs);

PhysicalGroupBy::new_single hard-codes null_expr: vec![], a single all-false groups row, and has_grouping_set: false. A grouping-set aggregate therefore comes back as a plain GROUP BY, dropping both the extra grouping-set rows and the __grouping_id column that group_fields appends whenever has_grouping_set is set. The AVG output projection has the same gap — it iterates group.expr(), which never includes __grouping_id.

Nothing downstream catches the narrowed schema, because the rule opts out of schema_check (COUNT decomposes to SUM, widening non-null to nullable). The parent projection/window keeps binding aggregate outputs at their original indices, so the column at the __grouping_id slot is read as the first aggregate and the assertion in ProjectionMapping fires.

Confirmed by neutering the new guard — the aggregate output schema loses a column:

left:  [d_name, sum(f_amount)]
right: [d_name, __grouping_id, sum(f_amount)]

Fix

Decline the rewrite when the partial aggregate's grouping reports has_grouping_set().

The guard is on the partial grouping deliberately: as_final() always returns has_grouping_set: false (it folds __grouping_id into its expression list instead), so top_final.group_expr() can never report a grouping set and a guard there would miss the bug entirely.

This is the minimum safe fix. Supporting grouping sets properly means preserving the partial grouping's full shape — remapping null_expr alongside expr into the rebuilt join's schema and keeping __grouping_id at its original output index, since GROUPING(...) results are bound positionally by parent projections and window PARTITION BY/ORDER BY. That is recorded in the module's "Deferred / not yet supported" list rather than attempted here.

Testing

cargo test -p datafusion-physical-optimizer --lib — 58 passed, 0 failed (was 56 before this change).

Two new regression tests reuse the existing beneficial-join fixture but group by ROLLUP(d_name), so the grouping set is the only difference from fires_on_beneficial_join and the cost gate reaches the same verdict:

• declines_grouping_set_aggregate — no pre-aggregation is pushed below the join.
• grouping_set_aggregate_preserves_output_schema — the aggregate output schema, __grouping_id included, is unchanged by the rule.

Both were verified to fail with the guard neutered to if false (56 passed; 2 failed) and pass with it.

The plain-grouping fixture now routes through a new agg_over_join_grouped helper that takes the PhysicalGroupBy, so all existing accept-path tests still exercise the unchanged behaviour (no snapshot updates were needed).

cargo clippy -p datafusion-physical-optimizer --lib --tests -- -Dwarnings and cargo fmt are clean for the touched file.

Notes

• No behaviour change for plain GROUP BY, and none at all upstream, where enable_eager_aggregation defaults to false.
• Refs TPC-DS q36 fails when eager aggregation rewrites ROLLUP aggregate spiceai#11827.