Generic Rows-backed GroupColumn so nested GROUP BY keys stay on the column-wise fast path
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
Part of #22715.
### Problem
`GroupValuesColumn` (the column-wise multi-column group-by path) is all-or-nothing: if a single group-by column has no type-specialized `GroupColumn`, `supported_schema` returns `false` and the **entire** key falls back to the row-wise `GroupValuesRows` — even when every other column would have qualified for the column-wise fast path. Nested types (`Struct`, `List`, `FixedSizeList`, ...) currently trigger this, so a mixed key like `GROUP BY int_col, struct_col` loses the fast path (and its lower memory footprint) entirely because of that one column.
### Proposal
(Suggested by @alamb in https://github.com/apache/datafusion/pull/23128#issuecomment-4903031891.)
Add a single generic `GroupColumn` backed by arrow's row format (a one-field `RowConverter`) as a fallback. Then `GroupValuesColumn` stays usable for any row-encodable type, and row-encoding is confined to just the columns that lack a native builder — instead of dropping the whole key onto `GroupValuesRows`. Hashing is unchanged (done on the raw input columns, which already supports nested types).
### Scope (first step)
The generic `RowsGroupColumn` framework + routing for the nested types `RowConverter` can encode (`Struct` / `List` / `LargeList` / `FixedSizeList`, recursively). A quick local prototype suggests a mixed native + nested key uses roughly **half** the memory of the all-rows fallback, with grouping identical to the `GroupValuesRows` reference.
### Follow-ups
- Cover types `RowConverter` cannot encode (e.g. `Map`, which needs key-order normalization) so `supported_schema` can always be `true`.
- With full coverage, retire `GroupValuesRows` entirely.
- Batch-scoped cache for the scalar `equal_to` / `append_val` path (needs a `GroupColumn` batch hook).
Contributor guide
Assessment
This issue has not been assessed yet.