nielspardon opened a new pull request, #12724: URL: https://github.com/apache/gluten/pull/12724
## What changes are proposed in this pull request? Substrait 0.98 moved the grouping expressions out of the per-grouping `Grouping.grouping_expressions` (field 1) into a rel-level pool `AggregateRel.grouping_expressions = 5` that each grouping set references by index through `Grouping.expression_references = 2`. This migrates Gluten's vendored proto, the JVM producer, and both native consumers in lockstep, as one step of the Substrait `v0.23.0` → `0.98.0` proto rebase (#12597). Gluten only ever emits a single grouping set with a flat list of grouping expressions — GROUPING SETS / CUBE / ROLLUP are expanded into an `ExpandRel` upstream, and both native consumers enforce at-most-one grouping. So the mapping is order-preserving: the flat list becomes the rel-level pool in declaration order, and the single grouping references every entry as `[0, 1, ..., n-1]`. Empty grouping (global aggregation) stays one empty grouping with zero references — no behavior change. `AggregateRel` stays at Rel-oneof field 3, so there is no oneof renumbering. - **Proto:** reserve `Grouping` field 1, add `Grouping.expression_references = 2` and rel-level `AggregateRel.grouping_expressions = 5` (vendored verbatim from 0.98). - **Producer:** `AggregateRelNode` populates the pool in declaration order and has the single grouping reference every entry by index. - **Velox:** `SubstraitToVeloxPlan` and `SubstraitToVeloxPlanValidator` resolve references against the pool; `VeloxToSubstraitPlan` (the Velox→Substrait reverse converter) emits pool + references. - **ClickHouse:** `AggregateRelParser` and `ExpandRelParser` read grouping keys through the pool via `expression_references`. ## How was this patch tested? - New `AggregateRelProtoSuite` pins the producer contract (flat grouping list → rel-level pool + single grouping referencing `[0, 1]`; empty grouping → one empty grouping with zero references). - Migrated the `q1_first_stage.json` Velox fixture to the pooled shape (consumer coverage). - Locally verified: `protoc` dup-field check; `gluten-substrait` builds and the new suite passes (`mvn -Pspark-3.5`); the Velox native backend compiles (consumer, validator, and reverse converter); `clang-format` (v15) clean on all changed C++ files. - Existing end-to-end group-by suites exercise the consumer resolution on both backends. ## Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) 🤖 Generated with AI -- 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]
