alexandrefimov opened a new issue, #25208: URL: https://github.com/apache/datafusion/issues/25208
### Describe the bug The output of an `AggregateRel` with more than one grouping set ends with "an extra `i32` column" whose value "will be the zero-based index of the grouping set that yielded the record" ([logical_relations.md at v0.102.0](https://github.com/substrait-io/substrait/blob/v0.102.0/site/docs/relations/logical_relations.md#aggregate-operation)). DataFusion's consumer maps that column to its own `__grouping_id`, so executing the plan puts the grouping-id bitmask in it rather than the set's index. For the two sets below that bitmask is a `UInt8`. #23468 moved the column to the position Substrait gives it and left its contents as they were. The producer makes the same identification when it writes a plan: it maps DataFusion's `[groups, grouping_id, measures]` onto Substrait's `[groups, measures, grouping_id]` ([`producer/rel/aggregate_rel.rs:57-58`](https://github.com/apache/datafusion/blob/f8cc678272c50c781a56b74b49912b999aebd89e/datafusion/substrait/src/logical_plan/producer/rel/aggregate_rel.rs#L57-L58)). A plan passed between DataFusion and an implementation that follows the spec, such as substrait-java, which types the column `i32`, is therefore read with two meanings for it. ### To Reproduce On `main` at `f8cc67827`, this plan has two grouping sets, `(a)` and `(b)`, over a two-row virtual table: ```json {"relations": [{"root": { "names": ["a", "b", "grouping_set"], "input": {"aggregate": { "input": {"read": { "baseSchema": {"names": ["a", "b"], "struct": {"types": [ {"i64": {"nullability": "NULLABILITY_REQUIRED"}}, {"i64": {"nullability": "NULLABILITY_REQUIRED"}}], "nullability": "NULLABILITY_REQUIRED"}}, "virtualTable": {"expressions": [ {"fields": [{"literal": {"i64": "1"}}, {"literal": {"i64": "10"}}]}, {"fields": [{"literal": {"i64": "2"}}, {"literal": {"i64": "20"}}]}]}}}, "groupingExpressions": [ {"selection": {"directReference": {"structField": {"field": 0}}, "rootReference": {}}}, {"selection": {"directReference": {"structField": {"field": 1}}, "rootReference": {}}}], "groupings": [{"expressionReferences": [0]}, {"expressionReferences": [1]}]}}}}]} ``` Consumed with `from_substrait_plan` and executed, it returns: ```text grouping_set: UInt8 (nullable: false) +---+----+--------------+ | a | b | grouping_set | +---+----+--------------+ | 2 | | 1 | | | 20 | 2 | | 1 | | 1 | | | 10 | 2 | +---+----+--------------+ ``` ### Expected behavior `grouping_set` as a required `Int32`, holding 0 on the two rows from `(a)` and 1 on the two rows from `(b)`. The bitmask and the index coincide for some lists of sets: for `(a, b)` followed by `(a)` both are 0 and then 1. A test needs a list on which they differ, such as `(a)` followed by `(b)`. ### Additional context First seen in a [relation conformance case](https://github.com/alexandrefimov/substrait-conformance-cases/blob/a046105/tests/relations/cases/aggregate/grouping_set_index.yaml). -- 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]
