Skip to content

fix: write empty nested list streams with structural layouts - #10533

Merged
myrrc merged 1 commit into
vortex-data:developfrom
danking:dk/fix-empty-nested-list-writes
Oct 23, 2026
Merged

myrrc merged 1 commit into
vortex-data:developfrom
danking:dk/fix-empty-nested-list-writes

Conversation

@danking

@danking danking commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Writing list<list<int32>> rows containing only empty or null outer lists fails with ListLayoutStrategy needs at least one chunk when structural list layouts are enabled. This reproduces in PyVortex 0.86.1 and 0.88.0 with VORTEX_EXPERIMENTAL_LIST_LAYOUT=1.

Changes

Handle an empty list stream before spawning child writers by returning the same typed zero-row chunked layout used by the flat writer. Pass the first chunk explicitly into the nonempty transposition path, preserving its sequence ID, bounded streaming, and existing layout behavior.

Add file write/read regression cases for empty lists, nullable empty lists, all-null lists, and mixed empty/null rows. Replace the old empty-stream rejection test with nullable/nonnullable dtype and row-count checks.

Validation

  • All four new file-roundtrip cases fail on unmodified develop (18aebea) with the reported error.
  • After the fix, 76 selected tests pass, including all four regression cases and both empty-stream nullability cases:
    cargo nextest run --locked -p vortex-file -p vortex-layout -E 'test(nested_) | test(writer_empty_chunks) | test(layouts::list::writer::tests)'
  • Formatted both changed files with CI's pinned nightly-2026-09-10 rustfmt; git diff --check passes.

Implemented with OpenAI Codex assistance.

Signed-off-by: Daniel King <dan@spiraldb.com>
@danking danking self-assigned this Oct 15, 2026
@danking
danking requested a review from mhk197 October 15, 2026 21:24
@codspeed

codspeed Bot commented Oct 15, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 2.99%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ 23 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

ℹ️ 40 benchmarks have no flame graph

The flamegraph could not be generated because we failed to resolve the symbols in the profiling trace. This happens when we fail to harvest information about language frames.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
❌ 4 regressed benchmarks
✅ 2253 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ ℹ️ Simulation test_scan_filter_to_arrow_array[int_float] 3.5 ms 4.1 ms -13.81%
❌ ⚠️ Simulation test_repeated_scan_filter[int] 2.5 ms 2.8 ms -13.04%
❌ ⚠️ Simulation test_scan_filter_to_arrow_array[int] 2.5 ms 2.9 ms -12.57%
❌ ℹ️ Simulation test_repeated_scan_filter[int_float] 3.4 ms 3.9 ms -11.07%
⚡ ℹ️ Simulation test_repeated_scan_scalar_at[int_float] 486.7 µs 394.2 µs +23.48%
⚡ WallTime filter_neon[80] 2.9 µs 2.5 µs +15.82%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing danking:dk/fix-empty-nested-list-writes (d54e60f) with develop (18aebea)

Open in CodSpeed

@mhk197 mhk197 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@myrrc myrrc added the changelog/fix A bug fix label Oct 23, 2026
@myrrc
myrrc enabled auto-merge (squash) October 23, 2026 14:05
@myrrc
myrrc disabled auto-merge October 23, 2026 14:06
@myrrc
myrrc merged commit 3cc1a5f into vortex-data:develop Oct 23, 2026
159 of 162 checks passed
joseph-isaacs added a commit that referenced this pull request Oct 23, 2026
…ist test

#10533 added `nested_empty_list_roundtrip` calling `ListArray::try_new`
without the `&mut ExecutionCtx` argument it now takes, which broke the
`vortex-file` test build on develop.

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BL868qtvZQwUKzuhNuWAS2
mhk197 added a commit that referenced this pull request Oct 23, 2026
## Summary

Develop doesn't compile vortex-file's tests. #10533 added
`nested_empty_list_roundtrip`, which calls `ListArray::try_new` with
three arguments, but #10594 had already added a fourth, the execution
context. Both passed CI on their own. Since #10533 merged, the Rust
build, lint and test jobs fail on develop and on every PR.

## Changes

- `nested_empty_list_roundtrip` creates an execution context and passes
it to both `ListArray::try_new` calls, as
`nested_list_of_list_roundtrip` just above it already does.

## Testing

- `cargo nextest run -p vortex-file -E
'test(/nested_empty_list_roundtrip|nested_list_of_list_roundtrip/)'` (5
passed)
- `cargo clippy -p vortex-file --all-targets --all-features -- -D
warnings`
- `cargo +nightly-2026-09-10 fmt -p vortex-file --check`

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants