diff --git a/CHANGELOG.md b/CHANGELOG.md index 36695cff..e10d33ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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)) - `vortex.sparse` over utf8/binary values now carries the same row validity as the primitive path: a `fill_value: null` string column nulls every unpatched position instead of rendering an empty string, and a nullable patch value nulls its own position instead of surfacing its raw bytes. The Rust `ValidityVTable` is generic over the values encoding, so VarBin reuses the identical sparse-bool validity — completing the #226 fix for string/binary columns. ([#232](https://github.com/dfa1/vortex-java/issues/232)) +- `vortex.datetimeparts` propagates null component rows instead of throwing `DateTimeParts: null cell at index`: when any part (days/seconds/subseconds) decodes to a null — as a nullable `vortex.runend` child now does after #225 — the reassembled timestamp row is null, unblocking scans of bi-yalelanguages and bi-euro2016. ([#235](https://github.com/dfa1/vortex-java/issues/235)) ### Added diff --git a/integration/src/test/resources/raincloud/expected-status.csv b/integration/src/test/resources/raincloud/expected-status.csv index a16ead86..ff628924 100644 --- a/integration/src/test/resources/raincloud/expected-status.csv +++ b/integration/src/test/resources/raincloud/expected-status.csv @@ -29,7 +29,7 @@ bi-cmsprovider,untriaged bi-commongovernment,untriaged bi-corporations,ok bi-eixo,untriaged -bi-euro2016,gap:235 +bi-euro2016,gap:234 bi-food,ok bi-generico,untriaged bi-hashtags,untriaged @@ -67,7 +67,7 @@ bi-trainsuk2,untriaged bi-uberlandia,untriaged bi-uscensus,untriaged bi-wins,untriaged -bi-yalelanguages,gap:235 +bi-yalelanguages,ok bigscience-p3-eval-subset,untriaged building-permit-applications-data,untriaged c4-en-validation,untriaged diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/array/DateTimePartsArrays.java b/reader/src/main/java/io/github/dfa1/vortex/reader/array/DateTimePartsArrays.java index b1ffb9ab..e24fca8d 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/array/DateTimePartsArrays.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/array/DateTimePartsArrays.java @@ -13,26 +13,22 @@ final class DateTimePartsArrays { private DateTimePartsArrays() { } - /// Reads `arr[i]` as a signed long. Recurses through [MaskedArray]; - /// throws on null cells so callers don't silently get garbage for nullable - /// columns. + /// Reads `arr[i]` as a signed long. Recurses through [MaskedArray] to its + /// raw payload without inspecting validity: the decoder intersects component + /// validities into the reassembled array's own mask (#235), so a null row's + /// filler value here is harmless — callers gate on that outer mask. /// /// @param arr source typed Array /// @param i row index /// @return cell value as long - /// @throws VortexException for null cells or unsupported array types + /// @throws VortexException for unsupported array types static long readLong(Array arr, long i) { return switch (arr) { case ByteArray a -> a.getByte(i); case ShortArray a -> a.getShort(i); case IntArray a -> a.getInt(i); case LongArray a -> a.getLong(i); - case MaskedArray a -> { - if (!a.isValid(i)) { - throw new VortexException("DateTimeParts: null cell at index " + i); - } - yield readLong(a.inner(), i); - } + case MaskedArray a -> readLong(a.inner(), i); default -> throw new VortexException( "DateTimeParts: unsupported child array type: " + arr.getClass().getSimpleName()); }; diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoder.java index 9ec7265b..cf93649b 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoder.java @@ -7,10 +7,14 @@ import io.github.dfa1.vortex.core.model.TimeUnit; import io.github.dfa1.vortex.core.proto.ProtoDateTimePartsMetadata; import io.github.dfa1.vortex.reader.array.Array; +import io.github.dfa1.vortex.reader.array.BoolArray; import io.github.dfa1.vortex.reader.array.LazyDateTimePartsLongArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; +import io.github.dfa1.vortex.reader.array.MaterializedBoolArray; import java.io.IOException; import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; /// Read-only decoder for `vortex.datetimeparts`. /// @@ -58,8 +62,59 @@ public Array decode(DecodeContext ctx) { long unitsPerSecond = readUnitsPerSecond(ext); long unitsPerDay = SECONDS_PER_DAY * unitsPerSecond; - return new LazyDateTimePartsLongArray(ctx.dtype(), ctx.rowCount(), + // A row is null when ANY component is null (mirrors the Rust reference — + // every part must be present to reassemble the epoch count). PR #225 made + // nullable RunEnd children surface real nulls as MaskedArray, so unwrap each + // component to its raw values, intersect their validities, and re-wrap the + // reassembled array with the combined mask instead of throwing (#235). + BoolArray validity = null; + if (days instanceof MaskedArray masked) { + validity = intersect(ctx, validity, masked.validity()); + days = masked.inner(); + } + if (seconds instanceof MaskedArray masked) { + validity = intersect(ctx, validity, masked.validity()); + seconds = masked.inner(); + } + if (subseconds instanceof MaskedArray masked) { + validity = intersect(ctx, validity, masked.validity()); + subseconds = masked.inner(); + } + + Array reassembled = new LazyDateTimePartsLongArray(ctx.dtype(), ctx.rowCount(), days, seconds, subseconds, unitsPerDay, unitsPerSecond); + return validity != null ? new MaskedArray(reassembled, validity) : reassembled; + } + + /// Combines a running validity bitmap with an incoming one via logical AND, + /// so that a row stays valid only when every component is valid. + /// + /// A `null` incoming bitmap means the component is entirely valid and + /// leaves `current` unchanged; a `null` `current` adopts `incoming` directly. + /// Two non-null bitmaps are AND-ed into a fresh off-heap bitmap allocated + /// from the decode arena. + /// + /// @param ctx decode context supplying the output arena + /// @param current running combined validity, or `null` if none yet + /// @param incoming a component's validity bitmap, or `null` if all-valid + /// @return the combined validity bitmap, or `null` when both inputs are all-valid + private static BoolArray intersect(DecodeContext ctx, BoolArray current, BoolArray incoming) { + if (incoming == null) { + return current; + } + if (current == null) { + return incoming; + } + long n = current.length(); + MemorySegment bits = ctx.arena().allocate((n + 7) / 8); + for (long i = 0; i < n; i++) { + if (current.getBoolean(i) && incoming.getBoolean(i)) { + long byteIndex = i >>> 3; + byte b = bits.get(ValueLayout.JAVA_BYTE, byteIndex); + bits.set(ValueLayout.JAVA_BYTE, byteIndex, (byte) ((b & 0xff) | (1 << (i & 7)))); + } + } + return new MaterializedBoolArray(DType.BOOL, n, bits.asReadOnly()); } /// Returns `TimeUnit.divisor()` for the extension's declared time unit, or diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/array/DateTimePartsArraysTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/array/DateTimePartsArraysTest.java index 4025f9dc..7a0cf942 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/array/DateTimePartsArraysTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/array/DateTimePartsArraysTest.java @@ -33,14 +33,16 @@ void readLong_recursesThroughValidMaskedCell() { } @Test - void readLong_nullMaskedCell_throws() { - // Given — a masked array whose cell is null + void readLong_nullMaskedCell_readsInnerFiller() { + // Given — a masked array whose cell is null (validity false) MaskedArray masked = new MaskedArray(longs(42L, 43L), bools(true, false)); - // When / Then - assertThatThrownBy(() -> DateTimePartsArrays.readLong(masked, 1)) - .isInstanceOf(VortexException.class) - .hasMessageContaining("null cell"); + // When — readLong ignores validity and returns the inner filler value; the + // decoder tracks null rows via the reassembled array's own mask instead (#235) + long result = DateTimePartsArrays.readLong(masked, 1); + + // Then + assertThat(result).isEqualTo(43L); } @Test diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java index 4f62a654..8585e51d 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java @@ -7,7 +7,10 @@ import io.github.dfa1.vortex.core.model.TimeUnit; import io.github.dfa1.vortex.core.proto.ProtoDateTimePartsMetadata; import io.github.dfa1.vortex.reader.ReadRegistry; +import io.github.dfa1.vortex.reader.array.Array; import io.github.dfa1.vortex.reader.array.LongArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; +import io.github.dfa1.vortex.reader.array.TestArrays; import org.junit.jupiter.api.Test; import java.lang.foreign.Arena; @@ -125,4 +128,46 @@ void decode_nonExtensionDtype_throws() { // When / Then assertThatThrownBy(() -> SUT.decode(c)).hasMessageContaining("expected Extension dtype"); } + + @Test + void decode_nullDaysComponent_propagatesNullRowInsteadOfThrowing() { + // Given a days child that decodes to a MaskedArray with row 0 null (as a nullable + // RunEnd child does after PR #225) — the decoder must intersect that validity into + // the reassembled array's own mask rather than throwing on the null cell (#235) + var registry = TestRegistry.ofDecoders(SUT, new PrimitiveEncodingDecoder(), new MaskedDaysDecoder()); + ArrayNode days = new ArrayNode(MaskedDaysDecoder.ID, null, new ArrayNode[0], new int[0]); + ArrayNode seconds = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{0}); + ArrayNode subseconds = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{1}); + ArrayNode node = new ArrayNode(EncodingId.VORTEX_DATETIMEPARTS, i64Meta(), + new ArrayNode[]{days, seconds, subseconds}, new int[0]); + DecodeContext c = new DecodeContext(node, timestampDType(TimeUnit.Milliseconds, true), 2, + new MemorySegment[]{TestSegments.leLongs(0L, 0L), TestSegments.leLongs(0L, 0L)}, + registry, Arena.ofAuto()); + + // When + Array result = SUT.decode(c); + + // Then — the reassembled array is masked: row 0 is null, row 1 valid and reassembled + assertThat(result).isInstanceOf(MaskedArray.class); + MaskedArray masked = (MaskedArray) result; + assertThat(masked.isValid(0)).isFalse(); + assertThat(masked.isValid(1)).isTrue(); + assertThat(((LongArray) masked.inner()).getLong(1)).isEqualTo(SECONDS_PER_DAY * 1000L); + } + + /// Stub decoder standing in for a nullable RunEnd days child: returns two days, + /// each `1`, with row 0 marked null via the validity bitmap (mirrors PR #225). + private static final class MaskedDaysDecoder implements EncodingDecoder { + static final EncodingId ID = EncodingId.parse("test.masked-days"); + + @Override + public EncodingId encodingId() { + return ID; + } + + @Override + public Array decode(DecodeContext ctx) { + return new MaskedArray(TestArrays.longs(1L, 1L), TestArrays.bools(false, true)); + } + } }