Skip to content

Commit bbc0a8f

Browse files
dfa1claude
andcommitted
fix: DateTimeParts propagates null component rows instead of throwing (#235)
When any component (days/seconds/subseconds) of a vortex.datetimeparts array decodes to a null — as a nullable vortex.runend child now does after PR #225 — the reassembled timestamp row is null. The decoder unwraps each MaskedArray component to its raw values, intersects their validities, and re-wraps the reassembled array with the combined mask, instead of throwing "DateTimeParts: null cell at index". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d813964 commit bbc0a8f

5 files changed

Lines changed: 116 additions & 17 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2424
- `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))
2525
- `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))
2626
- `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<Sparse>` 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))
27+
- `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))
2728

2829
### Added
2930

reader/src/main/java/io/github/dfa1/vortex/reader/array/DateTimePartsArrays.java

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,22 @@ final class DateTimePartsArrays {
1313
private DateTimePartsArrays() {
1414
}
1515

16-
/// Reads `arr[i]` as a signed long. Recurses through [MaskedArray];
17-
/// throws on null cells so callers don't silently get garbage for nullable
18-
/// columns.
16+
/// Reads `arr[i]` as a signed long. Recurses through [MaskedArray] to its
17+
/// raw payload without inspecting validity: the decoder intersects component
18+
/// validities into the reassembled array's own mask (#235), so a null row's
19+
/// filler value here is harmless — callers gate on that outer mask.
1920
///
2021
/// @param arr source typed Array
2122
/// @param i row index
2223
/// @return cell value as long
23-
/// @throws VortexException for null cells or unsupported array types
24+
/// @throws VortexException for unsupported array types
2425
static long readLong(Array arr, long i) {
2526
return switch (arr) {
2627
case ByteArray a -> a.getByte(i);
2728
case ShortArray a -> a.getShort(i);
2829
case IntArray a -> a.getInt(i);
2930
case LongArray a -> a.getLong(i);
30-
case MaskedArray a -> {
31-
if (!a.isValid(i)) {
32-
throw new VortexException("DateTimeParts: null cell at index " + i);
33-
}
34-
yield readLong(a.inner(), i);
35-
}
31+
case MaskedArray a -> readLong(a.inner(), i);
3632
default -> throw new VortexException(
3733
"DateTimeParts: unsupported child array type: " + arr.getClass().getSimpleName());
3834
};

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

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@
77
import io.github.dfa1.vortex.core.model.TimeUnit;
88
import io.github.dfa1.vortex.core.proto.ProtoDateTimePartsMetadata;
99
import io.github.dfa1.vortex.reader.array.Array;
10+
import io.github.dfa1.vortex.reader.array.BoolArray;
1011
import io.github.dfa1.vortex.reader.array.LazyDateTimePartsLongArray;
12+
import io.github.dfa1.vortex.reader.array.MaskedArray;
13+
import io.github.dfa1.vortex.reader.array.MaterializedBoolArray;
1114

1215
import java.io.IOException;
1316
import java.lang.foreign.MemorySegment;
17+
import java.lang.foreign.ValueLayout;
1418

1519
/// Read-only decoder for `vortex.datetimeparts`.
1620
///
@@ -58,8 +62,59 @@ public Array decode(DecodeContext ctx) {
5862
long unitsPerSecond = readUnitsPerSecond(ext);
5963
long unitsPerDay = SECONDS_PER_DAY * unitsPerSecond;
6064

61-
return new LazyDateTimePartsLongArray(ctx.dtype(), ctx.rowCount(),
65+
// A row is null when ANY component is null (mirrors the Rust reference —
66+
// every part must be present to reassemble the epoch count). PR #225 made
67+
// nullable RunEnd children surface real nulls as MaskedArray, so unwrap each
68+
// component to its raw values, intersect their validities, and re-wrap the
69+
// reassembled array with the combined mask instead of throwing (#235).
70+
BoolArray validity = null;
71+
if (days instanceof MaskedArray masked) {
72+
validity = intersect(ctx, validity, masked.validity());
73+
days = masked.inner();
74+
}
75+
if (seconds instanceof MaskedArray masked) {
76+
validity = intersect(ctx, validity, masked.validity());
77+
seconds = masked.inner();
78+
}
79+
if (subseconds instanceof MaskedArray masked) {
80+
validity = intersect(ctx, validity, masked.validity());
81+
subseconds = masked.inner();
82+
}
83+
84+
Array reassembled = new LazyDateTimePartsLongArray(ctx.dtype(), ctx.rowCount(),
6285
days, seconds, subseconds, unitsPerDay, unitsPerSecond);
86+
return validity != null ? new MaskedArray(reassembled, validity) : reassembled;
87+
}
88+
89+
/// Combines a running validity bitmap with an incoming one via logical AND,
90+
/// so that a row stays valid only when every component is valid.
91+
///
92+
/// A `null` incoming bitmap means the component is entirely valid and
93+
/// leaves `current` unchanged; a `null` `current` adopts `incoming` directly.
94+
/// Two non-null bitmaps are AND-ed into a fresh off-heap bitmap allocated
95+
/// from the decode arena.
96+
///
97+
/// @param ctx decode context supplying the output arena
98+
/// @param current running combined validity, or `null` if none yet
99+
/// @param incoming a component's validity bitmap, or `null` if all-valid
100+
/// @return the combined validity bitmap, or `null` when both inputs are all-valid
101+
private static BoolArray intersect(DecodeContext ctx, BoolArray current, BoolArray incoming) {
102+
if (incoming == null) {
103+
return current;
104+
}
105+
if (current == null) {
106+
return incoming;
107+
}
108+
long n = current.length();
109+
MemorySegment bits = ctx.arena().allocate((n + 7) / 8);
110+
for (long i = 0; i < n; i++) {
111+
if (current.getBoolean(i) && incoming.getBoolean(i)) {
112+
long byteIndex = i >>> 3;
113+
byte b = bits.get(ValueLayout.JAVA_BYTE, byteIndex);
114+
bits.set(ValueLayout.JAVA_BYTE, byteIndex, (byte) ((b & 0xff) | (1 << (i & 7))));
115+
}
116+
}
117+
return new MaterializedBoolArray(DType.BOOL, n, bits.asReadOnly());
63118
}
64119

65120
/// Returns `TimeUnit.divisor()` for the extension's declared time unit, or

reader/src/test/java/io/github/dfa1/vortex/reader/array/DateTimePartsArraysTest.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,16 @@ void readLong_recursesThroughValidMaskedCell() {
3333
}
3434

3535
@Test
36-
void readLong_nullMaskedCell_throws() {
37-
// Given — a masked array whose cell is null
36+
void readLong_nullMaskedCell_readsInnerFiller() {
37+
// Given — a masked array whose cell is null (validity false)
3838
MaskedArray masked = new MaskedArray(longs(42L, 43L), bools(true, false));
3939

40-
// When / Then
41-
assertThatThrownBy(() -> DateTimePartsArrays.readLong(masked, 1))
42-
.isInstanceOf(VortexException.class)
43-
.hasMessageContaining("null cell");
40+
// When — readLong ignores validity and returns the inner filler value; the
41+
// decoder tracks null rows via the reassembled array's own mask instead (#235)
42+
long result = DateTimePartsArrays.readLong(masked, 1);
43+
44+
// Then
45+
assertThat(result).isEqualTo(43L);
4446
}
4547

4648
@Test

reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77
import io.github.dfa1.vortex.core.model.TimeUnit;
88
import io.github.dfa1.vortex.core.proto.ProtoDateTimePartsMetadata;
99
import io.github.dfa1.vortex.reader.ReadRegistry;
10+
import io.github.dfa1.vortex.reader.array.Array;
1011
import io.github.dfa1.vortex.reader.array.LongArray;
12+
import io.github.dfa1.vortex.reader.array.MaskedArray;
13+
import io.github.dfa1.vortex.reader.array.TestArrays;
1114
import org.junit.jupiter.api.Test;
1215

1316
import java.lang.foreign.Arena;
@@ -125,4 +128,46 @@ void decode_nonExtensionDtype_throws() {
125128
// When / Then
126129
assertThatThrownBy(() -> SUT.decode(c)).hasMessageContaining("expected Extension dtype");
127130
}
131+
132+
@Test
133+
void decode_nullDaysComponent_propagatesNullRowInsteadOfThrowing() {
134+
// Given a days child that decodes to a MaskedArray with row 0 null (as a nullable
135+
// RunEnd child does after PR #225) — the decoder must intersect that validity into
136+
// the reassembled array's own mask rather than throwing on the null cell (#235)
137+
var registry = TestRegistry.ofDecoders(SUT, new PrimitiveEncodingDecoder(), new MaskedDaysDecoder());
138+
ArrayNode days = new ArrayNode(MaskedDaysDecoder.ID, null, new ArrayNode[0], new int[0]);
139+
ArrayNode seconds = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{0});
140+
ArrayNode subseconds = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{1});
141+
ArrayNode node = new ArrayNode(EncodingId.VORTEX_DATETIMEPARTS, i64Meta(),
142+
new ArrayNode[]{days, seconds, subseconds}, new int[0]);
143+
DecodeContext c = new DecodeContext(node, timestampDType(TimeUnit.Milliseconds, true), 2,
144+
new MemorySegment[]{TestSegments.leLongs(0L, 0L), TestSegments.leLongs(0L, 0L)},
145+
registry, Arena.ofAuto());
146+
147+
// When
148+
Array result = SUT.decode(c);
149+
150+
// Then — the reassembled array is masked: row 0 is null, row 1 valid and reassembled
151+
assertThat(result).isInstanceOf(MaskedArray.class);
152+
MaskedArray masked = (MaskedArray) result;
153+
assertThat(masked.isValid(0)).isFalse();
154+
assertThat(masked.isValid(1)).isTrue();
155+
assertThat(((LongArray) masked.inner()).getLong(1)).isEqualTo(SECONDS_PER_DAY * 1000L);
156+
}
157+
158+
/// Stub decoder standing in for a nullable RunEnd days child: returns two days,
159+
/// each `1`, with row 0 marked null via the validity bitmap (mirrors PR #225).
160+
private static final class MaskedDaysDecoder implements EncodingDecoder {
161+
static final EncodingId ID = EncodingId.parse("test.masked-days");
162+
163+
@Override
164+
public EncodingId encodingId() {
165+
return ID;
166+
}
167+
168+
@Override
169+
public Array decode(DecodeContext ctx) {
170+
return new MaskedArray(TestArrays.longs(1L, 1L), TestArrays.bools(false, true));
171+
}
172+
}
128173
}

0 commit comments

Comments
 (0)