Skip to content

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

Open
danking wants to merge 1 commit into
vortex-data:developfrom
danking:dk/fix-empty-nested-list-writes
Open

danking wants to merge 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

This branch has not been deployed

No deployments
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.

1 participant