Skip to content

Commit 39c1d08

Browse files
dfa1claude
andcommitted
fix: nullable vortex.bool columns decoded as false instead of null
BoolEncodingDecoder ignored validity children entirely. When a vortex.bool node has one child, that child is a bit-packed validity bitmask (false = null row) — the same pattern used by PrimitiveEncodingDecoder. Null rows were silently decoded as 'false', causing silent corruption in any corpus file with a nullable boolean column (e.g. bi-eixo st_extemporaneo_matricula). Also refactors RaincloudConformanceIntegrationTest to stream oracle and vortex export in parallel via virtual threads + PipedWriter/PipedReader, comparing line-by-line without temp files. This eliminates the OOM for large slugs (bi-bimbo, 74 M rows) and roughly halves wall time by overlapping I/O. Conformance matrix: bi-bimbo, bi-eixo, bi-medicare3, bi-medpayment1 → ok. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 8401ca3 commit 39c1d08

5 files changed

Lines changed: 249 additions & 32 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ input (`ConstantEncodingDecoder`, `AlpRdEncodingDecoder`, `SparseEncodingDecoder
3737
- `DateTimePartsArrays.readLong` now zero-extends unsigned children (`U8`/`U16`/`U32`) instead of sign-extending them; previously a `U16` seconds-within-day value ≥ 32 768 was widened as a negative short, producing a 65 536-second error in the reconstructed timestamp (#252, bi-euro2016 corpus).
3838
- `RaincloudConformanceIntegrationTest` now writes both the vortex-java and Parquet oracle CSVs to temp files and streams them line-by-line for comparison; previously it materialized both into heap `String` objects, causing OOM for large corpus files (≥157 MB) that were previously hidden by throwing before reaching the corrupted line. ([#254](https://github.com/dfa1/vortex-java/issues/254))
3939
- `RaincloudConformanceIntegrationTest` oracle now formats `INT32`/`INT64` Parquet columns with a `UINT_32`/`UINT_64` logical-type annotation using `Integer.toUnsignedString`/`Long.toUnsignedString`, matching the unsigned representation that Vortex stores as `U32`/`U64`; previously the Parquet signed interpretation diverged for values above `Integer.MAX_VALUE` (e.g. `total_boosters` in the covid-world-vaccination-progress corpus). ([#253](https://github.com/dfa1/vortex-java/issues/253))
40+
- `RaincloudConformanceIntegrationTest` now runs the Parquet oracle and vortex-java export in parallel via virtual threads connected by `PipedWriter`/`PipedReader`, comparing line-by-line without temp files; previously both CSV streams were written to disk and then compared, causing OOM for large corpus slugs (e.g. bi-bimbo with 74 M rows) and doubling wall time. (internal)
41+
- `BoolEncodingDecoder` now handles nullable `vortex.bool` columns: when the encoding node has one child, it is decoded as a validity bitmask and the values array is wrapped in a `MaskedArray`; previously the validity child was silently ignored and null rows were decoded as `false`, causing silent corruption in any corpus file with a nullable boolean column (e.g. bi-eixo `st_extemporaneo_matricula`). (internal)
4042

4143
## [Unreleased]
4244

integration/src/test/java/io/github/dfa1/vortex/integration/RaincloudConformanceIntegrationTest.java

Lines changed: 98 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,13 @@
1717

1818
import java.io.BufferedReader;
1919
import java.io.IOException;
20+
import java.io.PipedReader;
21+
import java.io.PipedWriter;
2022
import java.io.UncheckedIOException;
2123
import java.io.Writer;
2224
import java.nio.file.Files;
2325
import java.nio.file.Path;
26+
import java.util.concurrent.atomic.AtomicReference;
2427
import java.util.ArrayList;
2528
import java.util.HashMap;
2629
import java.util.LinkedHashMap;
@@ -86,20 +89,81 @@ Stream<DynamicTest> conformancePerSlug() throws IOException {
8689
}
8790

8891
private static void assertMatchesParquetOracle(Path vortex, Path parquet) throws IOException {
89-
// Given — write both CSVs to temp files; never hold multi-GB strings in heap
90-
Path oracleCsv = Files.createTempFile("oracle-", ".csv");
91-
Path vortexCsv = Files.createTempFile("vortex-", ".csv");
92-
try {
93-
writeOracle(parquet, oracleCsv);
92+
// Given — both producers run in parallel via virtual threads; compare line-by-line
93+
// without temp files, cutting wall time roughly in half for large slugs.
94+
AtomicReference<Throwable> oracleError = new AtomicReference<>();
95+
AtomicReference<Throwable> vortexError = new AtomicReference<>();
96+
PipedWriter oracleSink = new PipedWriter();
97+
PipedWriter vortexSink = new PipedWriter();
98+
try (BufferedReader oracleReader = new BufferedReader(new PipedReader(oracleSink, 1 << 17));
99+
BufferedReader vortexReader = new BufferedReader(new PipedReader(vortexSink, 1 << 17))) {
94100

95-
// When
96-
CsvExporter.exportCsv(vortex, vortexCsv, ExportOptions.defaults());
101+
Thread oracleThread = Thread.ofVirtual().start(() -> {
102+
try (oracleSink) {
103+
writeOracleCsv(parquet, oracleSink);
104+
} catch (Throwable t) {
105+
oracleError.set(t);
106+
}
107+
});
108+
Thread vortexThread = Thread.ofVirtual().start(() -> {
109+
try (vortexSink) {
110+
CsvExporter.exportCsv(vortex, vortexSink, ExportOptions.defaults());
111+
} catch (Throwable t) {
112+
vortexError.set(t);
113+
}
114+
});
97115

98-
// Then
99-
assertFilesMatch(vortexCsv, oracleCsv);
100-
} finally {
101-
Files.deleteIfExists(oracleCsv);
102-
Files.deleteIfExists(vortexCsv);
116+
// When / Then
117+
Throwable mainError = null;
118+
try {
119+
assertFilesMatch(vortexReader, oracleReader);
120+
} catch (Throwable t) {
121+
mainError = t;
122+
}
123+
124+
try {
125+
oracleThread.join();
126+
vortexThread.join();
127+
} catch (InterruptedException ie) {
128+
Thread.currentThread().interrupt();
129+
}
130+
131+
// Propagate in priority order: mismatch > decode gap > oracle abort
132+
Throwable oe = oracleError.get();
133+
Throwable ve = vortexError.get();
134+
if (mainError instanceof AssertionError e) {
135+
throw e;
136+
}
137+
if (ve instanceof VortexException e) {
138+
throw e;
139+
}
140+
if (ve instanceof IndexOutOfBoundsException e) {
141+
throw e;
142+
}
143+
if (oe instanceof TestAbortedException e) {
144+
throw e;
145+
}
146+
if (oe != null) {
147+
throw new TestAbortedException("oracle cannot read the parquet sibling: " + oe);
148+
}
149+
if (ve instanceof IOException e) {
150+
throw e;
151+
}
152+
if (ve instanceof RuntimeException e) {
153+
throw e;
154+
}
155+
if (ve != null) {
156+
throw new RuntimeException(ve);
157+
}
158+
if (mainError instanceof IOException e) {
159+
throw e;
160+
}
161+
if (mainError instanceof RuntimeException e) {
162+
throw e;
163+
}
164+
if (mainError != null) {
165+
throw new RuntimeException(mainError);
166+
}
103167
}
104168
}
105169

@@ -159,24 +223,32 @@ private static void assertStillFails(Path vortex, Path parquet, String status) t
159223
}
160224
}
161225

162-
/// Stream-compares two CSV files line by line without loading either into heap.
226+
/// Stream-compares two CSV sources line by line without loading either into heap.
163227
///
164-
/// @param result the vortex-java export
165-
/// @param oracle the parquet oracle export
166-
/// @throws AssertionError if any line differs or the files have different lengths
167-
/// @throws IOException if either file cannot be read
228+
/// @param result the vortex-java export reader
229+
/// @param oracle the parquet oracle reader
230+
/// @throws AssertionError if any line differs or the sources have different lengths
231+
/// @throws IOException if either reader throws
232+
private static void assertFilesMatch(BufferedReader result, BufferedReader oracle) throws IOException {
233+
long lineNum = 0;
234+
String rLine;
235+
while ((rLine = result.readLine()) != null) {
236+
lineNum++;
237+
String oLine = oracle.readLine();
238+
assertThat(oLine).as("oracle ended before vortex output at line %d", lineNum).isNotNull();
239+
assertThat(rLine).as("line %d", lineNum).isEqualTo(oLine);
240+
}
241+
assertThat(oracle.readLine()).as("vortex output ended before oracle at line %d", lineNum + 1).isNull();
242+
}
243+
244+
/// @param result the vortex-java export file
245+
/// @param oracle the parquet oracle file
246+
/// @throws AssertionError if any line differs or the files have different lengths
247+
/// @throws IOException if either file cannot be read
168248
private static void assertFilesMatch(Path result, Path oracle) throws IOException {
169249
try (BufferedReader r = Files.newBufferedReader(result);
170250
BufferedReader o = Files.newBufferedReader(oracle)) {
171-
long lineNum = 0;
172-
String rLine;
173-
while ((rLine = r.readLine()) != null) {
174-
lineNum++;
175-
String oLine = o.readLine();
176-
assertThat(oLine).as("oracle ended before vortex output at line %d", lineNum).isNotNull();
177-
assertThat(rLine).as("line %d", lineNum).isEqualTo(oLine);
178-
}
179-
assertThat(o.readLine()).as("vortex output ended before oracle at line %d", lineNum + 1).isNull();
251+
assertFilesMatch(r, o);
180252
}
181253
}
182254

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ basketball,untriaged
2323
behavioral-risk-factor-surveillance-system,ok
2424
beir-msmarco,untriaged
2525
bi-arade,ok
26-
bi-bimbo,untriaged
26+
bi-bimbo,ok
2727
bi-citymaxcapita,ok
2828
bi-cmsprovider,untriaged
2929
bi-commongovernment,untriaged
3030
bi-corporations,ok
31-
bi-eixo,untriaged
31+
bi-eixo,ok
3232
bi-euro2016,ok
3333
bi-food,ok
3434
bi-generico,untriaged
@@ -39,8 +39,8 @@ bi-iglocations2,untriaged
3939
bi-iublibrary,ok
4040
bi-medicare1,untriaged
4141
bi-medicare2,untriaged
42-
bi-medicare3,untriaged
43-
bi-medpayment1,untriaged
42+
bi-medicare3,ok
43+
bi-medpayment1,ok
4444
bi-medpayment2,untriaged
4545
bi-mlb,untriaged
4646
bi-motos,untriaged
Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
package io.github.dfa1.vortex.reader.decode;
22

3+
import io.github.dfa1.vortex.core.error.VortexException;
4+
import io.github.dfa1.vortex.core.model.DType;
35
import io.github.dfa1.vortex.core.model.EncodingId;
46
import io.github.dfa1.vortex.reader.array.Array;
7+
import io.github.dfa1.vortex.reader.array.BoolArray;
8+
import io.github.dfa1.vortex.reader.array.MaskedArray;
59
import io.github.dfa1.vortex.reader.array.MaterializedBoolArray;
610

711
/// Read-only decoder for `vortex.bool` (bit-packed boolean arrays, LSB first).
812
///
9-
/// Read-only decoder for bit-packed boolean arrays.
13+
/// When the encoding node has one child, that child is the validity bitmask:
14+
/// a [BoolArray] where `false` marks null rows. The values array is wrapped in
15+
/// a [MaskedArray] so callers see nulls as invalid rows.
1016
public final class BoolEncodingDecoder implements EncodingDecoder {
1117

1218
@Override
@@ -16,6 +22,16 @@ public EncodingId encodingId() {
1622

1723
@Override
1824
public Array decode(DecodeContext ctx) {
19-
return new MaterializedBoolArray(ctx.dtype(), ctx.rowCount(), ctx.buffer(0));
25+
long n = ctx.rowCount();
26+
Array values = new MaterializedBoolArray(ctx.dtype(), n, ctx.buffer(0));
27+
if (ctx.node().children().length == 1) {
28+
Array va = ctx.decodeChild(0, DType.BOOL, n);
29+
if (!(va instanceof BoolArray validity)) {
30+
throw new VortexException(EncodingId.VORTEX_BOOL,
31+
"validity child decoded to unexpected type: " + va.getClass().getSimpleName());
32+
}
33+
return new MaskedArray(values, validity);
34+
}
35+
return values;
2036
}
2137
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
package io.github.dfa1.vortex.reader.decode;
2+
3+
import io.github.dfa1.vortex.core.model.EncodingId;
4+
import io.github.dfa1.vortex.encoding.DTypes;
5+
import io.github.dfa1.vortex.reader.ReadRegistry;
6+
import io.github.dfa1.vortex.reader.array.BoolArray;
7+
import io.github.dfa1.vortex.reader.array.MaskedArray;
8+
9+
import org.junit.jupiter.api.Test;
10+
import org.junit.jupiter.params.ParameterizedTest;
11+
import org.junit.jupiter.params.provider.Arguments;
12+
import org.junit.jupiter.params.provider.MethodSource;
13+
14+
import java.lang.foreign.Arena;
15+
import java.lang.foreign.MemorySegment;
16+
import java.util.stream.Stream;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
class BoolEncodingDecoderTest {
21+
22+
// Bit-packs a boolean[] LSB-first into a byte[]: bit i is at byte[i/8] bit (i%8).
23+
private static MemorySegment packBits(boolean[] values) {
24+
byte[] bytes = new byte[(values.length + 7) / 8];
25+
for (int i = 0; i < values.length; i++) {
26+
if (values[i]) {
27+
bytes[i / 8] |= (byte) (1 << (i % 8));
28+
}
29+
}
30+
return MemorySegment.ofArray(bytes);
31+
}
32+
33+
// Builds a context with no validity child (non-nullable bool).
34+
private static DecodeContext nonNullableCtx(boolean[] values) {
35+
MemorySegment bits = packBits(values);
36+
ArrayNode node = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[0], new int[]{0});
37+
ReadRegistry registry = TestRegistry.ofDecoders(new BoolEncodingDecoder());
38+
return new DecodeContext(node, DTypes.BOOL, values.length, new MemorySegment[]{bits}, registry, Arena.ofAuto());
39+
}
40+
41+
// Builds a context with a vortex.bool validity child (nullable bool).
42+
// The validity child is itself bit-packed; false = null row.
43+
private static DecodeContext nullableCtx(boolean[] values, boolean[] valid) {
44+
MemorySegment bits = packBits(values);
45+
MemorySegment validBits = packBits(valid);
46+
ArrayNode validityNode = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[0], new int[]{1});
47+
ArrayNode node = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[]{validityNode}, new int[]{0});
48+
ReadRegistry registry = TestRegistry.ofDecoders(new BoolEncodingDecoder());
49+
return new DecodeContext(node, DTypes.BOOL_N, values.length,
50+
new MemorySegment[]{bits, validBits}, registry, Arena.ofAuto());
51+
}
52+
53+
static Stream<Arguments> nonNullableCases() {
54+
return Stream.of(
55+
Arguments.of("empty", new boolean[]{}),
56+
Arguments.of("all false", new boolean[]{false, false, false}),
57+
Arguments.of("all true", new boolean[]{true, true, true}),
58+
Arguments.of("mixed", new boolean[]{true, false, true, true, false}),
59+
// Crosses a byte boundary: bit 7 is in byte 0, bit 8 is in byte 1.
60+
Arguments.of("byte boundary", new boolean[]{false, false, false, false, false, false, false, true, false})
61+
);
62+
}
63+
64+
@ParameterizedTest(name = "{0}")
65+
@MethodSource("nonNullableCases")
66+
void decode_nonNullable_returnsBoolArray(String name, boolean[] values) {
67+
// Given
68+
DecodeContext ctx = nonNullableCtx(values);
69+
var sut = new BoolEncodingDecoder();
70+
71+
// When
72+
var result = sut.decode(ctx);
73+
74+
// Then — non-nullable path returns a plain BoolArray, not a MaskedArray
75+
assertThat(result).isInstanceOf(BoolArray.class);
76+
assertThat(result).isNotInstanceOf(MaskedArray.class);
77+
assertThat(result.length()).isEqualTo(values.length);
78+
BoolArray boolArr = (BoolArray) result;
79+
for (int i = 0; i < values.length; i++) {
80+
assertThat(boolArr.getBoolean(i)).as("index %d", i).isEqualTo(values[i]);
81+
}
82+
}
83+
84+
@Test
85+
void decode_nullable_returnsMaskedArray_withNullsHidingUnderlying() {
86+
// Given — rows 0,2 are valid; row 1 is null. The value bits at null positions are
87+
// irrelevant, but a non-zero bit is stored there to confirm the decoder ignores them.
88+
boolean[] values = {true, true, false};
89+
boolean[] valid = {true, false, true};
90+
DecodeContext ctx = nullableCtx(values, valid);
91+
var sut = new BoolEncodingDecoder();
92+
93+
// When
94+
var result = sut.decode(ctx);
95+
96+
// Then — nullable path wraps in MaskedArray; null rows report isValid=false.
97+
assertThat(result).isInstanceOf(MaskedArray.class);
98+
MaskedArray masked = (MaskedArray) result;
99+
assertThat(masked.length()).isEqualTo(3);
100+
assertThat(masked.isValid(0)).isTrue();
101+
assertThat(masked.isValid(1)).isFalse();
102+
assertThat(masked.isValid(2)).isTrue();
103+
// Valid rows deliver the correct boolean value via the inner BoolArray.
104+
BoolArray inner = (BoolArray) masked.inner();
105+
assertThat(inner.getBoolean(0)).isTrue();
106+
assertThat(inner.getBoolean(2)).isFalse();
107+
}
108+
109+
@Test
110+
void decode_nullable_allNulls_allRowsInvalid() {
111+
// Given — every validity bit is false; values buffer content does not matter
112+
boolean[] values = {false, false, false};
113+
boolean[] valid = {false, false, false};
114+
DecodeContext ctx = nullableCtx(values, valid);
115+
var sut = new BoolEncodingDecoder();
116+
117+
// When
118+
var result = sut.decode(ctx);
119+
120+
// Then
121+
assertThat(result).isInstanceOf(MaskedArray.class);
122+
MaskedArray masked = (MaskedArray) result;
123+
for (int i = 0; i < 3; i++) {
124+
assertThat(masked.isValid(i)).as("row %d must be null", i).isFalse();
125+
}
126+
}
127+
}

0 commit comments

Comments
 (0)