Repository navigation
fix: all-null chunks as a null constant, read back masked - #594
Merged
Merged
Conversation
A vortex.constant whose scalar is null, which Rust's compressor writes for every all-null chunk, decoded to a NullArray even for an I64?, Utf8? or bool? column. Every consumer that unwraps a MaskedArray and casts its payload (CLI filter and select, Calcite, Parquet export) then failed on that chunk, or treated the column as a different type from one chunk to the next. Rust canonicalizes a null constant to the dtype's own array with all-invalid validity. The decoder now does the same: a MaskedArray over a constant 0, false or "" placeholder, with a constant-false validity. Decimal and Variant keep the NullArray for now (no placeholder built for them). The vortex-jni all-null test now requires the MaskedArray shape. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Rust's compressor turns an all-invalid array into one vortex.constant with a null scalar before trying any scheme (vortex-compressor cascade.rs). Java wrapped it in vortex.masked over a constant 0 values child and a constant false validity: a different tree for the same data, and a values child that carries nothing. MaskedEncodingEncoder now emits the null constant when the whole chunk is invalid and cascading is on (depth 0 keeps the plain masked shape, as Rust's uncompressed write does). Each all-null column's data segment shrinks from 184 to 118 bytes, and zone stats are unchanged (an all-null zone has no min/max). NullSparseRunEndInteropIntegrationTest writes the same all-null column of I64, I16, F64, bool and utf8 through both writers and requires the same data encodings, and vortex-jni reads every row of Java's file as null. ListViewInteropIntegrationTest's all-null lists now expect the constant. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
Two commits, one per bug:
vortex.constant(what Rust writes for every all-null chunk) decoded to aNullArrayeven forI64?/Utf8?/bool?, so the CLI filter, Calcite and Parquet export failed on it. It now decodes as Rust canonicalizes it: aMaskedArraywith every row invalid.vortex.constant, as Rust's compressor does (cascade.rs), instead ofvortex.masked(constant 0, constant false).Parity test: the same all-null column (I64, I16, F64, bool, utf8) through vortex-jni and Java gives the same data encodings, and vortex-jni reads Java's file as all null.
🤖 Generated with Claude Code