andygrove opened a new pull request, #26767: URL: https://github.com/apache/datafusion/pull/26767
## Which issue does this PR close? - Part of #23393 on the `branch-55` release line. - Backports the merged fix in #26154. ## Rationale for this change In 55.x, a grouped `MIN`/`MAX` over strings or binary values reports less memory than it holds. Its `size()` counts the current length of each group's value rather than the capacity that the value's buffer keeps. So when a long value is replaced by a shorter one, the buffer keeps its old capacity but the reported size drops. Unused capacity in the outer per-group vector is not counted either. Under a memory limit, an aggregate can then hold more memory than the pool knows about, and it spills later than it should, or not at all. #26154 fixed this on `main`, but it is not in 55.1.0 or 55.2.0 and first ships in 56.0.0. This backport is for a 55.3.0 patch release, if one is made. ## What changes are included in this PR? - A cherry-pick with `git cherry-pick -x` of the commit that #26154 merged (0d79252f61bd8777990b2749c216ffd07079c30d). This PR changes nothing else. - `MinMaxBytesState` tracks the retained capacity of each group's value incrementally, keeping `size()` O(1). `size()` reports that capacity plus the outer vector's capacity. The change is confined to `datafusion/functions-aggregate/src/min_max/min_max_bytes.rs`. - The cherry-pick conflicts only in the test module. On `main`, the file already had a `tests` module containing `preserving_selected_min_values`, which uses `GroupSelection` and `evaluate_preserving`, and neither exists on `branch-55`. The resolution adds the `tests` module with only the test from #26154, `size_tracks_growth_and_emit_reuse`. The non-test changes apply as they are on `main`. ## What is the testing strategy for this PR? Covered by `size_tracks_growth_and_emit_reuse`, the test #26154 added. On `branch-55`: - `cargo fmt --all -- --check` - `cargo clippy -p datafusion-functions-aggregate --all-targets -- -D warnings` - `cargo test -p datafusion-functions-aggregate`: 196 unit tests and 2 doc tests passed, including `size_tracks_growth_and_emit_reuse` ## Are there any user-facing changes? No. Query results and public APIs are unchanged. Grouped byte `MIN`/`MAX` now reports its memory more accurately, so under a memory limit it may spill or fail to reserve memory earlier than before. -- 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]
