apache / apache/datafusion

Generic Rows-backed GroupColumn so nested GROUP BY keys stay on the column-wise fast path

Open
#23,404 1 comment 1 reaction 1 assignee Claimed by @zhuqi-lucas View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.