Skip to content

Commit abf892b

Browse files
dfa1claude
andcommitted
test: cover new decode/validity branches; fix S3034 dict bit-set masking (Sonar gate)
Fix the two java:S3034 findings (byte promoted to int before OR without masking) that gate-block new_reliability_rating; both are behavior-preserving (result is immediately re-cast to byte): - reader/decode/DictEncodingDecoder.java setBit (~L207) - reader/layout/DictLayoutDecoder.java validity-mask gather (~L200) Add coverage for the previously untested decode/validity branches these paths introduced: - B1: RleEncodingDecoder F32/F64 decode (run boundary, empty->constant array) and the readDoubles/readFloats broadcast helpers (new RleEncodingDecoderTest; helpers made package-private, unreachable via decode() as constant children fully materialize). - B6: LazyRleFloatArray.materialize constant-run + empty-chunk fast path (mirrors the double sibling). - B2: DictEncodingDecoder validity broadcast slow path (codesCap < rowCount, the setBit fix site) plus U16/U32 readCode widths with combined codes/pool nulls. - B3: DictLayoutDecoder byte/short dict pools (DictByteArray/DictShortArray) and the gatherRowValidity Short/Int/Long codes-type arms. - B4: ScanIterator merged-grid out-of-range guard (mismatched column totals) and the sliceArray StructArray branch via a hand-assembled nested-struct file (also drives collectFlats' struct arm). - B5: ZonedStatsSchema boolean/size stat arms, fixed-width (I64/I32) field skip, the >63-bit varint overflow, and inner-spec overflow-tag / unknown-wire-type bail-outs. Not reached (reported, not forced): the readCode/gatherRowValidity `default` throws are pre-empted upstream (the expand switch and DictArrays.validateCodes reject non-U8/U16/U32 codes before those arms run) — defensive dead code, left as-is rather than backed by an unkillable test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent be7c616 commit abf892b

10 files changed

Lines changed: 655 additions & 4 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ private static long readCode(MemorySegment codes, long i, PType codePType) {
204204
private static void setBit(MemorySegment bits, long i) {
205205
long byteIdx = i >>> 3;
206206
byte cur = bits.get(ValueLayout.JAVA_BYTE, byteIdx);
207-
bits.set(ValueLayout.JAVA_BYTE, byteIdx, (byte) (cur | (1 << (i & 7))));
207+
bits.set(ValueLayout.JAVA_BYTE, byteIdx, (byte) ((cur & 0xff) | (1 << (i & 7))));
208208
}
209209

210210
private static Array decodeUtf8DictLegacy(DecodeContext ctx, MemorySegment meta) {

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,10 @@ private static short[] readShorts(MemorySegment buf, int count, PType ptype) {
167167
return out;
168168
}
169169

170-
private static double[] readDoubles(MemorySegment buf, int count) {
170+
// Package-private (not private) so the broadcast branch (cap < count, a
171+
// ConstantEncoding values pool) is testable directly: through decode() a
172+
// constant child always materializes to `count` full elements, so cap == count.
173+
static double[] readDoubles(MemorySegment buf, int count) {
171174
double[] out = new double[count];
172175
long cap = SegmentBroadcast.capacity(buf, 8);
173176
for (int i = 0; i < count; i++) {
@@ -176,7 +179,7 @@ private static double[] readDoubles(MemorySegment buf, int count) {
176179
return out;
177180
}
178181

179-
private static float[] readFloats(MemorySegment buf, int count) {
182+
static float[] readFloats(MemorySegment buf, int count) {
180183
float[] out = new float[count];
181184
long cap = SegmentBroadcast.capacity(buf, 4);
182185
for (int i = 0; i < count; i++) {

reader/src/main/java/io/github/dfa1/vortex/reader/layout/DictLayoutDecoder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ private static BoolArray gatherRowValidity(Array codes, BoolArray codesValidity,
197197
if (valid) {
198198
long byteIdx = i >>> 3;
199199
byte cur = bits.get(ValueLayout.JAVA_BYTE, byteIdx);
200-
bits.set(ValueLayout.JAVA_BYTE, byteIdx, (byte) (cur | (1 << (i & 7))));
200+
bits.set(ValueLayout.JAVA_BYTE, byteIdx, (byte) ((cur & 0xff) | (1 << (i & 7))));
201201
}
202202
}
203203
return new MaterializedBoolArray(DType.BOOL, n, bits.asReadOnly());

reader/src/test/java/io/github/dfa1/vortex/reader/ScanIteratorChunkGridTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.github.dfa1.vortex.reader;
22

3+
import io.github.dfa1.vortex.core.error.VortexException;
34
import io.github.dfa1.vortex.core.model.ColumnName;
45
import io.github.dfa1.vortex.core.model.LayoutId;
56
import io.github.dfa1.vortex.reader.ScanIterator.ChunkSpec;
@@ -12,6 +13,7 @@
1213
import java.util.Map;
1314

1415
import static org.assertj.core.api.Assertions.assertThat;
16+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1517

1618
/// Unit tests for [ScanIterator#buildChunks(Map)] — the scan planner that merges each column's
1719
/// chunk grid into one split grid (the Rust reference's `StructReader::register_splits` union of
@@ -120,6 +122,21 @@ void disjointBoundariesSpliceAtTheMergedGrid() {
120122
assertThat(coveringFlat(result.get(2), B)).isSameAs(coveringFlat(result.get(3), B));
121123
}
122124

125+
@Test
126+
void columnShorterThanMergedGridThrows() {
127+
// Given two columns whose totals disagree — A spans 8 rows [4, 4] but B only 6 [3, 3]. A
128+
// well-formed file keeps every column the same length; a malformed/untrusted layout may not.
129+
// The merged grid {0, 3, 4, 6, 8} then produces a window [6, 8) that lies beyond B's last
130+
// chunk, so the covering-chunk advance loop runs B's cursor off the end. The guard must
131+
// surface that as a VortexException rather than an ArrayIndexOutOfBoundsException.
132+
var columnFlats = flats(A, new long[]{4, 4}, B, new long[]{3, 3});
133+
134+
// When / Then
135+
assertThatThrownBy(() -> ScanIterator.buildChunks(columnFlats))
136+
.isInstanceOf(VortexException.class)
137+
.hasMessageContaining("no chunk covering");
138+
}
139+
123140
@Test
124141
void emptyProjectionYieldsNoChunks() {
125142
// Given no columns

reader/src/test/java/io/github/dfa1/vortex/reader/VortexReaderDecodeChunkTest.java

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,46 @@ void scan_disjointMixedGrid_streamsExactValuesAcrossMergedWindows(@TempDir Path
310310
assertThat(streamedB).isEqualTo(MIXED_B_EXPECTED);
311311
}
312312

313+
// A NESTED struct column "s" (a vortex.struct under the root struct) with two I64 fields x, y,
314+
// stored as ONE full-range subtree beside the per-chunk column "a" [2, 3, 2]. collectFlats treats
315+
// the whole struct subtree as a single full-range chunk source, so the scan decodes it once as a
316+
// StructArray and slices it per window — the ScanIterator.sliceArray StructArray branch that no
317+
// primitive-only file reaches.
318+
private static final long[] NESTED_X = {1000, 1001, 1002, 1003, 1004, 1005, 1006};
319+
private static final long[] NESTED_Y = {2000, 2001, 2002, 2003, 2004, 2005, 2006};
320+
private static final List<Long> NESTED_A_EXPECTED =
321+
List.of(10L, 11L, 12L, 13L, 14L, 15L, 16L);
322+
323+
@Test
324+
void scan_nestedStructColumn_slicesEachFieldPerWindow(@TempDir Path tmp) throws Exception {
325+
// Given a struct whose second column "s" is itself a struct {x, y} spanning all 7 rows, beside
326+
// a per-chunk column "a" chunked [2, 3, 2]. Neither the Java nor JNI writer emits a nested
327+
// struct beside a per-chunk column in one file, so it is hand-assembled here.
328+
Path file = writeNestedStructFile(tmp);
329+
var streamedA = new ArrayList<Long>();
330+
var streamedX = new ArrayList<Long>();
331+
var streamedY = new ArrayList<Long>();
332+
333+
// When streaming the whole scan — "s" is decoded once over the full range then sliced into
334+
// each merged-grid window {0,2,5,7} via ScanIterator.sliceArray's StructArray arm, which
335+
// recursively slices each field into the window.
336+
try (var reader = VortexReader.open(file, registry());
337+
var iter = reader.scan(ScanOptions.all())) {
338+
iter.forEachRemaining(chunk -> {
339+
streamedA.addAll(values(chunk.column("a")));
340+
io.github.dfa1.vortex.reader.array.StructArray s = chunk.column("s");
341+
streamedX.addAll(fieldValues(s, 0));
342+
streamedY.addAll(fieldValues(s, 1));
343+
});
344+
}
345+
346+
// Then every field value survives the per-window struct slice, including the middle window
347+
// [2,5) sliced at a nonzero offset within the single full-range struct.
348+
assertThat(streamedA).isEqualTo(NESTED_A_EXPECTED);
349+
assertThat(streamedX).containsExactly(1000L, 1001L, 1002L, 1003L, 1004L, 1005L, 1006L);
350+
assertThat(streamedY).containsExactly(2000L, 2001L, 2002L, 2003L, 2004L, 2005L, 2006L);
351+
}
352+
313353
// ── Helpers ────────────────────────────────────────────────────────────────
314354

315355
private static ReadRegistry registry() {
@@ -330,6 +370,101 @@ private static List<Map<String, List<Long>>> streamAll(VortexReader reader) {
330370
return out;
331371
}
332372

373+
/// Reads field `fieldIdx` of a (possibly sliced) [StructArray] as a Long list.
374+
private static List<Long> fieldValues(io.github.dfa1.vortex.reader.array.StructArray s, int fieldIdx) {
375+
return values(s.field(fieldIdx));
376+
}
377+
378+
/// Assembles a struct file: column "a" chunked over [CHUNK_ROWS] (one flat per chunk) and a nested
379+
/// struct column "s" {x, y} stored as one full-range struct subtree (flat x, flat y). Segment
380+
/// order: a-chunk0..a-chunkN, then x, then y. Layout: flat=0, chunked=1, struct=2; array spec
381+
/// vortex.primitive=0.
382+
private static Path writeNestedStructFile(Path dir) throws Exception {
383+
int numChunks = CHUNK_ROWS.length;
384+
List<byte[]> segments = new ArrayList<>();
385+
for (int k = 0; k < numChunks; k++) {
386+
segments.add(primitiveSegment(COL_A[k], null));
387+
}
388+
int xSegIdx = segments.size();
389+
segments.add(primitiveSegment(NESTED_X, null));
390+
int ySegIdx = segments.size();
391+
segments.add(primitiveSegment(NESTED_Y, null));
392+
393+
long[] segOffsets = new long[segments.size()];
394+
long[] segLengths = new long[segments.size()];
395+
long off = 0;
396+
for (int i = 0; i < segments.size(); i++) {
397+
segOffsets[i] = off;
398+
segLengths[i] = segments.get(i).length;
399+
off += segments.get(i).length;
400+
}
401+
402+
ByteBuffer footerBuf = MalformedFiles.buildFooter(
403+
new String[]{"vortex.primitive", "vortex.bool"},
404+
new String[]{"vortex.flat", "vortex.chunked", "vortex.struct"},
405+
segOffsets, segLengths);
406+
ByteBuffer dtypeBuf = buildNestedStructDtype();
407+
ByteBuffer layoutBuf = buildNestedStructLayout(CHUNK_ROWS, xSegIdx, ySegIdx);
408+
409+
return writeVtxFile(dir, "nested.vtx", segments, footerBuf, dtypeBuf, layoutBuf);
410+
}
411+
412+
/// Root struct DType {a: I64 non-null, s: struct {x: I64, y: I64} non-null}.
413+
private static ByteBuffer buildNestedStructDtype() {
414+
var fbb = new FbsBuilder(512);
415+
int primA = FbsPrimitive.createFbsPrimitive(fbb, FbsPType.I64, false);
416+
int dtA = FbsDType.createFbsDType(fbb, FbsType.FbsPrimitive, primA);
417+
418+
int primX = FbsPrimitive.createFbsPrimitive(fbb, FbsPType.I64, false);
419+
int dtX = FbsDType.createFbsDType(fbb, FbsType.FbsPrimitive, primX);
420+
int primY = FbsPrimitive.createFbsPrimitive(fbb, FbsPType.I64, false);
421+
int dtY = FbsDType.createFbsDType(fbb, FbsType.FbsPrimitive, primY);
422+
int innerNames = FbsStruct_.createNamesVector(fbb,
423+
new int[]{fbb.createString("x"), fbb.createString("y")});
424+
int innerDtypes = FbsStruct_.createDtypesVector(fbb, new int[]{dtX, dtY});
425+
int innerStruct = FbsStruct_.createFbsStruct_(fbb, innerNames, innerDtypes, false);
426+
int dtS = FbsDType.createFbsDType(fbb, FbsType.FbsStruct_, innerStruct);
427+
428+
int names = FbsStruct_.createNamesVector(fbb,
429+
new int[]{fbb.createString("a"), fbb.createString("s")});
430+
int dtypes = FbsStruct_.createDtypesVector(fbb, new int[]{dtA, dtS});
431+
int rootStruct = FbsStruct_.createFbsStruct_(fbb, names, dtypes, false);
432+
int dt = FbsDType.createFbsDType(fbb, FbsType.FbsStruct_, rootStruct);
433+
FbsDType.finishFbsDTypeBuffer(fbb, dt);
434+
return MalformedFiles.slice(fbb);
435+
}
436+
437+
/// Layout: root struct(2) → [chunked "a" (flat per chunk), struct(2) "s" → [flat x, flat y]].
438+
/// Both "s" fields are single full-range flats, so the whole subtree is one full-range chunk
439+
/// source alongside "a"'s per-chunk grid.
440+
private static ByteBuffer buildNestedStructLayout(long[] chunkRows, int xSegIdx, int ySegIdx) {
441+
var fbb = new FbsBuilder(1024);
442+
int numChunks = chunkRows.length;
443+
long total = 0;
444+
for (long r : chunkRows) {
445+
total += r;
446+
}
447+
int[] flatOffs = new int[numChunks];
448+
for (int k = 0; k < numChunks; k++) {
449+
int segV = FbsLayout.createSegmentsVector(fbb, new long[]{k});
450+
flatOffs[k] = FbsLayout.createFbsLayout(fbb, 0, chunkRows[k], 0, 0, segV);
451+
}
452+
int chunkedChildV = FbsLayout.createChildrenVector(fbb, flatOffs);
453+
int chunkedA = FbsLayout.createFbsLayout(fbb, 1, total, 0, chunkedChildV, 0);
454+
455+
int xSegV = FbsLayout.createSegmentsVector(fbb, new long[]{xSegIdx});
456+
int flatX = FbsLayout.createFbsLayout(fbb, 0, total, 0, 0, xSegV);
457+
int ySegV = FbsLayout.createSegmentsVector(fbb, new long[]{ySegIdx});
458+
int flatY = FbsLayout.createFbsLayout(fbb, 0, total, 0, 0, ySegV);
459+
int sChildV = FbsLayout.createChildrenVector(fbb, new int[]{flatX, flatY});
460+
int structS = FbsLayout.createFbsLayout(fbb, 2, total, 0, sChildV, 0);
461+
462+
int structChildV = FbsLayout.createChildrenVector(fbb, new int[]{chunkedA, structS});
463+
int structOff = FbsLayout.createFbsLayout(fbb, 2, total, 0, structChildV, 0);
464+
FbsLayout.finishFbsLayoutBuffer(fbb, structOff);
465+
return MalformedFiles.slice(fbb);
466+
}
467+
333468
private static List<Long> values(Array arr) {
334469
List<Long> out = new ArrayList<>();
335470
if (arr instanceof MaskedArray masked) {

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,28 @@ void indexedClampAndEmptyChunk() {
521521
assertThat(empty.getFloat(0)).isZero();
522522
}
523523

524+
@Test
525+
void materializeConstantRunAndEmptyChunk() {
526+
// Given a constant-run chunk (1 distinct value) and, separately, an empty chunk
527+
// (0 distinct values): materialize's processChunk fast path must emit the lone
528+
// value — or 0.0f for the empty pool — for every row without touching indices.
529+
var constant = new LazyRleFloatArray(F32, 3, new float[]{6.25f}, new int[1024],
530+
new long[]{0L}, 0L, 1L, 1, 0);
531+
var empty = new LazyRleFloatArray(F32, 2, new float[0], new int[1024],
532+
new long[]{0L}, 0L, 0L, 1, 0);
533+
534+
// When / Then
535+
try (var arena = Arena.ofConfined()) {
536+
var constantResult = constant.materialize(arena);
537+
assertThat(constantResult.getAtIndex(VortexFormat.LE_FLOAT, 0)).isEqualTo(6.25f);
538+
assertThat(constantResult.getAtIndex(VortexFormat.LE_FLOAT, 2)).isEqualTo(6.25f);
539+
540+
var emptyResult = empty.materialize(arena);
541+
assertThat(emptyResult.getAtIndex(VortexFormat.LE_FLOAT, 0)).isEqualTo(0.0f);
542+
assertThat(emptyResult.getAtIndex(VortexFormat.LE_FLOAT, 1)).isEqualTo(0.0f);
543+
}
544+
}
545+
524546
@Test
525547
void materializeDecodesEveryRow() {
526548
// Given a mixed chunk; materialize must emit each logical row once, honoring

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,53 @@ void codeOutOfRangeWithPoolValidity_throwsVortexException() {
508508
.hasMessageContaining("out of range for pool validity");
509509
}
510510

511+
@Test
512+
void codesBroadcast_slowPathSetsValidityBits() {
513+
// Given — a single-element codes buffer (codesCap == 1 < rowCount == 4, the
514+
// ConstantEncoding fan-out shape) forces rowValidity's broadcast slow path
515+
// (the `i % codesCap` loop and the setBit S3034 fix site). The lone code points at
516+
// valid pool slot 0, so every row is valid and setBit runs on each iteration.
517+
ArrayNode codesNode = primitiveNode(0);
518+
ArrayNode valuesNode = maskedPrimitiveNode(1, 2);
519+
MemorySegment[] segs = {
520+
u8Codes(0), // 1 code, broadcast across 4 rows
521+
TestSegments.leInts(10, 20),
522+
boolBitmap(true, false) // pool slot 0 valid, slot 1 invalid
523+
};
524+
525+
// When
526+
Array result = decodeDict(DType.I32, PType.U8, 2, 4, segs, codesNode, valuesNode);
527+
528+
// Then — the broadcast code resolves to valid slot 0 for every row
529+
MaskedArray masked = assertMasked(result);
530+
assertValidity(masked, true, true, true, true);
531+
assertIntValues(masked, 10, 10, 10, 10);
532+
}
533+
534+
@ParameterizedTest(name = "codes={0}")
535+
@org.junit.jupiter.params.provider.EnumSource(value = PType.class, names = {"U16", "U32"})
536+
void poolNull_readsWiderCodeWidths(PType codePType) {
537+
// Given — codes at the wider widths (U16/U32) with a pool-null slot. rowValidity must
538+
// read each code at the correct stride via readCode's U16/U32 arms (only exercised when
539+
// pool validity is present), then mask rows whose code points at the dead slot. Here
540+
// codesCap == rowCount, so the fast validity loop runs.
541+
ArrayNode codesNode = primitiveNode(0);
542+
ArrayNode valuesNode = maskedPrimitiveNode(1, 2);
543+
MemorySegment[] segs = {
544+
codeSegment(codePType, new long[]{0, 1, 0, 1}),
545+
TestSegments.leInts(10, 20),
546+
boolBitmap(true, false) // slot 1 invalid
547+
};
548+
549+
// When
550+
Array result = decodeDict(DType.I32, codePType, 2, 4, segs, codesNode, valuesNode);
551+
552+
// Then — rows referencing the dead slot 1 are null; their expanded value is still present
553+
MaskedArray masked = assertMasked(result);
554+
assertValidity(masked, true, false, true, false);
555+
assertIntValues(masked, 10, 20, 10, 20);
556+
}
557+
511558
private Array decodeDict(DType dtype, PType codePType, int valuesLen, long rowCount,
512559
MemorySegment[] segs, ArrayNode codesNode, ArrayNode valuesNode) {
513560
MemorySegment meta = MemorySegment.ofArray(

0 commit comments

Comments
 (0)