andygrove opened a new pull request, #27243: URL: https://github.com/apache/datafusion/pull/27243
## Which issue does this PR close? - Backport of https://github.com/apache/datafusion/pull/25853 to `branch-56` - Part of https://github.com/apache/datafusion/issues/24461 - Closes https://github.com/apache/datafusion/issues/25851 ## Rationale for this change Aggregate spilling cuts batches by row count, so a row of aggregate state that holds a whole group (an `array_agg` list, a `string_agg` string) can make a spilled batch far larger than its row count suggests. Replay then fails with `Failed to allocate additional ...`. #25853 bounds the batches by bytes as well, and was merged to `main` on 2026-11-08. ## What changes are included in this PR? A cherry-pick of #25853 (32dd1d2c5), with two changes for `branch-56`: - `aggregate_memory_spill.slt`: the new case conflicted with cases that are only on `main` (byte array keys in compressed spill files from #26960, and native DISTINCT counts from #25988). The backport keeps only this PR's case, which is Case I here (it is Case J on `main`), and the `target_partitions` reset after it. - `spill/mod.rs`: `main`'s `clippy.toml` disallows `DictionaryArray::with_values`, which is why #25853 has `#[expect(clippy::disallowed_methods)]` on the call in `gc_array`. `branch-56` does not disallow it, so the unfulfilled `expect` would fail clippy. The backport drops the attribute. The Rust sources otherwise cherry-picked cleanly. ## What is the testing strategy for this PR? The same tests as #25853: the new `aggregate_memory_spill.slt` case, and the unit tests in `spilled_row_sizes.rs`, `spill/mod.rs`, `sorts/stream.rs` and `sorts/streaming_merge.rs`. Run on this branch: ``` cargo fmt --all cargo clippy -p datafusion-physical-plan --no-deps --all-targets -- -D warnings cargo test -p datafusion-physical-plan --lib # 2933 passed cargo test --profile release-nonlto -p datafusion-sqllogictest --test sqllogictests -- aggregate_memory_spill ``` (The clippy command also allowed `clippy::from_iter_instead_of_collect`, which fails on untouched code with the stable 1.97 toolchain used here.) ## Are there any user-facing changes? No API changes. Aggregates that spill state with large rows now spill and replay it in batches bounded by bytes, so queries that failed with `Failed to allocate additional ...` complete under the same memory limit. -- 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]
