adriangb opened a new pull request, #12184: URL: https://github.com/apache/arrow-rs/pull/12184
# Which issue does this PR close? - Closes https://github.com/apache/arrow-rs/issues/7186 # Rationale for this change Sliced arrays, view arrays, dictionaries and dense unions can keep memory alive that their rows do not reference. Downstream users (for example DataFusion spilling) each work around this on their own. The issue asks for one kernel in `arrow-select`. # What changes are included in this PR? A new `arrow_select::minify` module with `minify(&dyn Array) -> Result<ArrayRef, ArrowError>`. The result is logically equal to the input, has offset zero, and its buffers hold only referenced data. `minify` recurses into children, dictionary values and run-end encoded values. | Type | Behavior | | --- | --- | | Primitive (incl. decimal, temporal, interval), Boolean, FixedSizeBinary | Copy the visible range. Boolean bit offsets become zero. | | Null buffers | Copy the visible bits. Drop the buffer if it has no nulls. | | Utf8 / LargeUtf8 / Binary / LargeBinary | Rebase offsets to zero and copy only the referenced bytes. | | Utf8View / BinaryView | Use the existing `gc`. Inline-only arrays keep no data buffers. | | List / LargeList / Map | Slice the child to the referenced range, rebase offsets, recurse. | | ListView / LargeListView | Pick the smaller layout: slice the child to the span of the valid views (keeps overlap shared), or copy views back to back with `take` (drops gaps). Null rows get offset and size zero. | | FixedSizeList, Struct, sparse Union | Recurse into the (already sliced) children. | | Dense Union | Keep only referenced child rows (via `take`) and remap offsets. | | RunEndEncoded | Narrow runs and values to the slice, rebase run ends, recurse. | | Dictionary | Prune unreferenced values with `garbage_collect_dictionary`, then recurse into keys and values. | Reuse policy: `minify` copies a buffer only when its allocation is larger than the visible data needs (rounded up to 64 bytes). Already-minimal arrays are returned without copies, so `minify(minify(a))` copies nothing. Open questions for reviewers: - The issue proposes a non-exhaustive `MinifyOptions`. This PR ships only the all-defaults function to keep the first step small. Options (for example `zero_size_nulls`, dictionary deduplication, merging equal runs) could follow as `minify_with_options`. Is that split acceptable, or should the options struct land now? - `minify` keeps the value ranges of null list/byte rows. Zero-sizing them (as in https://github.com/apache/arrow-rs/pull/9970) seems like a natural option. - Moving the IPC writer logic onto this kernel is left for a follow-up. - A shared buffer counts as oversized, so `minify` copies it even if other arrays keep it alive. The docs call this out. # Are these changes tested? Yes. Unit tests cover every type above, plus nested combinations: dictionary of `Utf8View`, dictionary of list, dictionary inside struct and list children, run-end encoded dictionary, fixed-size list of struct, dense union of lists, and lists over sliced children. A shared `check` helper slices each array several ways and asserts that the result: - is logically equal to the slice and passes `validate_full`, - has offset zero and the same data type, - allocates no more (and for small slices strictly less) buffer memory than the slice, - reuses all buffers when minified a second time. No benchmark is included yet. # Are there any significant user-facing changes? New public API only: `arrow_select::minify::minify`. # AI usage disclosure Claude Code wrote the implementation, tests and this description under my direction. 🤖 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]
