Skip to content

fix(writer): declare the real zone stride so vortex-jni prunes correctly (#418) - #421

Merged
dfa1 merged 1 commit into
mainfrom
fix/418-zone-length
Oct 1, 2026
Merged

dfa1 merged 1 commit into
mainfrom
fix/418-zone-length

Conversation

@dfa1

@dfa1 dfa1 commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Closes #418. Stacked on #420. The filtered vortex-jni reads in these tests abort the JVM without #420's alignment fix. Merge #420 first; this PR then retargets to main.

Bug

Our legacy vortex.stats zone map holds one zone per chunk (one chunk per writeChunk call), but declared WriteOptions#chunkSize() (default 65 536) as its zone length. Rust reads the declared length as a uniform stride: row r belongs to zone r / zone_len (ZonedReader::zone_range). So a filtered vortex-jni read pruned using the wrong zone's stats:

chunks filter vortex-jni before after
[5,5,5,3], default options id >= 7 [] 7..17
[4,4,2,4] id >= 10 [] 10..13

With small batches every row maps to zone 0, whose max is chunk 0's, so everything gets pruned.

Fix

ZoneMapStatCodec.uniformZoneLength(rowCounts):

  • every chunk but the last has the same length, and the last is non-empty and no longer: declare that length, so Rust's stride matches our zones exactly and Rust prunes correctly;
  • otherwise declare 0, Rust's "no stride" (LegacyStatsLayout::zoned_reader). Rust then reads the column unpruned, which is correct and only slower.

vortex-java's reader doesn't use the declared length for this layout; it places legacy zones on the physical chunks (#419). So nothing changes on our side.

Not done

  • Switching to vortex.zoned (what Writer emits the legacy vortex.stats zone-map layout; Rust writes vortex.zoned with aggregate-spec metadata #418 originally asked for): not needed for correctness. vortex.stats stays the more compatible choice for older readers.
  • WriteOptions.chunkSize is now unused. Its only effect was this wrong zone length; writeChunk never splits batches. It should either be removed or start splitting batches; that's a separate decision.
  • Irregular batches lose vortex-jni pruning (still correct). Writing per-stride zones independent of chunks would recover it, at real cost; only worth it if there's demand.

Tests

  • ZoneMapStatCodecTest.UniformZoneLength: 9 shape cases (short tail, short middle chunk, longer tail, empty tail, single chunk, no rows, u32 overflow).
  • javaWriter_jniReader_zoneMapped_filterKeepsEveryMatch: three chunk shapes through a filtered vortex-jni read. The two multi-chunk shapes return [] with the previous writer (verified by reverting it). All pass now.
  • Full ./mvnw verify is green.

🤖 Generated with Claude Code

…tly (#418)

The legacy vortex.stats zone map holds one zone per chunk but declared
WriteOptions#chunkSize() as its zone length. Rust reads that length as a
uniform stride (row r in zone r / len), so whenever batches were not
exactly chunkSize rows a filtered vortex-jni read pruned on the wrong
zone's stats: with default options and small batches it returned no rows
at all.

Declare the shared chunk length when every chunk but the last has it,
and 0 otherwise, which Rust treats as "no stride" and reads unpruned.
vortex-java's own reader places legacy zones on physical chunks and is
unaffected.

Closes #418

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dfa1
dfa1 force-pushed the fix/418-zone-length branch from c193f95 to 8087363 Compare October 1, 2026 20:08
@dfa1
dfa1 changed the base branch from fix/buffer-alignment to main October 1, 2026 20:08
@dfa1
dfa1 merged commit bea5bd2 into main Oct 1, 2026
6 checks passed
@dfa1
dfa1 deleted the fix/418-zone-length branch October 1, 2026 20:21
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.

Writer emits the legacy vortex.stats zone-map layout; Rust writes vortex.zoned with aggregate-spec metadata

1 participant