You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alexandrefimov (via GitHub)" <gi...@apache.org> on 2026/09/11 20:41:55 UTC
[I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
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: github-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "namanjain24-sudo (via GitHub)" <gi...@apache.org>.
namanjain24-sudo commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-5747816089
take
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "namanjain24-sudo (via GitHub)" <gi...@apache.org>.
namanjain24-sudo commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6426157633
No — substrait-java isn't part of it, and nothing outside DataFusion executes anything. Feed DataFusion a Substrait plan for `GROUPING SETS ((a), (b))` through `from_substrait_plan` and the trailing column comes back as 1 and 2 where the spec says 0 and 1. That's the whole bug.
Not worth your bandwidth though — I've said the same on the PR.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6383217073
@alexandrefimov -- there is a proposed PR for this here, could you help review?
- https://github.com/apache/datafusion/pull/25527
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "namanjain24-sudo (via GitHub)" <gi...@apache.org>.
namanjain24-sudo commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6385205834
> How is this related to datafusion? Are you trying to run a datafusion plan in duckdb or ishmus?
No — this is entirely inside DataFusion. Nothing here involves running a DataFusion plan in DuckDB or Isthmus. That opening line is my fault: I was noting that I could not find a reference producer to cross-check against, and leaving it above the bug report made it look like the problem. Sorry for the confusion.
The spec ([logical_relations.md v0.102.0](https://github.com/substrait-io/substrait/blob/v0.102.0/site/docs/relations/logical_relations.md#aggregate-operation)):
> To further disambiguate which record belongs to which grouping set, an aggregate relation with more than one grouping set receives an extra `i32` column on the right-hand side. The value of this field will be the zero-based index of the grouping set that yielded the record.
DataFusion identifies that column with its own `__grouping_id`, which holds a bitmask, not an index. The producer does this explicitly — `grouping_set_output_mapping` is documented as mapping "Substrait's `[groups, measures, grouping_id]`" onto DataFusion's `[groups, grouping_id, measures]` — and the consumer reads it back the same way.
Reproducer, DataFusion only. The plan in the issue has sets `(a)` then `(b)`; consumed with `from_substrait_plan` and executed on `main` at `039c0b99d5b`:
```
grouping_set : UInt8
+---+----+--------------+
| a | b | grouping_set |
+---+----+--------------+
| 2 | | 1 | <- row from set (a), spec says 0
| 1 | | 1 |
| | 20 | 2 | <- row from set (b), spec says 1
| | 10 | 2 |
+---+----+--------------+
```
So the spec asks for an `i32` holding `0` and `1`, and we return a `UInt8` holding `1` and `2`.
Why this is easy to miss: the producer writes the same bitmask the consumer reads, so a DataFusion-to-DataFusion round trip agrees with itself and nothing looks wrong from inside DataFusion. It only diverges against an implementation that follows the spec. `(a)` then `(b)` is the smallest case where the two differ — for `(a, b)` then `(a)` the bitmask and the index are both `0, 1`, which is why the existing round-trip tests pass either way.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "alexandrefimov (via GitHub)" <gi...@apache.org>.
alexandrefimov commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6425622246
No, substrait-java isn't executing anything here.
I'm feeding a standalone Substrait plan directly into DataFusion and comparing its output against the Substrait spec. DataFusion returns the wrong type and values for the grouping set column.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "alexandrefimov (via GitHub)" <gi...@apache.org>.
alexandrefimov commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6412215582
@alamb I'm building a [Substrait conformance suite](https://github.com/alexandrefimov/substrait-conformance-cases) that runs the same plans through different implementations and compares their output schemas and rows. This issue came from testing multi-set aggregates: the trailing column has a different type and value in DataFusion than the spec defines.
DuckDB and Isthmus were mentioned as possible sources of SQL-generated plans for that comparison. I rechecked DataFusion main at `350ca765` today, and the mismatch is still present.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6402286961
But what are you trying to do?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6425039587
I am sorry -- my feeble mind still doesn't understand this. It seems that the claim is that a datafusion generated plan consumed by substrait-java gives the wrong / different answer?
I didn't substrait-java could run queries
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6383225495
Also, can you please explain what you trying to do?
> Neither producer of multi-set aggregates shows what the column holds: Isthmus's emit leaves the index out, and DuckDB's root names one column too few for the readers I ran to accept the plan (duckdb-substrait-extension 287).
How is this related to datafusion? Are you trying to run a datafusion plan in duckdb or ishmus? If so can you please share a (small) reproducer of what you tried and what isn't working?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Substrait: the grouping set column holds `__grouping_id`, not the set's index [datafusion]
Posted by "namanjain24-sudo (via GitHub)" <gi...@apache.org>.
namanjain24-sudo commented on issue #25208:
URL: https://github.com/apache/datafusion/issues/25208#issuecomment-6402474029
Honestly: nothing of mine broke. @alexandrefimov filed this from a Substrait conformance suite and I picked it up off the issue list.
The concrete gap is that the column we emit for a multi-set aggregate doesn't match what the spec says it holds, so a plan DataFusion produces is read differently by a consumer that follows the spec. I have not hit that in a real workload myself.
If that isn't worth the complexity, I am happy to close the PR and leave it to you and @alexandrefimov to decide whether the issue is worth doing at all.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org