Make StructArray use the array_slots macro accessors - #9006
Draft
joseph-isaacs wants to merge 2 commits into
Draft
Make StructArray use the array_slots macro accessors#9006joseph-isaacs wants to merge 2 commits into
joseph-isaacs wants to merge 2 commits into
Conversation
Previously StructSlots was only used for its two index constants; the generated view, ext trait, and conversions were dead. Make StructArrayExt a subtrait of the generated StructArraySlotsExt and route slot access through the generated accessors, matching UnionArrayExt: - struct_validity() uses the generated `validity()` slot accessor. - iter_unmasked_fields() / unmasked_fields() / unmasked_field() use the generated `fields()` SlotSlice (zero-copy borrow) instead of indexing raw slots with hand-written vortex_expect. - Export StructArraySlotsExt for parity with UnionArraySlotsExt. Read paths are unchanged in cost (SlotSlice is a Copy borrow); this only removes the duplicated hand-rolled slot indexing. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BF3K2eZTEnCsqMsN3wGtc
robert3005
approved these changes
Jul 27, 2026
- SlotSlice now implements IntoIterator (via a named SlotSliceIter that
is ExactSizeIterator + DoubleEndedIterator), so it can be used directly
in for-loops and passed anywhere an iterator is expected without
calling .iter().
- Route StructArray field callers through iter_unmasked_fields() instead
of the owned-Vec unmasked_fields():
- Pure-iteration sites (is_constant, listview conversion, tui browse)
no longer allocate a Vec or clone every field just to read them.
- Construction sites (struct mask reduce, push_validity_into_children,
masked struct execute, sparse canonicalization) now pass
iter_unmasked_fields().cloned() straight into the
impl IntoIterator<Item = ArrayRef> constructors, so the single
collect happens inside the constructor instead of building an
intermediate Vec first (saves one allocation + N moves per call).
- Widen iter_unmasked_fields() to impl ExactSizeIterator (its backing
SlotSliceIter already is), enabling len()-based capacity hints.
unmasked_fields() (owned Vec) is kept for the remaining indexed/test
callers.
Verification:
- cargo test -p vortex-array --lib -- arrays::struct_ arrays::masked aggregate_fn::fns::is_constant arrays::listview patched::array::tests (219 passed)
- cargo clippy --all-targets -p vortex-array
- cargo check -p vortex-tui -p vortex-sparse -p vortex-layout -p vortex-row -p vortex-compressor
- cargo +nightly fmt
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BF3K2eZTEnCsqMsN3wGtc
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.
Rationale for this change
Follow-up to #8950 (variadic
#[array_slots], now merged). After that PR,StructSlotswas only used for its two slot-index constants — the generated view, ext trait, and conversions were dead code.Unionalready subtypes its generated ext trait;Structshould too, so the macro is the single source of truth for struct slot access.What changes are included in this PR?
StructArrayExtnow extends the generatedStructArraySlotsExtsupertrait (matchingUnionArrayExt).struct_validity()uses the generatedvalidity()accessor.iter_unmasked_fields()/unmasked_fields()/unmasked_field()use the generatedfields()SlotSlice(aCopy, zero-copy borrow) rather thanslots()[FIELDS_OFFSET + i]+ hand-writtenvortex_expect.StructArraySlotsExtfor parity withUnionArraySlotsExt.What APIs are changed? Are there any user-facing changes?
No signature changes.
StructArraySlotsExtis newly exported. Read-path cost is unchanged —SlotSliceis a borrow, so this only removes duplicated hand-rolled slot indexing.Verification
cargo test -p vortex-array --lib -- arrays::struct_ arrays::union arrays::masked— passedcargo clippy --all-targets -p vortex-array— cleancargo +nightly fmt -p vortex-array— cleanCould not run a full
cargo build --workspacein this environment (protocmissing, DuckDB download blocked by network policy, limited disk); crates touching the changed API were checked individually.🤖 Generated with Claude Code
Generated by Claude Code