Skip to content

Commit 512cfbd

Browse files
dfa1claude
andcommitted
fix: propagate validity in RunEnd and Sparse lazy decoders (#225, #226)
Two silent-null-loss bugs surfaced by Raincloud conformance triage round 4, both the same class as #210 (validity dropped when unwrapping a MaskedArray child). Fixed by re-wrapping the decoded result in a MaskedArray whose per-row validity mirrors the Rust reference vtables — expressed lazily, O(runs)/O(patches) not O(rows), since n can be huge (uci-online-retail has 499712 rows). RunEnd (#225): a nullable run-value dropped its mask, so a null run expanded to a filler value (uci-online-retail `customerid` u16?: null rows emitted the FoR base). Per Rust `ValidityVTable<RunEnd>`, a RunEnd array's validity IS a RunEnd over the same ends whose per-run value is the run-value's validity bit; a row's validity is thus the validity of the run it falls in. Implemented as `LazyRunEndBoolArray(ends, values-validity, offset)`. Sparse (#226), two facets. Null fill was coerced to 0 (`scalarToLong` returns 0 for a null scalar), so a `fill_value: null` array decoded every unpatched position as 0.0 (world-energy-consumption `biofuel_cons_change_pct` f64?). Nullable patch values were stripped, so a patched-but-null position decoded to raw 0 (nuclear_share_energy). Per Rust `ValidityVTable<Sparse>`, row validity is a sparse bool whose fill is `fill_value.is_valid()` and whose per-patch value is the patch value's validity bit — so a position is valid iff (it is a patch AND that patch is valid) OR (it is unpatched AND the fill is non-null). Null fill is detected from the ScalarValue (explicit null_value or no value-bearing field set), not from `scalarToLong`'s lossy 0. Corpus evidence: both slugs now match their Parquet oracle value-for-value (uci-online-retail 4335272 cells, world-energy-consumption 3039010 cells); expected-status.csv flips both gap entries to ok. Closes #225 Closes #226 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 9b66746 commit 512cfbd

8 files changed

Lines changed: 454 additions & 42 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2121
- CSV export handles all-null (`DType.Null`) columns as empty fields instead of throwing `unsupported array type: NullArray`. ([#211](https://github.com/dfa1/vortex-java/issues/211))
2222
- String-dict columns whose values are FSST-compressed no longer fail to scan with `IndexOutOfBoundsException` — the dictionary value offsets are now read at their true ptype width instead of a hardcoded 8-byte stride (uci-magic-gamma-telescope's `class` column decompressed to 4-byte offsets). ([#215](https://github.com/dfa1/vortex-java/issues/215))
2323
- Null rows no longer silently decode as values: wrapper decoders now propagate the row validity that files from the Python bindings carry deep in the encoding tree. Three representations were dropped — a trailing validity child on `fastlanes.bitpacked` (reached through `vortex.alp`/`vortex.zigzag`/`fastlanes.for`, which delegate validity to their encoded child per the Rust `ValidityChild` contract), dict pools with invalid slots, and dict codes with their own validity — across both the eager `vortex.dict` decoder and the lazy dict layout path. Found on real data: penguins and kepler exported invented values (`32.1`, `0.0`) for thousands of null cells. ([#210](https://github.com/dfa1/vortex-java/issues/210))
24+
- `vortex.runend` propagates nullable run-values' validity: a null run now nulls every row it covers instead of expanding to a filler value (uci-online-retail `customerid` u16? nulls previously decoded as the FoR base). Row validity is a lazy run-end bool over the same run-ends, matching the Rust `ValidityVTable<RunEnd>`. ([#225](https://github.com/dfa1/vortex-java/issues/225))
25+
- `vortex.sparse` propagates nullability: a `fill_value: null` array nulls every unpatched position (world-energy-consumption `biofuel_cons_change_pct` f64? previously decoded them as 0.0), and a null patch value nulls its own position. Row validity is a lazy sparse bool whose fill is `fill_value.is_valid()` and whose patch bits are the patch values' validity, matching the Rust `ValidityVTable<Sparse>`. ([#226](https://github.com/dfa1/vortex-java/issues/226))
2426

2527
### Added
2628

docs/compatibility.md

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,12 @@ Per-slug status lives in `integration/src/test/resources/raincloud/expected-stat
6262
(`ok` must pass; `gap:<issue>` must still fail, so a fix flips the entry in the same change;
6363
`untriaged` runs and reports without failing the build). A scheduled workflow
6464
(`raincloud-conformance.yml`) hydrates a size-capped subset weekly. Current triage —
65-
38 `ok`, 2 known gaps (both silent null loss in lazy decoders:
66-
[#225](https://github.com/dfa1/vortex-java/issues/225) RunEnd run-values validity,
67-
[#226](https://github.com/dfa1/vortex-java/issues/226) Sparse null fill / nullable patches);
68-
207 slugs untriaged. Every gap found by earlier rounds is fixed
65+
40 `ok`, 0 known gaps; 207 slugs untriaged. Every gap found so far is fixed
6966
([#206](https://github.com/dfa1/vortex-java/issues/206)[#211](https://github.com/dfa1/vortex-java/issues/211),
7067
[#215](https://github.com/dfa1/vortex-java/issues/215)[#217](https://github.com/dfa1/vortex-java/issues/217),
71-
[#221](https://github.com/dfa1/vortex-java/issues/221)).
68+
[#221](https://github.com/dfa1/vortex-java/issues/221),
69+
[#225](https://github.com/dfa1/vortex-java/issues/225) RunEnd run-values validity,
70+
[#226](https://github.com/dfa1/vortex-java/issues/226) Sparse null fill / nullable patches).
7271

7372
## Encodings
7473

integration/src/test/resources/raincloud/expected-status.csv

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ uci-individual-household-electric-power-consumption,untriaged
219219
uci-iris,ok
220220
uci-magic-gamma-telescope,ok
221221
uci-mushroom,ok
222-
uci-online-retail,gap:225
222+
uci-online-retail,ok
223223
uci-online-retail-ii,untriaged
224224
uci-online-shoppers-purchasing-intention,untriaged
225225
uci-optical-recognition-of-handwritten-digits,ok
@@ -250,7 +250,7 @@ waxal-dagbani-asr-test,untriaged
250250
wdi,untriaged
251251
websight-v01,untriaged
252252
wikipedia-en,untriaged
253-
world-energy-consumption,gap:226
253+
world-energy-consumption,ok
254254
yellow_tripdata_2025,untriaged
255255
youtube-commons-sample,untriaged
256256
zoo-animal-classification,untriaged

reader/src/main/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoder.java

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,33 @@ public Array decode(DecodeContext ctx) {
5555
long n = ctx.rowCount();
5656
DType endsDtype = new DType.Primitive(endsPtype, false);
5757
Array endsArr = ctx.decodeChild(0, endsDtype, numRuns);
58+
Array endsData = endsArr instanceof MaskedArray m ? m.inner() : endsArr;
59+
60+
// Values-side validity mirrors the Rust reference `ValidityVTable<RunEnd>`: a
61+
// RunEnd array's validity IS a RunEnd over the same ends whose per-run value is
62+
// the run-value's validity bit. Row i's validity is thus the validity of the run
63+
// it falls in — expressed lazily as `LazyRunEndBoolArray`, O(numRuns) not O(n)
64+
// (n can be huge, e.g. uci-online-retail 499712 rows). A nullable values child
65+
// surfaces as a MaskedArray; dropping its mask expanded null runs to a filler
66+
// value (e.g. u16? null rows emitting the FoR base) — #225.
67+
Array valuesArr = ctx.decodeChild(1, ctx.dtype(), numRuns);
68+
BoolArray valuesValidity = null;
69+
Array valuesData = valuesArr;
70+
if (valuesArr instanceof MaskedArray m) {
71+
valuesData = m.inner();
72+
valuesValidity = m.validity();
73+
}
5874

5975
if (ctx.dtype() instanceof DType.Utf8 || ctx.dtype() instanceof DType.Binary) {
60-
VarBinArray valuesArr = (VarBinArray) ctx.decodeChild(1, ctx.dtype(), numRuns);
61-
MemorySegment endsSeg = ctx.materialize(endsArr);
62-
return expandStrings(endsSeg, VarBinArray.toOffsetMode(valuesArr, ctx.arena()), endsPtype, numRuns, offset, n, ctx.dtype(), ctx.arena());
76+
MemorySegment endsSeg = ctx.materialize(endsData);
77+
Array result = expandStrings(endsSeg, VarBinArray.toOffsetMode((VarBinArray) valuesData, ctx.arena()),
78+
endsPtype, numRuns, offset, n, ctx.dtype(), ctx.arena());
79+
return withRunValidity(result, valuesValidity, endsData, n, offset);
6380
}
6481

6582
if (ctx.dtype() instanceof DType.Bool) {
66-
Array valuesArr = ctx.decodeChild(1, ctx.dtype(), numRuns);
67-
Array valuesData = valuesArr instanceof MaskedArray m ? m.inner() : valuesArr;
68-
Array endsData = endsArr instanceof MaskedArray m ? m.inner() : endsArr;
69-
return new LazyRunEndBoolArray(ctx.dtype(), n, (BoolArray) valuesData, endsData, offset);
83+
Array result = new LazyRunEndBoolArray(ctx.dtype(), n, (BoolArray) valuesData, endsData, offset);
84+
return withRunValidity(result, valuesValidity, endsData, n, offset);
7085
}
7186

7287
if (!(ctx.dtype() instanceof DType.Primitive p)) {
@@ -77,16 +92,32 @@ public Array decode(DecodeContext ctx) {
7792
// Lazy path: wrap values + ends without expanding into an n-sized buffer.
7893
// VarBin keeps the eager path above — offset rebasing doesn't trivially
7994
// express as binary-search-on-read.
80-
Array valuesArr = ctx.decodeChild(1, ctx.dtype(), numRuns);
81-
Array valuesData = valuesArr instanceof MaskedArray m ? m.inner() : valuesArr;
82-
Array endsData = endsArr instanceof MaskedArray m ? m.inner() : endsArr;
83-
return switch (valuePtype) {
95+
Array result = switch (valuePtype) {
8496
case I64, U64 -> new LazyRunEndLongArray(ctx.dtype(), n, (LongArray) valuesData, endsData, offset);
8597
case I32, U32 -> new LazyRunEndIntArray(ctx.dtype(), n, (IntArray) valuesData, endsData, offset);
8698
case I16, U16 -> new LazyRunEndShortArray(ctx.dtype(), n, (ShortArray) valuesData, endsData, offset);
8799
case I8, U8 -> new LazyRunEndByteArray(ctx.dtype(), n, (ByteArray) valuesData, endsData, offset);
88100
default -> throw new VortexException(EncodingId.VORTEX_RUNEND, "unsupported ptype " + valuePtype);
89101
};
102+
return withRunValidity(result, valuesValidity, endsData, n, offset);
103+
}
104+
105+
/// Wraps `result` in a [MaskedArray] whose per-row validity is a run-end bool array
106+
/// over the same run-ends: row `i` is valid iff the run it falls in has a valid value.
107+
/// Returns `result` unchanged when the values child carried no validity (all valid).
108+
///
109+
/// @param result the decoded run-end value array
110+
/// @param valuesValidity per-run validity bits from the values child, or `null`
111+
/// @param endsData the run-ends array (raw, mask-unwrapped)
112+
/// @param n logical row count
113+
/// @param offset starting absolute position
114+
/// @return `result`, or a [MaskedArray] carrying the lazy per-row validity
115+
private static Array withRunValidity(Array result, BoolArray valuesValidity, Array endsData, long n, long offset) {
116+
if (valuesValidity == null) {
117+
return result;
118+
}
119+
BoolArray rowValidity = new LazyRunEndBoolArray(DType.BOOL, n, valuesValidity, endsData, offset);
120+
return new MaskedArray(result, rowValidity);
90121
}
91122

92123
private static Array expandStrings(

reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java

Lines changed: 81 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.github.dfa1.vortex.reader.array.LazySparseShortArray;
2424
import io.github.dfa1.vortex.reader.array.LongArray;
2525
import io.github.dfa1.vortex.reader.array.MaskedArray;
26+
import io.github.dfa1.vortex.reader.array.MaterializedBoolArray;
2627
import io.github.dfa1.vortex.reader.array.ShortArray;
2728
import io.github.dfa1.vortex.reader.array.VarBinArray;
2829

@@ -63,27 +64,37 @@ public Array decode(DecodeContext ctx) {
6364
return decodeVarBin(ctx, n, numPatches, offset, indicesPtype);
6465
}
6566

67+
// Row validity mirrors the Rust reference `ValidityVTable<Sparse>`: it is a sparse
68+
// bool array whose fill is `fill_value.is_valid()` and whose per-patch value is the
69+
// patch value's validity bit. So a position is valid iff (it is a patch AND that
70+
// patch is valid) OR (it is unpatched AND the fill is non-null). Dropping either
71+
// facet lost nulls: a `fill_value: null` array (world-energy `biofuel_cons_change_pct`
72+
// f64?) decoded unpatched rows as 0.0, and a null patch (nuclear_share_energy)
73+
// decoded to raw 0 — #226.
74+
MemorySegment fillBuf = ctx.buffer(0);
75+
ProtoScalarValue fillScalar = decodeFill(fillBuf);
76+
boolean fillValid = !isNullScalar(fillScalar);
77+
6678
if (ctx.dtype() instanceof DType.Bool) {
6779
DType indicesDtype = new DType.Primitive(indicesPtype, false);
6880
Array patchIndices = ctx.decodeChild(0, indicesDtype, numPatches);
6981
Array patchValues = ctx.decodeChild(1, ctx.dtype(), numPatches);
7082
Array idxData = patchIndices instanceof MaskedArray m ? m.inner() : patchIndices;
71-
Array valData = patchValues instanceof MaskedArray m ? m.inner() : patchValues;
72-
return new LazySparseBoolArray(ctx.dtype(), n, false, (BoolArray) valData, idxData, offset);
83+
BoolArray patchValidity = null;
84+
Array valData = patchValues;
85+
if (patchValues instanceof MaskedArray m) {
86+
valData = m.inner();
87+
patchValidity = m.validity();
88+
}
89+
boolean fillValue = Boolean.TRUE.equals(fillScalar.bool_value());
90+
Array result = new LazySparseBoolArray(ctx.dtype(), n, fillValue, (BoolArray) valData, idxData, offset);
91+
return withSparseValidity(ctx, result, fillValid, patchValidity, idxData, numPatches, n, offset);
7392
}
7493

7594
if (!(ctx.dtype() instanceof DType.Primitive)) {
7695
throw new VortexException(EncodingId.VORTEX_SPARSE, "expected primitive dtype, got " + ctx.dtype());
7796
}
7897
PType valuePtype = ((DType.Primitive) ctx.dtype()).ptype();
79-
80-
MemorySegment fillBuf = ctx.buffer(0);
81-
ProtoScalarValue fillScalar;
82-
try {
83-
fillScalar = ProtoScalarValue.decode(fillBuf, 0, fillBuf.byteSize());
84-
} catch (IOException e) {
85-
throw new VortexException(EncodingId.VORTEX_SPARSE, "invalid fill value", e);
86-
}
8798
long fillBits = scalarToLong(fillScalar);
8899

89100
// Lazy path: keep fill bits + decoded patches; no n-sized buffer allocated.
@@ -93,9 +104,14 @@ public Array decode(DecodeContext ctx) {
93104
Array patchIndices = ctx.decodeChild(0, indicesDtype, numPatches);
94105
Array patchValues = ctx.decodeChild(1, ctx.dtype(), numPatches);
95106
Array idxData = patchIndices instanceof MaskedArray m ? m.inner() : patchIndices;
96-
Array valData = patchValues instanceof MaskedArray m ? m.inner() : patchValues;
107+
BoolArray patchValidity = null;
108+
Array valData = patchValues;
109+
if (patchValues instanceof MaskedArray m) {
110+
valData = m.inner();
111+
patchValidity = m.validity();
112+
}
97113

98-
return switch (valuePtype) {
114+
Array result = switch (valuePtype) {
99115
case I64, U64 -> new LazySparseLongArray(ctx.dtype(), n, fillBits,
100116
(LongArray) valData, idxData, offset);
101117
case I32, U32 -> new LazySparseIntArray(ctx.dtype(), n, (int) fillBits,
@@ -114,6 +130,59 @@ public Array decode(DecodeContext ctx) {
114130
(ByteArray) valData, idxData, offset);
115131
default -> throw new VortexException(EncodingId.VORTEX_SPARSE, "unsupported ptype " + valuePtype);
116132
};
133+
return withSparseValidity(ctx, result, fillValid, patchValidity, idxData, numPatches, n, offset);
134+
}
135+
136+
/// Wraps `result` in a [MaskedArray] whose per-row validity is a sparse bool array:
137+
/// fill = `fillValid` (the fill scalar is non-null), each patch bit = that patch's
138+
/// validity. Returns `result` unchanged only when the fill is non-null and no patch
139+
/// carried a null (the all-valid no-regression path).
140+
///
141+
/// @param ctx decode context (allocation arena)
142+
/// @param result the decoded sparse value array
143+
/// @param fillValid `true` when the fill scalar is non-null
144+
/// @param patchValidity per-patch validity bits, or `null` when all patches are valid
145+
/// @param idxData sorted absolute patch positions (raw, mask-unwrapped)
146+
/// @param numPatches number of patches
147+
/// @param n logical row count
148+
/// @param offset starting absolute position
149+
/// @return `result`, or a [MaskedArray] carrying the lazy per-row validity
150+
private static Array withSparseValidity(DecodeContext ctx, Array result, boolean fillValid,
151+
BoolArray patchValidity, Array idxData, long numPatches, long n, long offset) {
152+
if (fillValid && patchValidity == null) {
153+
return result;
154+
}
155+
BoolArray patchBits = patchValidity != null ? patchValidity : allValid(ctx, numPatches);
156+
BoolArray rowValidity = new LazySparseBoolArray(DType.BOOL, n, fillValid, patchBits, idxData, offset);
157+
return new MaskedArray(result, rowValidity);
158+
}
159+
160+
/// Builds an all-valid bit-packed [BoolArray] of `len` bits — the patch-validity stand-in
161+
/// when the patch values are non-nullable but the fill is null (every patch punches in a
162+
/// valid position over an all-invalid base).
163+
private static BoolArray allValid(DecodeContext ctx, long len) {
164+
MemorySegment bits = ctx.arena().allocate(Math.max(1, (len + 7) >>> 3));
165+
bits.fill((byte) 0xFF);
166+
return new MaterializedBoolArray(DType.BOOL, len, bits.asReadOnly());
167+
}
168+
169+
private static ProtoScalarValue decodeFill(MemorySegment fillBuf) {
170+
try {
171+
return ProtoScalarValue.decode(fillBuf, 0, fillBuf.byteSize());
172+
} catch (IOException e) {
173+
throw new VortexException(EncodingId.VORTEX_SPARSE, "invalid fill value", e);
174+
}
175+
}
176+
177+
/// Detects a null fill scalar: either an explicit `null_value`, or a scalar with no
178+
/// value-bearing field set (Rust encodes a null fill as `ScalarValue::Null`, and a
179+
/// non-null fill always sets exactly one typed field — even integer/float `0`).
180+
private static boolean isNullScalar(ProtoScalarValue s) {
181+
return s.null_value() != null
182+
|| (s.bool_value() == null && s.int64_value() == null && s.uint64_value() == null
183+
&& s.f32_value() == null && s.f64_value() == null && s.string_value() == null
184+
&& s.bytes_value() == null && s.f16_value() == null && s.list_value() == null
185+
&& s.variant_value() == null);
117186
}
118187

119188
private static Array decodeVarBin(

0 commit comments

Comments
 (0)