Skip to content

perf(array): accumulate nested builder validity without a null buffer [builders-child-stack]#8966

Open
robert3005 wants to merge 1 commit into
claude/builders-min-chunk-len-9ze0t6from
claude/builders-lazy-validity-9ze0t6
Open

perf(array): accumulate nested builder validity without a null buffer [builders-child-stack]#8966
robert3005 wants to merge 1 commit into
claude/builders-min-chunk-len-9ze0t6from
claude/builders-lazy-validity-9ze0t6

Conversation

@robert3005

@robert3005 robert3005 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

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. LazyBitBufferBuilder treated both the same, so every appended array paid

self.nulls.append_validity_mask(&array.validity()?.execute_mask(array.len(), ctx)?);

— executing the array's validity into a Mask and copying its bits into a running buffer.

StructArray::try_concat has never done that. It hands its per-chunk validities to Validity::concat, which returns AllValid/AllInvalid/NonNullable when they are uniform and a Validity::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 through StructBuilder today 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) mirrors ChildBuilder: appended validity is kept as a run, bits appended one at a time go into a LazyBitBufferBuilder, and finish flushes the buffer into the run list and concatenates. StructBuilder, ListBuilder, ListViewBuilder and FixedSizeListBuilder hold their validity this way; the leaf builders keep LazyBitBufferBuilder unchanged.

The call sites lose their execute_mask:

self.nulls.append_validity(array.validity()?, array.len(), ctx)?;

Two details worth calling out for review:

  • Runs shorter than the chunk threshold are still copied into the null buffer, driven by the same set_min_chunk_len from 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::concat treats NonNullable and AllValid as 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", so finish_with_nullability checks definitely_no_nulls across 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 a Validity::Array and tripped the nullability assertion.

Eight unit tests on ValidityBuilder (runs preserved, uniform runs collapsing, short validity materialized, bits and runs interleaving in order, set_validity replacing 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 — ValidityBuilder is pub(crate). The observable change is representational: a nested builder that consumed nullable arrays now returns Validity::Array(ChunkedArray<Bool>) where it previously returned a single BoolArray. That is the same shape StructArray::try_concat has always produced.

Checks

  • cargo test -p vortex-array — 3174 lib tests
  • cargo test --lib --tests green for vortex-arrow, vortex-layout, vortex-scan, vortex-file, vortex-datafusion, vortex-btrblocks, vortex-compressor, vortex-row, vortex, vortex-ipc
  • cargo +nightly fmt --all, cargo clippy -p vortex-array --all-targets --all-features

Generated by Claude Code

Stacked PR Chain: builders-child-stack

PR Title Merges Into
#8964 feat(array): builders no longer canonicalize their children [builders-child-stack] N/A
#8965 feat(array): let callers choose a nested builder's chunk threshold [builders-child-stack] #8964
#8966 perf(array): accumulate nested builder validity without a null buffer [builders-child-stack] #8965
#8967 refactor(array): canonicalize chunked structs and FSLs through the builder [builders-child-stack] #8966

Generated by Claude Code

@claude claude Bot changed the title perf(array): accumulate nested builder validity without a null buffer perf(array): accumulate nested builder validity without a null buffer [builders-child-stack] Jul 25, 2026
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
robert3005 force-pushed the claude/builders-min-chunk-len-9ze0t6 branch from cc36904 to 6f3ef13 Compare July 25, 2026 21:07
@robert3005
robert3005 force-pushed the claude/builders-lazy-validity-9ze0t6 branch from dc71bbf to 85f8dcd Compare July 25, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants