rluvaton opened a new pull request, #24889: URL: https://github.com/apache/datafusion/pull/24889
## Which issue does this PR close? - No issue filed; found by a fuzz test that runs every aggregate execution chain under a memory limit. ## Rationale for this change `GROUP BY` on a single nested column (`Struct`, `Map`) returns duplicate groups when the legacy `GroupedHashAggregateStream` spills: the same key comes out as several rows, with the aggregate values split between them. It needs the legacy stream (`datafusion.execution.enable_migration_aggregate = false`, or a `PartialReduce` stage on ordered input), a single nested group key, and enough memory pressure to spill in a `Final` or `Single` stage. Results are silently wrong rather than an error. After spilling, the stream re-aggregates the merged spill files with `GroupOrderingFull`, which requires group ids in first-seen order along the sorted input. The stream recreates its group values collector for that phase to guarantee the order, but only when there is more than one group column, assuming a single column always uses a sequential single-column collector. A single nested column has no specialized single-column collector and is served by `GroupValuesColumn` through a row-backed column, whose vectorized interning assigns new ids out of input order under hash collisions. In a merged batch of 28 sorted rows the ids came out as 0 to 4, then 9 to 13, then 5 to 8. `GroupOrderingFull` then treated a group that was still arriving as complete and emitted it, and the next batch reopened it as a new group. ## What changes are included in this PR? `GroupedHashAggregateStream` now always recreates the group values collector when it switches to merging spill files, instead of only for multi-column keys. ## What is the testing strategy for this PR? New integration test `memory_limit::legacy_stream_nested_key_spill_keeps_groups_unique`: a 200k-row table grouped by a struct of a list and an integer, with null and empty lists, null numbers and null structs mixed in, aggregated with six aggregates including `count(distinct)` on the legacy stream under a 4 MB `FairSpillPool` with a 64-row batch size, compared against the same query with unlimited memory. Without the fix it fails deterministically with 70 rows instead of 67, three keys split into two rows whose counts add up to the reference. With the fix it passes. ## Are there any user-facing changes? No API changes. Queries that hit this path now return correct results. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
