bvolpato opened a new pull request, #23468:
URL: https://github.com/apache/datafusion/pull/23468

   ## Which issue does this PR close?
   
   - Closes #17910.
   
   ## Rationale for this change
   
   For multiple grouping sets, Substrait's aggregate direct output is grouping 
expressions, measures, then grouping-set ID. DataFusion's aggregate schema 
places its internal `__grouping_id` before measures. The consumer applied 
`RelCommon.emit` against DataFusion's order, so a valid mapping such as `[0, 1, 
2]` selected `__grouping_id` instead of the first measure and silently returned 
wrong results.
   
   See the [Substrait AggregateRel output mapping 
specification](https://substrait.io/relations/logical_relations/#aggregate-operation).
   
   ## What changes are included in this PR?
   
   - Reorder multi-grouping-set consumer output to Substrait's direct order 
before applying emit mappings.
   - Add producer emit mappings that preserve DataFusion's expected aggregate 
schema for parent relations.
   - Deduplicate producer grouping expressions so grouping references and 
output field counts match Substrait's schema.
   - Add an executable consumer regression plan based on the issue and a 
two-measure producer roundtrip assertion.
   
   ## Are these changes tested?
   
   Yes.
   
   - `cargo test -p datafusion-substrait --features protoc`: 49 unit, 201 
integration, and 3 doc tests passed; 6 existing tests ignored.
   - `cargo clippy --all-targets --all-features -- -D warnings`: passed.
   - `PROTOC=<vendored protoc> ./dev/rust_lint.sh`: passed, including fmt, CI 
clippy, TOML, license, typos, Prettier, and rustdoc.
   - Extended workspace test: 453/453 DataFusion library tests passed and 
1027/1028 core integration tests passed. Unrelated `sort_with_mem_limit_1` 
failed its host RSS ceiling (215.9 MB measured, 190.7 MB limit), including an 
isolated rerun.
   
   ## Are there any user-facing changes?
   
   Yes. Substrait plans with multiple grouping sets now resolve emitted measure 
columns in the specified order. No public API change.
   


-- 
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]

Reply via email to