perf(array): accumulate nested builder validity without a null buffer [builders-child-stack]#8966
Open
robert3005 wants to merge 1 commit into
Conversation
A nested builder learns about validity from two sources: one row at a time as scalars are appended, and a whole array's worth at a time as arrays are. Only the first needs a null buffer, but `LazyBitBufferBuilder` treated both the same, so every appended array had its validity executed into a `Mask` and its bits copied. `ValidityBuilder` keeps a whole array's validity as a run and concatenates the runs at the end, the way `Validity::concat` already does for `StructArray::try_concat`. `AllValid` and `AllInvalid` runs cost nothing, array-backed runs are bool arrays that are already built, and a builder that only ever saw uniform validity still answers from its nullability rather than producing a bool array. Runs shorter than the chunk threshold are still copied into the null buffer, so a builder fed one row at a time does not accumulate a run per row. `StructBuilder`, `ListBuilder`, `ListViewBuilder` and `FixedSizeListBuilder` use it; the leaf builders keep `LazyBitBufferBuilder`. Signed-off-by: Claude <noreply@anthropic.com>
robert3005
force-pushed
the
claude/builders-min-chunk-len-9ze0t6
branch
from
July 25, 2026 21:07
cc36904 to
6f3ef13
Compare
robert3005
force-pushed
the
claude/builders-lazy-validity-9ze0t6
branch
from
July 25, 2026 21:07
dc71bbf to
85f8dcd
Compare
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
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.
Rationale for this change
Stacked on #8965, which is stacked on #8964 — review those first.
A nested builder learns about validity from two sources: one row at a time as scalars are appended, and a whole array's worth at a time as arrays are. Only the first needs a null buffer.
LazyBitBufferBuildertreated both the same, so every appended array paid— executing the array's validity into a
Maskand copying its bits into a running buffer.StructArray::try_concathas never done that. It hands its per-chunk validities toValidity::concat, which returnsAllValid/AllInvalid/NonNullablewhen they are uniform and aValidity::Array(ChunkedArray<Bool>)otherwise — no bitmap, no execution. That gap is the one thing standing between the builders and the swizzle helpers they are meant to replace: routing chunked struct canonicalization throughStructBuildertoday would trade a lazy validity for a materialized one on every nullable chunked struct, which is the kind of regression that shows up in a scan rather than in a test.What changes are included in this PR?
ValidityBuilder(vortex-array/src/builders/validity.rs) mirrorsChildBuilder: appended validity is kept as a run, bits appended one at a time go into aLazyBitBufferBuilder, andfinishflushes the buffer into the run list and concatenates.StructBuilder,ListBuilder,ListViewBuilderandFixedSizeListBuilderhold their validity this way; the leaf builders keepLazyBitBufferBuilderunchanged.The call sites lose their
execute_mask:Two details worth calling out for review:
set_min_chunk_lenfrom feat(array): let callers choose a nested builder's chunk threshold [builders-child-stack] #8965 so validity splits on the same boundaries as the children. Without it, a builder fed one row at a time would accumulate a run per row.Validity::concattreatsNonNullableandAllValidas different kinds and falls back to a bool array when both appear — which happens as soon as a non-nullable array is appended next to a scalar. Both mean "no nulls", sofinish_with_nullabilitychecksdefinitely_no_nullsacross the runs first and answers from the declared nullability instead. Without that, appending an array and then a scalar to a non-nullable builder produced aValidity::Arrayand tripped the nullability assertion.Eight unit tests on
ValidityBuilder(runs preserved, uniform runs collapsing, short validity materialized, bits and runs interleaving in order,set_validityreplacing runs, non-nullable finishing non-nullable, finish resetting), plus a builder-level test asserting that two appended nullable struct arrays come back with a chunked bool validity rather than one copied buffer.What APIs are changed? Are there any user-facing changes?
None —
ValidityBuilderispub(crate). The observable change is representational: a nested builder that consumed nullable arrays now returnsValidity::Array(ChunkedArray<Bool>)where it previously returned a singleBoolArray. That is the same shapeStructArray::try_concathas always produced.Checks
cargo test -p vortex-array— 3174 lib testscargo test --lib --testsgreen forvortex-arrow,vortex-layout,vortex-scan,vortex-file,vortex-datafusion,vortex-btrblocks,vortex-compressor,vortex-row,vortex,vortex-ipccargo +nightly fmt --all,cargo clippy -p vortex-array --all-targets --all-featuresGenerated by Claude Code
Stacked PR Chain: builders-child-stack
Generated by Claude Code