perf: optimize spark_array_compact (no-op fast path for null-free arrays)#4934
Open
andygrove wants to merge 3 commits into
Open
perf: optimize spark_array_compact (no-op fast path for null-free arrays)#4934andygrove wants to merge 3 commits into
spark_array_compact (no-op fast path for null-free arrays)#4934andygrove wants to merge 3 commits into
Conversation
…ree arrays) array_compact only removes null elements. When the values buffer has no null elements there is nothing to remove, so the result is bit-identical to the input. Add a fast path that returns the input unchanged in that case, reusing its buffers zero-copy and skipping the per-element MutableArrayData::extend loop. The null-containing path is left byte-for-byte unchanged so shapes with sparse or dense element nulls do not regress. Add Rust unit tests (the expression previously had only SQL/Scala coverage) and a criterion benchmark covering no-null, null-row, sparse-null, and dense-null shapes.
andygrove
marked this pull request as ready for review
July 15, 2026 14:19
15 tasks
mbutrovich
reviewed
Jul 15, 2026
mbutrovich
left a comment
mbutrovich
left a comment
Contributor
There was a problem hiding this comment.
First pass, thanks @andygrove!
…hild logical_null_count() has O(1) overrides on Dictionary/Run/Null arrays that avoid materializing a fresh null bitmap, while logical_nulls() would allocate on exactly this path for those types. Adds null_array_values_child_all_rows_empty covering the List<Null> shape produced by make_array(NULL) so a future refactor that dropped logical_null_count() would fail.
mbutrovich
approved these changes
Jul 22, 2026
mbutrovich
left a comment
mbutrovich
left a comment
Contributor
There was a problem hiding this comment.
This LGTM after revision, thanks @andygrove!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
N/A
Rationale for this change
Optimize existing expression.
What changes are included in this PR?
array_compactonly removes null elements. When the values buffer contains no null elements there is nothing to remove, so the output is bit-identical to the input. This adds a fast path tocompact_listthat returns the input array unchanged in that case, reusing its buffers zero-copy and skipping the per-elementMutableArrayData::extendloop.The null-containing path is left byte-for-byte unchanged, so arrays with sparse or dense element nulls take exactly the same code as before and do not regress. This deliberately avoids the per-row run-batching approach that regressed the dense-null shape in a previous attempt (#4886).
Also adds Rust unit tests (the expression previously had only SQL file / Scala coverage) and a criterion benchmark covering no-null, null-row, sparse-null, and dense-null shapes.
How are these changes tested?
New Rust unit tests assert the fast path is bit-identical to the input (including null and empty rows) and that the null-removal path is unchanged; existing SQL file tests and
CometArrayExpressionSuitecontinue to cover end-to-end behavior.Benchmark (criterion), baseline
mainvs this branch, 8192-row list columns:The null-free shapes hit the zero-copy fast path; the sparse/dense-null shapes run the unchanged loop and are within noise across repeated samples.