You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 4d167a1
Browse filesBrowse the repository at this point in the historyBrowse files
fix(physical-plan): replace approximate map_size accounting with exact HashTable::allocation_size() in GroupValuesRows (#25750)
## Which issue does this PR close?
Closes#25735
Related to #25188
## Rationale for this change
**Background**
`GroupValuesRows` is the general-purpose fallback implementation used
for grouping complex nested types (like `Struct`, `List`, `Map`) and
multi-column schemas.
Its `size()` method is highly critical because it tells DataFusion's
memory pool exactly how much memory is being consumed, which dictates
when the system should safely spill to disk to prevent Out of Memory
(OOM) errors.
**The Problem**
Previously, the memory size of the hash table was poorly estimated. It
naively calculated the size as `capacity * size_of::<(u64, usize)>()`.
This completely ignored the actual internal memory overhead of the hash
table (like control bytes and entry slots), meaning DataFusion was
under-reporting its memory usage.
## What changes are included in this PR?
This PR completely removes the inaccurate manual `self.map_size`
calculation. Instead, it directly calls `self.map.allocation_size()`
inside the `size()` function. This ensures that DataFusion's memory pool
accurately accounts for every single byte of overhead used by the
underlying hash table.
## Are these changes tested?
Yes! I have added two comprehensive tests to verify the exact memory
accounting:
- `test_exact_hash_table_allocation_accounting_multi_column`: Validates
precise memory tracking during inserts, partial emits, full emits, and
table shrink operations for multi-column groupings.
- `test_exact_hash_table_allocation_accounting_nested_keys`: Validates
the exact tracking for nested types (Lists) to ensure the row converter
and map sizes are perfectly aligned.
## Are there any user-facing changes?
No API changes. This is strictly an internal reliability fix to improve
the accuracy of query execution memory tracking and prevent potential
OOM crashes.
Co-authored-by: kosiew <[email protected]>
0 commit comments