diff --git a/CHANGELOG.md b/CHANGELOG.md index cd723ca5..6d0e2b61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A `vortex.bytebool` column is now read in place from its mmapped buffer, as `docs/compatibility.md` already claimed: decode allocated an `n/8`-byte bitmap and ran a read-modify-write over every row to fill it, for the one boolean encoding whose buffer is already indexable per row. Callers that want a bitmap still get one from `materialize`. ([#339](https://github.com/dfa1/vortex-java/issues/339)) +- A `vortex.bytebool` buffer shorter than the declared row count now fails as `VortexException` instead of a raw `IndexOutOfBoundsException` on whichever row ran off the end. ([#339](https://github.com/dfa1/vortex-java/issues/339)) +- Same for a `vortex.bool` bitmap holding fewer than the `(rows + 7) / 8` bytes it needs, reached either as a column or as another column's validity child. ([#339](https://github.com/dfa1/vortex-java/issues/339)) + - A malformed `fastlanes.delta` column no longer fails with a raw JDK exception: a row window running past the elements the chunks reconstruct threw `ArrayIndexOutOfBoundsException`, and an absurd or negative declared element count sized a heap array before anything checked it (`NegativeArraySizeException`, or `OutOfMemoryError`). All now fail as `VortexException`. ([#338](https://github.com/dfa1/vortex-java/issues/338)) - A `fastlanes.delta` column no longer routes its decode through four row-scaled heap `long[]` arrays, every value widened to 8 bytes whatever the column's width; values are reconstructed into a single arena segment at the ptype's real width, and only the chunks overlapping the requested rows are reconstructed at all. ([#338](https://github.com/dfa1/vortex-java/issues/338)) diff --git a/docs/compatibility.md b/docs/compatibility.md index e17b28bd..1b63a561 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -134,7 +134,7 @@ decoder falls into one of three shapes: | `vortex.primitive` | Zero-copy | Zero-copy | mmap slice | | `vortex.bool` | Zero-copy | Zero-copy | mmap slice (bit-packed) | | `vortex.null` | n/a | n/a | no per-row data | -| `vortex.bytebool` | Zero-copy | Zero-copy | mmap slice | +| `vortex.bytebool` | Zero-copy | Zero-copy | `LazyByteBoolArray` — mmap slice read byte-per-row; bitmap on `materialize` | | `vortex.zigzag` | Lazy | Lazy | `LazyZigZagXxxArray` (I8/I16/I32/I64); broadcast → `LazyConstantXxxArray`, ADR 0010 + 0015 | | `vortex.constant` | Lazy | Lazy | `LazyConstantXxxArray` (primitive + bool + decimal) + `VarBinConstantArray` (Utf8/Binary); per-row broadcast, no buffer, ADR 0015 | | `vortex.ext` | Zero-copy | Zero-copy | wraps storage | diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/array/LazyByteBoolArray.java b/reader/src/main/java/io/github/dfa1/vortex/reader/array/LazyByteBoolArray.java new file mode 100644 index 00000000..5e9c2f95 --- /dev/null +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/array/LazyByteBoolArray.java @@ -0,0 +1,45 @@ +package io.github.dfa1.vortex.reader.array; + +import io.github.dfa1.vortex.core.model.DType; + +import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; +import java.util.Objects; + +/// Zero-copy [BoolArray] over a `vortex.bytebool` buffer: one byte per row, non-zero meaning +/// `true`. +/// +/// `vortex.bytebool` is the one boolean encoding whose source buffer is already directly +/// indexable per row, so decode has nothing to do — this reads the mmapped bytes in place +/// instead of allocating an `n/8`-byte bitmap and running a read-modify-write over every row +/// to fill it. +/// +/// [#materialize(java.lang.foreign.SegmentAllocator)] still hands out the LSB-first bitmap the +/// rest of the reader speaks, via [BoolArray]'s default — the same packing loop this decode +/// used to run eagerly, now paid only by callers that actually want a bitmap. +/// +/// [Array#segmentIfPresent()] is left empty (the interface default): the backing segment is a +/// byte-per-row buffer, not the bit-packed layout a caller asking for a bool array's segment +/// expects, so handing it over would be misread. +/// +/// @param dtype logical [DType.Bool] type +/// @param length number of logical rows +/// @param bytes one byte per row; non-zero is `true`. Must hold at least `length` bytes — +/// [io.github.dfa1.vortex.reader.decode.ByteBoolEncodingDecoder] checks that +/// once, so the accessors here do not re-check it per row +public record LazyByteBoolArray(DType dtype, long length, MemorySegment bytes) implements BoolArray { + + @Override + public boolean getBoolean(long i) { + Objects.checkIndex(i, length); + return bytes.get(ValueLayout.JAVA_BYTE, i) != 0; + } + + @Override + public void forEachBoolean(BooleanConsumer c) { + long n = length; + for (long i = 0; i < n; i++) { + c.accept(bytes.get(ValueLayout.JAVA_BYTE, i) != 0); + } + } +} diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/array/MaterializedBoolArray.java b/reader/src/main/java/io/github/dfa1/vortex/reader/array/MaterializedBoolArray.java index 0f1cb791..9b8691ca 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/array/MaterializedBoolArray.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/array/MaterializedBoolArray.java @@ -8,6 +8,12 @@ /// Buffer-backed [BoolArray] — the fallback used when an encoding decoder /// either materializes the output eagerly or has no lazy variant of its own. +/// +/// [#getBoolean(long)] indexes `buffer` without a bounds check, so the buffer must hold at +/// least `(length + 7) / 8` bytes. Every decoder that builds the bitmap itself allocates +/// exactly that; the one that passes a file buffer through +/// (`io.github.dfa1.vortex.reader.decode.BoolEncodingDecoder`) checks the size once before +/// constructing this, rather than paying a bound per row. public final class MaterializedBoolArray extends AbstractMaterializedArray implements BoolArray { /// Constructs a `MaterializedBoolArray` backed by the given bit-packed buffer. diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/BoolEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/BoolEncodingDecoder.java index 8a4f1c2b..6eed49a7 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/BoolEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/BoolEncodingDecoder.java @@ -8,6 +8,8 @@ import io.github.dfa1.vortex.reader.array.MaskedArray; import io.github.dfa1.vortex.reader.array.MaterializedBoolArray; +import java.lang.foreign.MemorySegment; + /// Read-only decoder for `vortex.bool` (bit-packed boolean arrays, LSB first). /// /// When the encoding node has one child, that child is the validity bitmask: @@ -23,7 +25,20 @@ public EncodingId encodingId() { @Override public Array decode(DecodeContext ctx) { long n = ctx.rowCount(); - Array values = new MaterializedBoolArray(ctx.dtype(), n, ctx.buffer(0)); + MemorySegment bits = ctx.buffer(0); + // The bitmap comes straight from the file and needs one byte per 8 rows. A shorter one + // is malformed, and [MaterializedBoolArray#getBoolean] indexes its buffer unchecked — + // deliberately, since every other construction site allocates the bitmap itself at + // exactly this size — so without this the read of whichever row runs off the end is a + // raw IndexOutOfBoundsException (ADR 0003). O(1), and it also covers `materialize`, + // which hands the same short buffer straight to the caller. + long needed = (n + 7) >>> 3; + if (bits.byteSize() < needed) { + throw new VortexException(EncodingId.VORTEX_BOOL, + "bool bitmap of " + bits.byteSize() + " byte(s) is shorter than the " + + needed + " byte(s) needed for " + n + " row(s)"); + } + Array values = new MaterializedBoolArray(ctx.dtype(), n, bits); if (ctx.node().children().length == 1) { Array va = ctx.decodeChild(0, DType.BOOL, n); if (!(va instanceof BoolArray validity)) { diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/ByteBoolEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/ByteBoolEncodingDecoder.java index 1d95f53f..5427e93f 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/ByteBoolEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/ByteBoolEncodingDecoder.java @@ -1,14 +1,13 @@ package io.github.dfa1.vortex.reader.decode; +import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.model.EncodingId; import io.github.dfa1.vortex.reader.array.Array; -import io.github.dfa1.vortex.reader.array.MaterializedBoolArray; +import io.github.dfa1.vortex.reader.array.LazyByteBoolArray; import java.lang.foreign.MemorySegment; -import java.lang.foreign.ValueLayout; -/// Read-only decoder for `vortex.bytebool` — packs the input byte buffer into the -/// bit-packed [MaterializedBoolArray] layout used by `vortex.bool`. +/// Read-only decoder for `vortex.bytebool` — one byte per boolean, read in place. public final class ByteBoolEncodingDecoder implements EncodingDecoder { @Override @@ -20,15 +19,16 @@ public EncodingId encodingId() { public Array decode(DecodeContext ctx) { long n = ctx.rowCount(); MemorySegment bytes = ctx.buffer(0); - long packedBytes = (n + 7) >>> 3; - MemorySegment packed = ctx.arena().allocate(packedBytes > 0 ? packedBytes : 1); - for (long i = 0; i < n; i++) { - if (bytes.get(ValueLayout.JAVA_BYTE, i) != 0) { - long byteIdx = i >>> 3; - byte cur = packed.get(ValueLayout.JAVA_BYTE, byteIdx); - packed.set(ValueLayout.JAVA_BYTE, byteIdx, (byte) ((cur & 0xff) | (1 << (i & 7)))); - } + // The buffer comes straight from the file and holds one byte per row, so a shorter one + // is malformed. Checked once here, in O(1), rather than per row: it keeps + // LazyByteBoolArray's accessor uniform, and a crafted file fails as a VortexException + // instead of a raw IndexOutOfBoundsException on whichever row runs off the end + // (ADR 0003) — which is what the eager packing loop this replaces did. + if (bytes.byteSize() < n) { + throw new VortexException(EncodingId.VORTEX_BYTEBOOL, + "bytebool buffer of " + bytes.byteSize() + " byte(s) is shorter than the " + + n + " declared row(s)"); } - return new MaterializedBoolArray(ctx.dtype(), n, packed); + return new LazyByteBoolArray(ctx.dtype(), n, bytes); } } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/array/LazyByteBoolArrayTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/array/LazyByteBoolArrayTest.java new file mode 100644 index 00000000..1d6267b6 --- /dev/null +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/array/LazyByteBoolArrayTest.java @@ -0,0 +1,158 @@ +package io.github.dfa1.vortex.reader.array; + +import io.github.dfa1.vortex.core.model.DType; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class LazyByteBoolArrayTest { + + private static final DType BOOL = DType.BOOL; + + @Nested + class Accessors { + + /// Any non-zero byte is `true`, not just 1 — the encoder writes 1, but the format does + /// not promise it and a Rust-written file may carry other values. + @ParameterizedTest + @CsvSource({"0,false", "1,true", "2,true", "42,true", "-1,true", "-128,true"}) + void getBoolean_treatsAnyNonZeroByteAsTrue(byte value, boolean expected) { + // Given + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, 1, bytes(value)); + + // When + boolean result = sut.getBoolean(0); + + // Then + assertThat(result).isEqualTo(expected); + } + + @Test + void getBoolean_resolvesEachRowIndependently() { + // Given + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, 4, bytes((byte) 0, (byte) 1, (byte) 0, (byte) 1)); + + // When / Then + assertThat(sut.getBoolean(0)).isFalse(); + assertThat(sut.getBoolean(1)).isTrue(); + assertThat(sut.getBoolean(2)).isFalse(); + assertThat(sut.getBoolean(3)).isTrue(); + } + + @Test + void getBoolean_outOfBoundsIndex_throws() { + // Given + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, 2, bytes((byte) 1, (byte) 0)); + + // When / Then + assertThatThrownBy(() -> sut.getBoolean(2)).isInstanceOf(IndexOutOfBoundsException.class); + } + + /// A shorter `length` than the buffer holds is how a sliced or trimmed column arrives; + /// the trailing bytes must stay invisible. + @Test + void getBoolean_lengthShorterThanBuffer_hidesTrailingBytes() { + // Given — 4 bytes of data but only 2 rows claimed + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, 2, bytes((byte) 1, (byte) 0, (byte) 1, (byte) 1)); + + // When / Then + assertThat(sut.getBoolean(1)).isFalse(); + assertThatThrownBy(() -> sut.getBoolean(2)).isInstanceOf(IndexOutOfBoundsException.class); + } + + @Test + void forEachBoolean_visitsEveryRowInOrder() { + // Given + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, 3, bytes((byte) 1, (byte) 0, (byte) 1)); + List seen = new ArrayList<>(); + + // When + sut.forEachBoolean(seen::add); + + // Then + assertThat(seen).containsExactly(true, false, true); + } + } + + @Nested + class Representation { + + /// The byte-per-row buffer is not the LSB-first bitmap a caller asking a bool array for + /// its segment expects, so it must not be handed over as one. + @Test + void segmentIfPresent_isEmpty() { + // Given + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, 2, bytes((byte) 1, (byte) 0)); + + // When / Then + assertThat(sut.segmentIfPresent()).isEmpty(); + } + + /// The bit-packing the decoder used to do eagerly, now on demand: bits are LSB-first, + /// so rows 0 and 9 land in bit 0 of bytes 0 and 1. + @Test + void materialize_packsLsbFirstBitmap() { + // Given — 10 rows, true at 0 and 9 + byte[] raw = new byte[10]; + raw[0] = 1; + raw[9] = 1; + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, 10, bytes(raw)); + + // When + MemorySegment result = sut.materialize(Arena.ofAuto()); + + // Then + assertThat(result.byteSize()).isEqualTo(2); + assertThat(result.get(ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) 0b0000_0001); + assertThat(result.get(ValueLayout.JAVA_BYTE, 1)).isEqualTo((byte) 0b0000_0010); + } + + /// Round-trip through the bitmap must agree with reading the bytes directly — the two + /// are what a consumer picks between, so they cannot disagree. + @Test + void materialize_agreesWithGetBoolean() { + // Given — 20 rows with an irregular pattern, so a byte-boundary slip is visible + byte[] raw = new byte[20]; + for (int i = 0; i < raw.length; i++) { + raw[i] = (byte) (i % 3 == 0 ? 1 : 0); + } + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, raw.length, bytes(raw)); + + // When + MemorySegment result = sut.materialize(Arena.ofAuto()); + + // Then + BoolArray packed = new MaterializedBoolArray(BOOL, raw.length, result); + for (long i = 0; i < raw.length; i++) { + assertThat(packed.getBoolean(i)).as("row %d", i).isEqualTo(sut.getBoolean(i)); + } + } + + @Test + void limited_capsTheRowCount() { + // Given + LazyByteBoolArray sut = new LazyByteBoolArray(BOOL, 4, bytes((byte) 1, (byte) 0, (byte) 1, (byte) 1)); + + // When + Array result = sut.limited(2); + + // Then + assertThat(result.length()).isEqualTo(2); + assertThat(((BoolArray) result).getBoolean(1)).isFalse(); + } + } + + private static MemorySegment bytes(byte... values) { + return MemorySegment.ofArray(values); + } +} diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/BoolEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/BoolEncodingDecoderTest.java index 48e7cf29..15ccc3cb 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/BoolEncodingDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/BoolEncodingDecoderTest.java @@ -1,5 +1,6 @@ package io.github.dfa1.vortex.reader.decode; +import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.model.EncodingId; import io.github.dfa1.vortex.core.testing.DTypes; import io.github.dfa1.vortex.reader.ReadRegistry; @@ -9,6 +10,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import java.lang.foreign.Arena; @@ -16,6 +18,7 @@ import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class BoolEncodingDecoderTest { @@ -105,6 +108,73 @@ void decode_nullable_returnsMaskedArray_withNullsHidingUnderlying() { assertThat(inner.getBoolean(2)).isFalse(); } + /// The bitmap is untrusted and needs one byte per 8 rows. `MaterializedBoolArray` indexes + /// its buffer without a per-row bound — deliberately, since every other decoder that builds + /// a bitmap allocates it at exactly the right size — so a short one used to fault as a raw + /// `IndexOutOfBoundsException` on whichever row ran off the end, or hand the truncated + /// buffer straight out of `materialize`. It must be a [VortexException] (ADR 0003). + /// + /// The row counts are one past each byte boundary, where a bitmap is at its most + /// deceptive: 9 rows need 2 bytes, not the 1 that covers the first 8. + @ParameterizedTest + @CsvSource({"9, 1", "17, 2", "65, 8", "8, 0"}) + void decode_bitmapShorterThanRowCount_throws(int rows, int suppliedBytes) { + // Given + MemorySegment bits = MemorySegment.ofArray(new byte[suppliedBytes]); + ArrayNode node = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[0], new int[]{0}); + ReadRegistry registry = TestRegistry.ofDecoders(new BoolEncodingDecoder()); + DecodeContext ctx = new DecodeContext(node, DTypes.BOOL, rows, new MemorySegment[]{bits}, + registry, Arena.ofAuto()); + var sut = new BoolEncodingDecoder(); + + // When / Then + assertThatThrownBy(() -> sut.decode(ctx)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("bool bitmap"); + } + + /// The same guard reached through the validity child, which decodes as its own + /// `vortex.bool` array: a values bitmap long enough for the row count paired with a + /// truncated validity bitmap must fail just as loudly. + @Test + void decode_nullable_validityBitmapTooShort_throws() { + // Given — 9 rows of values (2 bytes) but a 1-byte validity bitmap + MemorySegment bits = MemorySegment.ofArray(new byte[2]); + MemorySegment validBits = MemorySegment.ofArray(new byte[1]); + ArrayNode validityNode = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[0], new int[]{1}); + ArrayNode node = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[]{validityNode}, new int[]{0}); + ReadRegistry registry = TestRegistry.ofDecoders(new BoolEncodingDecoder()); + DecodeContext ctx = new DecodeContext(node, DTypes.BOOL_N, 9, + new MemorySegment[]{bits, validBits}, registry, Arena.ofAuto()); + var sut = new BoolEncodingDecoder(); + + // When / Then + assertThatThrownBy(() -> sut.decode(ctx)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("bool bitmap"); + } + + /// A bitmap longer than the row count needs is legal — trailing padding, or a buffer shared + /// with a longer array — and must not be rejected by the size guard. + @Test + void decode_bitmapLongerThanNeeded_isAccepted() { + // Given — 3 rows (1 byte needed) over an 8-byte buffer with bit 0 set + MemorySegment bits = MemorySegment.ofArray(new byte[]{1, 0, 0, 0, 0, 0, 0, 0}); + ArrayNode node = new ArrayNode(EncodingId.VORTEX_BOOL, null, new ArrayNode[0], new int[]{0}); + ReadRegistry registry = TestRegistry.ofDecoders(new BoolEncodingDecoder()); + DecodeContext ctx = new DecodeContext(node, DTypes.BOOL, 3, new MemorySegment[]{bits}, + registry, Arena.ofAuto()); + var sut = new BoolEncodingDecoder(); + + // When + var result = sut.decode(ctx); + + // Then + assertThat(result.length()).isEqualTo(3); + assertThat(((BoolArray) result).getBoolean(0)).isTrue(); + assertThat(((BoolArray) result).getBoolean(1)).isFalse(); + } + @Test void decode_nullable_allNulls_allRowsInvalid() { // Given — every validity bit is false; values buffer content does not matter diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/ByteBoolEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/ByteBoolEncodingDecoderTest.java index 96f421f4..2b5d9764 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/ByteBoolEncodingDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/ByteBoolEncodingDecoderTest.java @@ -2,10 +2,13 @@ import io.github.dfa1.vortex.reader.ReadRegistry; +import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.reader.array.BoolArray; +import io.github.dfa1.vortex.reader.array.LazyByteBoolArray; import io.github.dfa1.vortex.core.testing.DTypes; import io.github.dfa1.vortex.core.model.EncodingId; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -15,6 +18,7 @@ import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class ByteBoolEncodingDecoderTest { @@ -37,7 +41,7 @@ private static DecodeContext buildCtx(byte[] byteValues) { @ParameterizedTest(name = "{0}") @MethodSource("cases") - void decode_byteBool_packsToBitArray(String name, byte[] input, boolean[] expected) { + void decode_byteBool_readsBytesInPlace(String name, byte[] input, boolean[] expected) { // Given DecodeContext ctx = buildCtx(input); var sut = new ByteBoolEncodingDecoder(); @@ -45,12 +49,33 @@ void decode_byteBool_packsToBitArray(String name, byte[] input, boolean[] expect // When var result = sut.decode(ctx); - // Then - assertThat(result).isInstanceOf(BoolArray.class); + // Then — the concrete type is asserted because the values alone would also pass on the + // eager bit-packing path this replaced + assertThat(result).isInstanceOf(LazyByteBoolArray.class); assertThat(result.length()).isEqualTo(expected.length); BoolArray boolArr = (BoolArray) result; for (int i = 0; i < expected.length; i++) { assertThat(boolArr.getBoolean(i)).as("index %d", i).isEqualTo(expected[i]); } } + + /// The buffer is untrusted and holds one byte per row, so a shorter one is malformed. The + /// eager packing loop faulted on whichever row ran off the end, as a raw + /// `IndexOutOfBoundsException`; the reader must fail as a [VortexException] (ADR 0003), and + /// checking once at decode also keeps the carrier's accessor free of a per-row bound. + @Test + void decode_bufferShorterThanRowCount_throws() { + // Given — 3 bytes of data but 8 declared rows + MemorySegment buf = MemorySegment.ofArray(new byte[]{1, 0, 1}); + ArrayNode node = new ArrayNode(EncodingId.VORTEX_BYTEBOOL, null, new ArrayNode[0], new int[]{0}); + ReadRegistry registry = TestRegistry.ofDecoders(new ByteBoolEncodingDecoder()); + DecodeContext ctx = new DecodeContext(node, DTypes.BOOL, 8, new MemorySegment[]{buf}, registry, + Arena.ofAuto()); + var sut = new ByteBoolEncodingDecoder(); + + // When / Then + assertThatThrownBy(() -> sut.decode(ctx)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("shorter than the 8 declared row(s)"); + } } diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/RleEncodingEncoderTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/RleEncodingEncoderTest.java index d0f8c051..7b3fe37a 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/RleEncodingEncoderTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/RleEncodingEncoderTest.java @@ -315,8 +315,12 @@ void decode_nullableIndices_returnsMaskedArrayWithCorrectValidity() { EncodeResult encoded = ENCODER.encode(dtype, data, EncodeTestHelper.testCtx()); List originalBufs = new ArrayList<>(encoded.buffers()); - MemorySegment validityBuf = MemorySegment.ofArray(new byte[]{0x05}); - originalBufs.add(validityBuf); + // The indices child declares `indices_len` rows, which the encoder pads to a 1024 + // chunk boundary, so its validity bitmap has to cover all 1024 — not just the four + // rows this fixture cares about. Bits 0 and 2 set: rows 0 and 2 valid, 1 and 3 null. + byte[] validityBits = new byte[(1024 + 7) / 8]; + validityBits[0] = 0x05; + originalBufs.add(MemorySegment.ofArray(validityBits)); MemorySegment[] segments = originalBufs.toArray(new MemorySegment[0]); ArrayNode origRoot = toArrayNode(encoded.rootNode());