From fcbd36d9dcb353216dfc19cc851d9da20748a1b6 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Tue, 7 Jul 2026 20:37:14 +0200 Subject: [PATCH] fix: propagate validity in RunEnd and Sparse lazy decoders (#225, #226) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`, 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`, 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 --- CHANGELOG.md | 2 + docs/compatibility.md | 9 +- .../resources/raincloud/expected-status.csv | 4 +- .../reader/decode/RunEndEncodingDecoder.java | 53 ++++-- .../reader/decode/SparseEncodingDecoder.java | 93 ++++++++-- .../decode/RunEndEncodingDecoderTest.java | 140 +++++++++++++++ .../decode/SparseEncodingDecoderTest.java | 167 ++++++++++++++++++ .../encode/SparseEncodingEncoderTest.java | 28 +-- 8 files changed, 454 insertions(+), 42 deletions(-) create mode 100644 reader/src/test/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoderTest.java create mode 100644 reader/src/test/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoderTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d7d7d27..653304ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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)) - 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)) - 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)) +- `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`. ([#225](https://github.com/dfa1/vortex-java/issues/225)) +- `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`. ([#226](https://github.com/dfa1/vortex-java/issues/226)) ### Added diff --git a/docs/compatibility.md b/docs/compatibility.md index 18b7e362..795fcd51 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -62,13 +62,12 @@ Per-slug status lives in `integration/src/test/resources/raincloud/expected-stat (`ok` must pass; `gap:` must still fail, so a fix flips the entry in the same change; `untriaged` runs and reports without failing the build). A scheduled workflow (`raincloud-conformance.yml`) hydrates a size-capped subset weekly. Current triage — -38 `ok`, 2 known gaps (both silent null loss in lazy decoders: -[#225](https://github.com/dfa1/vortex-java/issues/225) RunEnd run-values validity, -[#226](https://github.com/dfa1/vortex-java/issues/226) Sparse null fill / nullable patches); -207 slugs untriaged. Every gap found by earlier rounds is fixed +40 `ok`, 0 known gaps; 207 slugs untriaged. Every gap found so far is fixed ([#206](https://github.com/dfa1/vortex-java/issues/206)–[#211](https://github.com/dfa1/vortex-java/issues/211), [#215](https://github.com/dfa1/vortex-java/issues/215)–[#217](https://github.com/dfa1/vortex-java/issues/217), -[#221](https://github.com/dfa1/vortex-java/issues/221)). +[#221](https://github.com/dfa1/vortex-java/issues/221), +[#225](https://github.com/dfa1/vortex-java/issues/225) RunEnd run-values validity, +[#226](https://github.com/dfa1/vortex-java/issues/226) Sparse null fill / nullable patches). ## Encodings diff --git a/integration/src/test/resources/raincloud/expected-status.csv b/integration/src/test/resources/raincloud/expected-status.csv index 00fbd697..4a480d60 100644 --- a/integration/src/test/resources/raincloud/expected-status.csv +++ b/integration/src/test/resources/raincloud/expected-status.csv @@ -219,7 +219,7 @@ uci-individual-household-electric-power-consumption,untriaged uci-iris,ok uci-magic-gamma-telescope,ok uci-mushroom,ok -uci-online-retail,gap:225 +uci-online-retail,ok uci-online-retail-ii,untriaged uci-online-shoppers-purchasing-intention,untriaged uci-optical-recognition-of-handwritten-digits,ok @@ -250,7 +250,7 @@ waxal-dagbani-asr-test,untriaged wdi,untriaged websight-v01,untriaged wikipedia-en,untriaged -world-energy-consumption,gap:226 +world-energy-consumption,ok yellow_tripdata_2025,untriaged youtube-commons-sample,untriaged zoo-animal-classification,untriaged diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoder.java index 5da5798c..45db7821 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoder.java @@ -55,18 +55,33 @@ public Array decode(DecodeContext ctx) { long n = ctx.rowCount(); DType endsDtype = new DType.Primitive(endsPtype, false); Array endsArr = ctx.decodeChild(0, endsDtype, numRuns); + Array endsData = endsArr instanceof MaskedArray m ? m.inner() : endsArr; + + // Values-side validity mirrors the Rust reference `ValidityVTable`: a + // RunEnd array's validity IS a RunEnd over the same ends whose per-run value is + // the run-value's validity bit. Row i's validity is thus the validity of the run + // it falls in — expressed lazily as `LazyRunEndBoolArray`, O(numRuns) not O(n) + // (n can be huge, e.g. uci-online-retail 499712 rows). A nullable values child + // surfaces as a MaskedArray; dropping its mask expanded null runs to a filler + // value (e.g. u16? null rows emitting the FoR base) — #225. + Array valuesArr = ctx.decodeChild(1, ctx.dtype(), numRuns); + BoolArray valuesValidity = null; + Array valuesData = valuesArr; + if (valuesArr instanceof MaskedArray m) { + valuesData = m.inner(); + valuesValidity = m.validity(); + } if (ctx.dtype() instanceof DType.Utf8 || ctx.dtype() instanceof DType.Binary) { - VarBinArray valuesArr = (VarBinArray) ctx.decodeChild(1, ctx.dtype(), numRuns); - MemorySegment endsSeg = ctx.materialize(endsArr); - return expandStrings(endsSeg, VarBinArray.toOffsetMode(valuesArr, ctx.arena()), endsPtype, numRuns, offset, n, ctx.dtype(), ctx.arena()); + MemorySegment endsSeg = ctx.materialize(endsData); + Array result = expandStrings(endsSeg, VarBinArray.toOffsetMode((VarBinArray) valuesData, ctx.arena()), + endsPtype, numRuns, offset, n, ctx.dtype(), ctx.arena()); + return withRunValidity(result, valuesValidity, endsData, n, offset); } if (ctx.dtype() instanceof DType.Bool) { - Array valuesArr = ctx.decodeChild(1, ctx.dtype(), numRuns); - Array valuesData = valuesArr instanceof MaskedArray m ? m.inner() : valuesArr; - Array endsData = endsArr instanceof MaskedArray m ? m.inner() : endsArr; - return new LazyRunEndBoolArray(ctx.dtype(), n, (BoolArray) valuesData, endsData, offset); + Array result = new LazyRunEndBoolArray(ctx.dtype(), n, (BoolArray) valuesData, endsData, offset); + return withRunValidity(result, valuesValidity, endsData, n, offset); } if (!(ctx.dtype() instanceof DType.Primitive p)) { @@ -77,16 +92,32 @@ public Array decode(DecodeContext ctx) { // Lazy path: wrap values + ends without expanding into an n-sized buffer. // VarBin keeps the eager path above — offset rebasing doesn't trivially // express as binary-search-on-read. - Array valuesArr = ctx.decodeChild(1, ctx.dtype(), numRuns); - Array valuesData = valuesArr instanceof MaskedArray m ? m.inner() : valuesArr; - Array endsData = endsArr instanceof MaskedArray m ? m.inner() : endsArr; - return switch (valuePtype) { + Array result = switch (valuePtype) { case I64, U64 -> new LazyRunEndLongArray(ctx.dtype(), n, (LongArray) valuesData, endsData, offset); case I32, U32 -> new LazyRunEndIntArray(ctx.dtype(), n, (IntArray) valuesData, endsData, offset); case I16, U16 -> new LazyRunEndShortArray(ctx.dtype(), n, (ShortArray) valuesData, endsData, offset); case I8, U8 -> new LazyRunEndByteArray(ctx.dtype(), n, (ByteArray) valuesData, endsData, offset); default -> throw new VortexException(EncodingId.VORTEX_RUNEND, "unsupported ptype " + valuePtype); }; + return withRunValidity(result, valuesValidity, endsData, n, offset); + } + + /// Wraps `result` in a [MaskedArray] whose per-row validity is a run-end bool array + /// over the same run-ends: row `i` is valid iff the run it falls in has a valid value. + /// Returns `result` unchanged when the values child carried no validity (all valid). + /// + /// @param result the decoded run-end value array + /// @param valuesValidity per-run validity bits from the values child, or `null` + /// @param endsData the run-ends array (raw, mask-unwrapped) + /// @param n logical row count + /// @param offset starting absolute position + /// @return `result`, or a [MaskedArray] carrying the lazy per-row validity + private static Array withRunValidity(Array result, BoolArray valuesValidity, Array endsData, long n, long offset) { + if (valuesValidity == null) { + return result; + } + BoolArray rowValidity = new LazyRunEndBoolArray(DType.BOOL, n, valuesValidity, endsData, offset); + return new MaskedArray(result, rowValidity); } private static Array expandStrings( diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java index e30ed1c5..5e18e807 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java @@ -23,6 +23,7 @@ import io.github.dfa1.vortex.reader.array.LazySparseShortArray; import io.github.dfa1.vortex.reader.array.LongArray; import io.github.dfa1.vortex.reader.array.MaskedArray; +import io.github.dfa1.vortex.reader.array.MaterializedBoolArray; import io.github.dfa1.vortex.reader.array.ShortArray; import io.github.dfa1.vortex.reader.array.VarBinArray; @@ -63,27 +64,37 @@ public Array decode(DecodeContext ctx) { return decodeVarBin(ctx, n, numPatches, offset, indicesPtype); } + // Row validity mirrors the Rust reference `ValidityVTable`: it is a sparse + // bool array 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). Dropping either + // facet lost nulls: a `fill_value: null` array (world-energy `biofuel_cons_change_pct` + // f64?) decoded unpatched rows as 0.0, and a null patch (nuclear_share_energy) + // decoded to raw 0 — #226. + MemorySegment fillBuf = ctx.buffer(0); + ProtoScalarValue fillScalar = decodeFill(fillBuf); + boolean fillValid = !isNullScalar(fillScalar); + if (ctx.dtype() instanceof DType.Bool) { DType indicesDtype = new DType.Primitive(indicesPtype, false); Array patchIndices = ctx.decodeChild(0, indicesDtype, numPatches); Array patchValues = ctx.decodeChild(1, ctx.dtype(), numPatches); Array idxData = patchIndices instanceof MaskedArray m ? m.inner() : patchIndices; - Array valData = patchValues instanceof MaskedArray m ? m.inner() : patchValues; - return new LazySparseBoolArray(ctx.dtype(), n, false, (BoolArray) valData, idxData, offset); + BoolArray patchValidity = null; + Array valData = patchValues; + if (patchValues instanceof MaskedArray m) { + valData = m.inner(); + patchValidity = m.validity(); + } + boolean fillValue = Boolean.TRUE.equals(fillScalar.bool_value()); + Array result = new LazySparseBoolArray(ctx.dtype(), n, fillValue, (BoolArray) valData, idxData, offset); + return withSparseValidity(ctx, result, fillValid, patchValidity, idxData, numPatches, n, offset); } if (!(ctx.dtype() instanceof DType.Primitive)) { throw new VortexException(EncodingId.VORTEX_SPARSE, "expected primitive dtype, got " + ctx.dtype()); } PType valuePtype = ((DType.Primitive) ctx.dtype()).ptype(); - - MemorySegment fillBuf = ctx.buffer(0); - ProtoScalarValue fillScalar; - try { - fillScalar = ProtoScalarValue.decode(fillBuf, 0, fillBuf.byteSize()); - } catch (IOException e) { - throw new VortexException(EncodingId.VORTEX_SPARSE, "invalid fill value", e); - } long fillBits = scalarToLong(fillScalar); // Lazy path: keep fill bits + decoded patches; no n-sized buffer allocated. @@ -93,9 +104,14 @@ public Array decode(DecodeContext ctx) { Array patchIndices = ctx.decodeChild(0, indicesDtype, numPatches); Array patchValues = ctx.decodeChild(1, ctx.dtype(), numPatches); Array idxData = patchIndices instanceof MaskedArray m ? m.inner() : patchIndices; - Array valData = patchValues instanceof MaskedArray m ? m.inner() : patchValues; + BoolArray patchValidity = null; + Array valData = patchValues; + if (patchValues instanceof MaskedArray m) { + valData = m.inner(); + patchValidity = m.validity(); + } - return switch (valuePtype) { + Array result = switch (valuePtype) { case I64, U64 -> new LazySparseLongArray(ctx.dtype(), n, fillBits, (LongArray) valData, idxData, offset); case I32, U32 -> new LazySparseIntArray(ctx.dtype(), n, (int) fillBits, @@ -114,6 +130,59 @@ public Array decode(DecodeContext ctx) { (ByteArray) valData, idxData, offset); default -> throw new VortexException(EncodingId.VORTEX_SPARSE, "unsupported ptype " + valuePtype); }; + return withSparseValidity(ctx, result, fillValid, patchValidity, idxData, numPatches, n, offset); + } + + /// Wraps `result` in a [MaskedArray] whose per-row validity is a sparse bool array: + /// fill = `fillValid` (the fill scalar is non-null), each patch bit = that patch's + /// validity. Returns `result` unchanged only when the fill is non-null and no patch + /// carried a null (the all-valid no-regression path). + /// + /// @param ctx decode context (allocation arena) + /// @param result the decoded sparse value array + /// @param fillValid `true` when the fill scalar is non-null + /// @param patchValidity per-patch validity bits, or `null` when all patches are valid + /// @param idxData sorted absolute patch positions (raw, mask-unwrapped) + /// @param numPatches number of patches + /// @param n logical row count + /// @param offset starting absolute position + /// @return `result`, or a [MaskedArray] carrying the lazy per-row validity + private static Array withSparseValidity(DecodeContext ctx, Array result, boolean fillValid, + BoolArray patchValidity, Array idxData, long numPatches, long n, long offset) { + if (fillValid && patchValidity == null) { + return result; + } + BoolArray patchBits = patchValidity != null ? patchValidity : allValid(ctx, numPatches); + BoolArray rowValidity = new LazySparseBoolArray(DType.BOOL, n, fillValid, patchBits, idxData, offset); + return new MaskedArray(result, rowValidity); + } + + /// Builds an all-valid bit-packed [BoolArray] of `len` bits — the patch-validity stand-in + /// when the patch values are non-nullable but the fill is null (every patch punches in a + /// valid position over an all-invalid base). + private static BoolArray allValid(DecodeContext ctx, long len) { + MemorySegment bits = ctx.arena().allocate(Math.max(1, (len + 7) >>> 3)); + bits.fill((byte) 0xFF); + return new MaterializedBoolArray(DType.BOOL, len, bits.asReadOnly()); + } + + private static ProtoScalarValue decodeFill(MemorySegment fillBuf) { + try { + return ProtoScalarValue.decode(fillBuf, 0, fillBuf.byteSize()); + } catch (IOException e) { + throw new VortexException(EncodingId.VORTEX_SPARSE, "invalid fill value", e); + } + } + + /// Detects a null fill scalar: either an explicit `null_value`, or a scalar with no + /// value-bearing field set (Rust encodes a null fill as `ScalarValue::Null`, and a + /// non-null fill always sets exactly one typed field — even integer/float `0`). + private static boolean isNullScalar(ProtoScalarValue s) { + return s.null_value() != null + || (s.bool_value() == null && s.int64_value() == null && s.uint64_value() == null + && s.f32_value() == null && s.f64_value() == null && s.string_value() == null + && s.bytes_value() == null && s.f16_value() == null && s.list_value() == null + && s.variant_value() == null); } private static Array decodeVarBin( diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoderTest.java new file mode 100644 index 00000000..0e9e603e --- /dev/null +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/RunEndEncodingDecoderTest.java @@ -0,0 +1,140 @@ +package io.github.dfa1.vortex.reader.decode; + +import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.core.model.EncodingId; +import io.github.dfa1.vortex.core.model.PType; +import io.github.dfa1.vortex.core.proto.ProtoPType; +import io.github.dfa1.vortex.core.proto.ProtoRunEndMetadata; +import io.github.dfa1.vortex.encoding.TestSegments; +import io.github.dfa1.vortex.reader.ReadRegistry; +import io.github.dfa1.vortex.reader.array.Array; +import io.github.dfa1.vortex.reader.array.IntArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; +import org.junit.jupiter.api.Test; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; + +import static org.assertj.core.api.Assertions.assertThat; + +class RunEndEncodingDecoderTest { + + private static final RunEndEncodingDecoder SUT = new RunEndEncodingDecoder(); + private static final ReadRegistry REGISTRY = TestRegistry.ofDecoders( + SUT, new PrimitiveEncodingDecoder(), new BoolEncodingDecoder()); + + @Test + void encodingId_isVortexRunEnd() { + // Given / When / Then + assertThat(SUT.encodingId()).isEqualTo(EncodingId.VORTEX_RUNEND); + } + + /// A nullable run-value must null every ROW in its run, not expand to a filler value. + /// Mirrors uci-online-retail `customerid` (u16?): a null run previously emitted the FoR + /// base (#225). Row validity is the run-end bool over the same ends (Rust + /// `ValidityVTable`), so valid runs stay correct while the null run's rows go null. + @Test + void nullRun_nullsAllRowsInRunAndKeepsValidRunsCorrect() { + // Given — ends [2,3,5] over values [10,20,30] with run 1 (value 20) null. + // Rows 0..1 -> 10 (valid), row 2 -> 20 (null run), rows 3..4 -> 30 (valid). + MemorySegment[] segs = { + u8Bytes(2, 3, 5), // run ends + TestSegments.leInts(10, 20, 30), // run values + boolBitmap(true, false, true) // run-value validity: run 1 null + }; + ArrayNode endsNode = primitiveNode(0); + ArrayNode valuesNode = maskedPrimitiveNode(1, 2); + + // When + Array result = decode(nullableI32(), PType.U8, 3, 0, 5, segs, endsNode, valuesNode); + + // Then + MaskedArray masked = assertMasked(result); + assertValidity(masked, true, true, false, true, true); + assertIntValues(masked, 10, 10, 20, 30, 30); + } + + /// No-regression: a non-nullable values child must NOT produce a [MaskedArray]. + @Test + void nonNullableValues_returnsPlainArray() { + // Given — ends [2,4] over values [10,20], no validity child + MemorySegment[] segs = { + u8Bytes(2, 4), + TestSegments.leInts(10, 20) + }; + ArrayNode endsNode = primitiveNode(0); + ArrayNode valuesNode = primitiveNode(1); + + // When + Array result = decode(DType.I32, PType.U8, 2, 0, 4, segs, endsNode, valuesNode); + + // Then + assertThat(result).isInstanceOf(IntArray.class).isNotInstanceOf(MaskedArray.class); + IntArray ints = (IntArray) result; + assertThat(ints.getInt(0)).isEqualTo(10); + assertThat(ints.getInt(3)).isEqualTo(20); + } + + private static Array decode(DType dtype, PType endsPtype, long numRuns, long offset, long n, + MemorySegment[] segs, ArrayNode endsNode, ArrayNode valuesNode) { + MemorySegment meta = MemorySegment.ofArray( + new ProtoRunEndMetadata(ProtoPType.valueOf(endsPtype.name()), numRuns, offset).encode()); + ArrayNode node = new ArrayNode(EncodingId.VORTEX_RUNEND, meta, + new ArrayNode[]{endsNode, valuesNode}, new int[]{}); + DecodeContext ctx = new DecodeContext(node, dtype, n, segs, REGISTRY, Arena.ofAuto()); + return SUT.decode(ctx); + } + + private static DType nullableI32() { + return new DType.Primitive(PType.I32, true); + } + + private static MaskedArray assertMasked(Array result) { + assertThat(result).isInstanceOf(MaskedArray.class); + return (MaskedArray) result; + } + + private static void assertValidity(MaskedArray masked, boolean... expected) { + for (int i = 0; i < expected.length; i++) { + assertThat(masked.isValid(i)).as("valid row %d", i).isEqualTo(expected[i]); + } + } + + private static void assertIntValues(MaskedArray masked, int... expected) { + IntArray inner = (IntArray) masked.inner(); + for (int i = 0; i < expected.length; i++) { + assertThat(inner.getInt(i)).as("row %d", i).isEqualTo(expected[i]); + } + } + + private static ArrayNode primitiveNode(int bufferIndex) { + return new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{bufferIndex}); + } + + /// A primitive node whose single `vortex.bool` validity child makes + /// [PrimitiveEncodingDecoder] surface it as a [MaskedArray]. + private static ArrayNode maskedPrimitiveNode(int dataBufIndex, int validityBufIndex) { + ArrayNode validity = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[0], + new int[]{validityBufIndex}); + return new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[]{validity}, + new int[]{dataBufIndex}); + } + + private static MemorySegment u8Bytes(int... values) { + byte[] a = new byte[values.length]; + for (int i = 0; i < values.length; i++) { + a[i] = (byte) values[i]; + } + return MemorySegment.ofArray(a); + } + + private static MemorySegment boolBitmap(boolean... valid) { + byte[] bytes = new byte[(valid.length + 7) / 8]; + for (int i = 0; i < valid.length; i++) { + if (valid[i]) { + bytes[i >>> 3] |= (byte) (1 << (i & 7)); + } + } + return MemorySegment.ofArray(bytes); + } +} diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoderTest.java new file mode 100644 index 00000000..a4183b9e --- /dev/null +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoderTest.java @@ -0,0 +1,167 @@ +package io.github.dfa1.vortex.reader.decode; + +import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.core.model.EncodingId; +import io.github.dfa1.vortex.core.model.PType; +import io.github.dfa1.vortex.core.proto.ProtoNullValue; +import io.github.dfa1.vortex.core.proto.ProtoPType; +import io.github.dfa1.vortex.core.proto.ProtoPatchesMetadata; +import io.github.dfa1.vortex.core.proto.ProtoScalarValue; +import io.github.dfa1.vortex.core.proto.ProtoSparseMetadata; +import io.github.dfa1.vortex.encoding.TestSegments; +import io.github.dfa1.vortex.reader.ReadRegistry; +import io.github.dfa1.vortex.reader.array.Array; +import io.github.dfa1.vortex.reader.array.DoubleArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; +import org.junit.jupiter.api.Test; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.within; + +class SparseEncodingDecoderTest { + + private static final SparseEncodingDecoder SUT = new SparseEncodingDecoder(); + private static final ReadRegistry REGISTRY = TestRegistry.ofDecoders( + SUT, new PrimitiveEncodingDecoder(), new BoolEncodingDecoder()); + + @Test + void encodingId_isVortexSparse() { + // Given / When / Then + assertThat(SUT.encodingId()).isEqualTo(EncodingId.VORTEX_SPARSE); + } + + /// A `fill_value: null` sparse array must null every UNPATCHED position, not decode it as + /// the fill's zero bits. Mirrors world-energy-consumption `biofuel_cons_change_pct` (f64?): + /// unpatched rows previously decoded as 0.0 (#226). Row validity is a sparse bool whose fill + /// is `fill.is_valid()` (Rust `ValidityVTable`). + @Test + void nullFill_nullsUnpatchedPositions_keepsPatchesValid() { + // Given — null fill; patches at positions 1 and 3 with values 5.0 and 7.0 over 5 rows. + MemorySegment[] segs = { + nullFill(), // fill_value: null + TestSegments.leInts(1, 3), // patch indices (U32) + TestSegments.leDoubles(5.0, 7.0) // patch values (all valid) + }; + ArrayNode idxNode = primitiveNode(1); + ArrayNode valNode = primitiveNode(2); + + // When + Array result = decode(nullableF64(), 2, 0, PType.U32, 5, segs, idxNode, valNode); + + // Then — only the patched positions are valid + MaskedArray masked = assertMasked(result); + assertValidity(masked, false, true, false, true, false); + DoubleArray inner = (DoubleArray) masked.inner(); + assertThat(inner.getDouble(1)).isCloseTo(5.0, within(1e-9)); + assertThat(inner.getDouble(3)).isCloseTo(7.0, within(1e-9)); + } + + /// A non-null fill with a null PATCH value must null only that patch's position; other + /// patches and all fill positions stay valid. Mirrors nuclear_share_energy: a patched-but-null + /// slot previously decoded to raw 0 (#226). + @Test + void nonNullFill_withNullPatch_nullsOnlyThatPatch() { + // Given — fill 0.0 (valid); patches at 1 and 3 with the patch at position 3 null. + MemorySegment[] segs = { + f64Fill(0.0), // valid fill + TestSegments.leInts(1, 3), // patch indices + TestSegments.leDoubles(5.0, 7.0), // patch values + boolBitmap(true, false) // patch validity: patch 1 (pos 3) null + }; + ArrayNode idxNode = primitiveNode(1); + ArrayNode valNode = maskedPrimitiveNode(2, 3); + + // When + Array result = decode(nullableF64(), 2, 0, PType.U32, 5, segs, idxNode, valNode); + + // Then — fill positions valid, patch at pos 1 valid, patch at pos 3 null + MaskedArray masked = assertMasked(result); + assertValidity(masked, true, true, true, false, true); + DoubleArray inner = (DoubleArray) masked.inner(); + assertThat(inner.getDouble(0)).isCloseTo(0.0, within(1e-9)); + assertThat(inner.getDouble(1)).isCloseTo(5.0, within(1e-9)); + assertThat(inner.getDouble(2)).isCloseTo(0.0, within(1e-9)); + } + + /// No-regression: a non-null fill with non-nullable patches must NOT produce a [MaskedArray]. + @Test + void nonNullFill_validPatches_returnsPlainArray() { + // Given — fill 0.0 (valid), patches all valid + MemorySegment[] segs = { + f64Fill(0.0), + TestSegments.leInts(1, 3), + TestSegments.leDoubles(5.0, 7.0) + }; + ArrayNode idxNode = primitiveNode(1); + ArrayNode valNode = primitiveNode(2); + + // When + Array result = decode(DType.F64, 2, 0, PType.U32, 5, segs, idxNode, valNode); + + // Then + assertThat(result).isInstanceOf(DoubleArray.class).isNotInstanceOf(MaskedArray.class); + DoubleArray inner = (DoubleArray) result; + assertThat(inner.getDouble(0)).isCloseTo(0.0, within(1e-9)); + assertThat(inner.getDouble(1)).isCloseTo(5.0, within(1e-9)); + } + + private static Array decode(DType dtype, long numPatches, long offset, PType indicesPtype, long n, + MemorySegment[] segs, ArrayNode idxNode, ArrayNode valNode) { + ProtoPatchesMetadata patches = new ProtoPatchesMetadata( + numPatches, offset, ProtoPType.valueOf(indicesPtype.name()), null, null, null); + MemorySegment meta = MemorySegment.ofArray(new ProtoSparseMetadata(patches).encode()); + ArrayNode node = new ArrayNode(EncodingId.VORTEX_SPARSE, meta, + new ArrayNode[]{idxNode, valNode}, new int[]{0}); + DecodeContext ctx = new DecodeContext(node, dtype, n, segs, REGISTRY, Arena.ofAuto()); + return SUT.decode(ctx); + } + + private static DType nullableF64() { + return new DType.Primitive(PType.F64, true); + } + + private static MemorySegment nullFill() { + return MemorySegment.ofArray(ProtoScalarValue.ofNullValue(ProtoNullValue.NULL_VALUE).encode()); + } + + private static MemorySegment f64Fill(double v) { + return MemorySegment.ofArray(ProtoScalarValue.ofF64Value(v).encode()); + } + + private static MaskedArray assertMasked(Array result) { + assertThat(result).isInstanceOf(MaskedArray.class); + return (MaskedArray) result; + } + + private static void assertValidity(MaskedArray masked, boolean... expected) { + for (int i = 0; i < expected.length; i++) { + assertThat(masked.isValid(i)).as("valid row %d", i).isEqualTo(expected[i]); + } + } + + private static ArrayNode primitiveNode(int bufferIndex) { + return new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{bufferIndex}); + } + + /// A primitive node whose single `vortex.bool` validity child makes + /// [PrimitiveEncodingDecoder] surface it as a [MaskedArray]. + private static ArrayNode maskedPrimitiveNode(int dataBufIndex, int validityBufIndex) { + ArrayNode validity = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[0], + new int[]{validityBufIndex}); + return new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[]{validity}, + new int[]{dataBufIndex}); + } + + private static MemorySegment boolBitmap(boolean... valid) { + byte[] bytes = new byte[(valid.length + 7) / 8]; + for (int i = 0; i < valid.length; i++) { + if (valid[i]) { + bytes[i >>> 3] |= (byte) (1 << (i & 7)); + } + } + return MemorySegment.ofArray(bytes); + } +} diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/SparseEncodingEncoderTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/SparseEncodingEncoderTest.java index 13837dda..e6d0d040 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/SparseEncodingEncoderTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/SparseEncodingEncoderTest.java @@ -6,6 +6,7 @@ import io.github.dfa1.vortex.reader.array.BoolArray; import io.github.dfa1.vortex.reader.array.DoubleArray; import io.github.dfa1.vortex.reader.array.LongArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; import io.github.dfa1.vortex.reader.array.VarBinArray; import io.github.dfa1.vortex.reader.decode.ArrayNode; import io.github.dfa1.vortex.encoding.DTypes; @@ -301,8 +302,10 @@ void decode_offsetSubtracted() { } @Test - void decode_nullValueFill_treatedAsZero() { - // Given + void decode_nullValueFill_nullsAllUnpatchedRows() { + // Given — a null fill with no patches: every position is unpatched, so every row + // is null (Rust `ValidityVTable`: fill validity is fill.is_valid()). Before + // #226 the null fill's zero bits leaked through as literal 0 values. byte[] nullFill = ProtoScalarValue.ofNullValue(ProtoNullValue.NULL_VALUE).encode(); byte[] meta = buildSparseMetaBytes(0, 0L, PType.U32); DecodeContext ctx = buildCtx(DTypes.I64, 4, nullFill, meta, new byte[0], new byte[0]); @@ -310,10 +313,10 @@ void decode_nullValueFill_treatedAsZero() { // When Array result = DECODER.decode(ctx); - // Then - LongArray la = (LongArray) result; + // Then — all four rows are null + MaskedArray masked = (MaskedArray) result; for (int i = 0; i < 4; i++) { - assertThat(la.getLong(i)).as("index %d", i).isZero(); + assertThat(masked.isValid(i)).as("valid row %d", i).isFalse(); } } @@ -411,15 +414,16 @@ void decode_bool_withPatches_setsBitsAtIndices() { // When Array result = DECODER.decode(ctx); - // Then - BoolArray boolArr = (BoolArray) result; - assertThat(boolArr.length()).isEqualTo(6L); - assertThat(boolArr.getBoolean(0)).isFalse(); - assertThat(boolArr.getBoolean(1)).isFalse(); + // Then — patched bits sit at 2 and 5; the null fill nulls every unpatched row (#226), + // so only positions 2 and 5 are valid and both carry `true`. + MaskedArray masked = (MaskedArray) result; + assertThat(masked.length()).isEqualTo(6L); + BoolArray boolArr = (BoolArray) masked.inner(); assertThat(boolArr.getBoolean(2)).isTrue(); - assertThat(boolArr.getBoolean(3)).isFalse(); - assertThat(boolArr.getBoolean(4)).isFalse(); assertThat(boolArr.getBoolean(5)).isTrue(); + for (int i = 0; i < 6; i++) { + assertThat(masked.isValid(i)).as("valid row %d", i).isEqualTo(i == 2 || i == 5); + } } } }