fix(reader): read vortex.bytebool in place instead of repacking it

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))
 

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                                                            |

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);
+        }
+    }
+}

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.

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)) {

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);
     }
 }

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<Boolean> 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);
+    }
+}