From 3b2840efd9cf7d589ce80c0ea8003c9a0b4a3efb Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Fri, 7 Aug 2026 00:18:45 +0200 Subject: [PATCH 1/2] fix(reader): decode fastlanes.delta into the arena, not four heap long[] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DeltaEncodingDecoder routed the whole column through four row-scaled heap long[] arrays before writing one arena segment: bases and deltas copied out of their segments, a `decoded` array for the reconstruction, and a `result` array whose only job was to drop `offset` leading elements — a second full traversal for a slice. Every value was widened to 8 bytes whatever the column's width, so an I8 delta column allocated 8x its natural size on the GC heap, three times over. That is CLAUDE.md's allocation rule violated four times at row scale. Values are now reconstructed into a single `ctx.arena()` segment at the ptype's real width. The per-chunk scratch stays on the heap, which is what it is for: fixed-size, cache-resident, reused across chunks. readLongs carried both hot-loop anti-patterns at once — an `i % cap` per element and a per-element `switch (ptype)`. Both are hoisted: readElements branch-splits on whether the segment physically holds the range, so the fast path is a uniform modulo-free loop per ptype and the wrap-around stays on the cold path, where only a vortex.constant child reaches it. The write side gets the same treatment, replacing PrimitiveArrays.fromLongs' per-element PTypeIO.set switch. Chunks are independent — each carries its own lane bases — so only those overlapping the requested row window are reconstructed at all. Reading the tail of a long column no longer walks every chunk before it. Fixes three ADR 0003 violations on the way: a row window past the elements the chunks reconstruct reached System.arraycopy as a raw ArrayIndexOutOfBoundsException; a negative deltas_len sized a heap array (NegativeArraySizeException); and an absurd one was truncated by an (int) cast into either a negative size or an OutOfMemoryError. The window is now validated before any child decode, so bogus metadata never drives an allocation, and a legal window over an absurd declared length simply decodes. Coverage: the decoder had three unit tests, all I64, all single-element children, none past one chunk. Adds a Java-write/Java-read round-trip over all eight integer widths across three chunks with full-width random bit patterns (the high bit is where sign- and zero-extension diverge), a test that a non-zero offset window is exactly the slice of the full decode (the writer always emits offset 0, so that path was unreachable from a file), and the malformed-metadata cases above. Closes #338 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 3 + docs/compatibility.md | 2 +- .../JavaRoundTripIntegrationTest.java | 183 +++++++++++++- .../reader/decode/DeltaEncodingDecoder.java | 235 ++++++++++++++---- .../decode/DeltaEncodingDecoderTest.java | 57 +++++ 5 files changed, 423 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4bc84c33..cd723ca56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- 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)) + - A run-end-encoded Utf8/Binary column (`vortex.runend`) no longer expands every run into a fully materialized buffer on decode; rows now resolve through the runs lazily, removing an unbounded `sum(runLength * valueLength)` allocation that a crafted file could drive to `OutOfMemoryError`. ([#334](https://github.com/dfa1/vortex-java/issues/334)) - A `vortex.sequence` column no longer materializes `base + i * multiplier` into a full buffer on decode; rows are computed on access, so the encoding allocates nothing regardless of row count — closing an `OutOfMemoryError` risk from a metadata-only encoding whose row count no buffer bounds. ([#335](https://github.com/dfa1/vortex-java/issues/335)) - A primitive `vortex.dict` column decoded through the encoding path no longer expands its codes into an `n * elemSize` buffer; it now returns the same lazy `DictXxxArray` carriers the layout path already used, so a dict column keeps the dictionary's memory benefit however it is reached. ([#336](https://github.com/dfa1/vortex-java/issues/336)) diff --git a/docs/compatibility.md b/docs/compatibility.md index 2161b4f76..e17b28bda 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -159,7 +159,7 @@ decoder falls into one of three shapes: | `vortex.datetimeparts` | Lazy | Lazy | `LazyDateTimePartsLongArray` — reassembles parts on access | | `vortex.pco` | Materialized | Materialized | range-encoded decompression | | `fastlanes.bitpacked` | Materialized | Materialized | window unpacks bits | -| `fastlanes.delta` | Materialized | Materialized | cumulative sum requires sequential decode | +| `fastlanes.delta` | Materialized | Materialized | cumulative sum requires sequential decode; output is one arena segment at the ptype's width, and only the chunks the row window touches are reconstructed | | `fastlanes.for` | Lazy | Lazy | `LazyForXxxArray` (I8/U8/I16/U16/I32/U32/I64/U64), ADR 0010 + 0015 | | `fastlanes.rle` | Lazy | Lazy | `LazyRleXxxArray`; validity → `OffsetBoolArray`; empty → `LazyConstantXxxArray`, ADR 0015 | | `vortex.patched` | Materialized | Materialized | inner is full base + chunked patches (1024-elem blocks, lane-window-sorted); per-row access requires 2 laneOffsets reads + binary search inside the chunk window, so eager scatter wins for full scans | diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/JavaRoundTripIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/JavaRoundTripIntegrationTest.java index 66988e771..dd06c3c8d 100644 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/JavaRoundTripIntegrationTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/JavaRoundTripIntegrationTest.java @@ -1,34 +1,61 @@ package io.github.dfa1.vortex.integration; +import io.github.dfa1.vortex.core.compute.FastLanes; import io.github.dfa1.vortex.core.model.ColumnName; import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.Editions; +import io.github.dfa1.vortex.core.model.PType; +import io.github.dfa1.vortex.core.model.EncodingId; +import io.github.dfa1.vortex.core.proto.ProtoDeltaMetadata; +import io.github.dfa1.vortex.inspect.InspectorTree; import io.github.dfa1.vortex.reader.ReadRegistry; import io.github.dfa1.vortex.reader.ScanOptions; import io.github.dfa1.vortex.reader.VortexReader; +import io.github.dfa1.vortex.reader.array.Array; +import io.github.dfa1.vortex.reader.array.ByteArray; import io.github.dfa1.vortex.reader.array.IntArray; +import io.github.dfa1.vortex.reader.array.LongArray; +import io.github.dfa1.vortex.reader.array.ShortArray; +import io.github.dfa1.vortex.reader.decode.ArrayNode; +import io.github.dfa1.vortex.reader.decode.DecodeContext; +import io.github.dfa1.vortex.reader.decode.DeltaEncodingDecoder; import io.github.dfa1.vortex.writer.VortexWriter; import io.github.dfa1.vortex.writer.WriteOptions; +import io.github.dfa1.vortex.writer.WriteRegistry; +import io.github.dfa1.vortex.writer.encode.EncodeContext; +import io.github.dfa1.vortex.writer.encode.EncodeResult; +import io.github.dfa1.vortex.writer.encode.DeltaEncodingEncoder; import io.github.dfa1.vortex.writer.encode.PatchedEncodingEncoder; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import java.io.IOException; +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; import java.nio.channels.FileChannel; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.util.Arrays; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Random; import static org.assertj.core.api.Assertions.assertThat; -/// Java writer → Java reader round-trips for encodings the bundled `vortex-jni` build cannot read -/// back, so they have no Java→Rust coverage. This is still a real cross-module integration test: it -/// drives the writer's encode, the on-disk file format, and the reader's decode end to end. +/// Java writer → Java reader round-trips for encodings whose Java *decode* has no other end-to-end +/// cover. This is a real cross-module integration test either way: it drives the writer's encode, +/// the on-disk file format, and the reader's decode end to end. /// -/// `vortex.patched` is the case here — the JNI reader rejects a standalone patched array with -/// "Unknown encoding: vortex.patched", so the round-trip is asserted on the Java side instead. +/// Two reasons land a case here: +/// - the bundled `vortex-jni` build cannot read the encoding back, so there is no Java→Rust test. +/// `vortex.patched` is this case — the JNI reader rejects a standalone patched array with +/// "Unknown encoding: vortex.patched". +/// - Java→Rust cover exists but only exercises the *encoder*. `fastlanes.delta` is this case: +/// `JavaWritesRustReadsIntegrationTest#javaWriter_rustReader_delta_i64` proves what Java writes +/// is readable, and says nothing about `DeltaEncodingDecoder`. class JavaRoundTripIntegrationTest { private static final DType.Struct I32_SCHEMA = new DType.Struct( @@ -62,6 +89,152 @@ void patched_i32_javaWriteJavaRead(@TempDir Path tmp) throws IOException { assertThat(decoded).containsExactly(data); } + /// `fastlanes.delta` decode across every width it accepts, over three FastLanes chunks. + /// + /// The unit tests reach the decoder only with I64 and single-element (constant) children, so + /// nothing covered the per-width read and write paths, and nothing covered more than one + /// chunk — which is where the chunk-window arithmetic lives. Values are full-width random + /// bit patterns, not a monotonic ramp: the high bit is exactly where a read that + /// sign-extends and one that zero-extends diverge, and delta round-trips any values at all + /// since encode and decode both wrap modulo the type width. + @ParameterizedTest + @EnumSource(value = PType.class, names = {"I8", "I16", "I32", "I64", "U8", "U16", "U32", "U64"}) + void delta_javaWriteJavaRead(PType ptype, @TempDir Path tmp) throws IOException { + // Given — 2500 rows is three 1024-element chunks, the last one padded. + long mask = FastLanes.lowMask(ptype.bits()); + Random rng = new Random(338); + long[] expected = new long[2500]; + for (int i = 0; i < expected.length; i++) { + expected[i] = rng.nextLong() & mask; + } + DType.Struct schema = new DType.Struct(List.of(ColumnName.of("v")), + List.of(new DType.Primitive(ptype, false)), false); + Path file = tmp.resolve("java_delta_" + ptype + ".vtx"); + + // When + try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE); + var sut = VortexWriter.create(ch, schema, + WriteOptions.defaults().withEdition(Editions.UNSTABLE_2025_05_0), + List.of(new DeltaEncodingEncoder()))) { + sut.writeChunk(Map.of(ColumnName.of("v"), narrow(expected, ptype))); + } + + // Then — the encoding is asserted too, so a writer that quietly stopped choosing delta + // would fail here rather than leave the decoder untested + try (var reader = VortexReader.open(file, ReadRegistry.loadAll())) { + assertThat(InspectorTree.build(reader).usedEncodings()).contains("fastlanes.delta"); + } + // compared as stored bit patterns, so signed and unsigned widths assert alike + assertThat(readColumnBits(file, "v", mask)).containsExactly(expected); + } + + /// `fastlanes.delta`'s `offset` metadata — which makes a decode start partway into the + /// reconstructed elements — has no round-trip cover, because the Java writer always emits 0; + /// a non-zero offset only ever arrives on a Rust-written sliced array. So this drives the + /// decoder directly over encoder-produced children instead of through a file, and asserts + /// the window is exactly the corresponding slice of the full decode. The window arithmetic + /// (which chunks to reconstruct, and where each lands in the output) is the part of decode + /// that only a non-zero offset reaches. + @Test + void delta_offsetWindowIsTheSliceOfTheFullDecode() { + // Given — 2500 rows, so the encoder pads to three chunks + DType dtype = new DType.Primitive(PType.I64, false); + Random rng = new Random(3381); + long[] data = new long[2500]; + for (int i = 0; i < data.length; i++) { + data[i] = rng.nextLong(); + } + try (Arena arena = Arena.ofConfined()) { + EncodeResult encoded = new DeltaEncodingEncoder().encode(dtype, data, + EncodeContext.of(arena, WriteRegistry.builder().registerDefaults().build())); + long padded = 3L * FastLanes.CHUNK; + long[] full = decodeDelta(encoded, dtype, padded, 0, padded, arena); + + // When — a window opening inside chunk 0 and closing inside chunk 2 + long[] result = decodeDelta(encoded, dtype, padded, 700, 1500, arena); + + // Then + assertThat(result).containsExactly(Arrays.copyOfRange(full, 700, 2200)); + } + } + + /// Decodes `encoded` as a `fastlanes.delta` array over the given window, bypassing the file + /// format so the `offset` the writer never emits can be set. + /// + /// @param encoded the encoder's output (bases buffer, deltas buffer) + /// @param dtype logical element type + /// @param deltasLen number of reconstructed elements the chunks cover + /// @param offset absolute index the first returned row maps to + /// @param rowCount number of rows to decode + /// @param arena allocator for the decoded segment + /// @return the decoded values + private static long[] decodeDelta(EncodeResult encoded, DType dtype, long deltasLen, + int offset, long rowCount, Arena arena) { + MemorySegment meta = MemorySegment.ofArray(new ProtoDeltaMetadata(deltasLen, offset).encode()); + ArrayNode bases = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{0}); + ArrayNode deltas = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{1}); + ArrayNode node = new ArrayNode(EncodingId.FASTLANES_DELTA, meta, + new ArrayNode[]{bases, deltas}, new int[0]); + DecodeContext ctx = new DecodeContext(node, dtype, rowCount, + encoded.buffers().toArray(new MemorySegment[0]), ReadRegistry.loadAll(), arena); + LongArray decoded = (LongArray) new DeltaEncodingDecoder().decode(ctx); + long[] out = new long[(int) decoded.length()]; + for (int i = 0; i < out.length; i++) { + out[i] = decoded.getLong(i); + } + return out; + } + + /// Narrows logical values to the Java array type the writer expects for `ptype`. + private static Object narrow(long[] values, PType ptype) { + return switch (ptype) { + case I8, U8 -> { + byte[] out = new byte[values.length]; + for (int i = 0; i < values.length; i++) { + out[i] = (byte) values[i]; + } + yield out; + } + case I16, U16 -> { + short[] out = new short[values.length]; + for (int i = 0; i < values.length; i++) { + out[i] = (short) values[i]; + } + yield out; + } + case I32, U32 -> { + int[] out = new int[values.length]; + for (int i = 0; i < values.length; i++) { + out[i] = (int) values[i]; + } + yield out; + } + default -> values.clone(); + }; + } + + /// Reads a primitive column back as raw bit patterns, masked to the type's width so a + /// sign-extending accessor and a zero-extending one compare equal. + private static long[] readColumnBits(Path file, String column, long mask) throws IOException { + var out = new ArrayList(); + try (var vf = VortexReader.open(file, ReadRegistry.loadAll()); + var iter = vf.scan(ScanOptions.columns(column))) { + iter.forEachRemaining(c -> { + Array arr = c.column(column); + for (long i = 0; i < arr.length(); i++) { + out.add(switch (arr) { + case ByteArray a -> a.getByte(i) & mask; + case ShortArray a -> a.getShort(i) & mask; + case IntArray a -> a.getInt(i) & mask; + case LongArray a -> a.getLong(i) & mask; + default -> throw new IllegalStateException("unexpected array " + arr.getClass()); + }); + } + }); + } + return out.stream().mapToLong(Long::longValue).toArray(); + } + @SuppressWarnings("SameParameterValue") private static int[] readIntColumn(Path file, String column) throws IOException { try (var vf = VortexReader.open(file, ReadRegistry.loadAll()); diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoder.java index af5ac5810..1f29fcad6 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoder.java @@ -5,7 +5,6 @@ import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.model.EncodingId; import io.github.dfa1.vortex.core.compute.FastLanes; -import io.github.dfa1.vortex.core.compute.PrimitiveArrays; import io.github.dfa1.vortex.core.io.VortexFormat; import io.github.dfa1.vortex.core.proto.ProtoDeltaMetadata; import io.github.dfa1.vortex.reader.array.Array; @@ -19,6 +18,12 @@ import java.lang.foreign.ValueLayout; /// Read-only decoder for `fastlanes.delta`. +/// +/// Delta is one of the few encodings that genuinely has to reconstruct values — a row is a +/// prefix sum along its lane — so there is no lazy carrier here. What decode does avoid is +/// doing that reconstruction on the heap: the only row-scaled buffer is the output segment, +/// allocated from `ctx.arena()` at the ptype's real width. The per-chunk scratch is fixed-size +/// ([FastLanes#CHUNK] elements, cache-resident) and reused across chunks. public final class DeltaEncodingDecoder implements EncodingDecoder { @Override @@ -49,54 +54,73 @@ public Array decode(DecodeContext ctx) { long deltasLen = meta.deltas_len(); int offset = meta.offset(); - if (deltasLen == 0L) { - MemorySegment empty = ctx.arena().allocate(0); - return switch (ptype) { - case I64, U64 -> new MaterializedLongArray(ctx.dtype(), 0L, empty); - case I32, U32 -> new MaterializedIntArray(ctx.dtype(), 0L, empty); - case I16, U16 -> new MaterializedShortArray(ctx.dtype(), 0L, empty); - case I8, U8 -> new MaterializedByteArray(ctx.dtype(), 0L, empty); - default -> throw new VortexException(EncodingId.FASTLANES_DELTA, "unsupported ptype: " + ptype); - }; + if (deltasLen == 0L || rowCount == 0L) { + return array(ctx, ptype, 0L, ctx.arena().allocate(0)); } - long basesLen = (deltasLen / FastLanes.CHUNK) * lanes; - DType dtype = ctx.dtype(); + // Rows come from the window `[offset, offset + rowCount)` of the `deltasLen` elements + // the chunks reconstruct. Both bounds are untrusted metadata: a negative offset or a + // window past the end used to surface as a raw ArrayIndexOutOfBoundsException from the + // final arraycopy, and an absurd `deltasLen` sized a heap array before anything checked + // it — NegativeArraySizeException or OutOfMemoryError, neither a VortexException + // (ADR 0003). Checked before any child decode, so a bogus length never drives an + // allocation. + if (offset < 0 || rowCount > deltasLen - offset) { + throw new VortexException(EncodingId.FASTLANES_DELTA, + "row window [" + offset + ", " + (offset + rowCount) + ") outside the " + + deltasLen + " delta element(s)"); + } - long[] basesAll = readLongs(ctx.decodeChildSegment(0, dtype, basesLen), (int) basesLen, ptype); - long[] deltasAll = readLongs(ctx.decodeChildSegment(1, dtype, deltasLen), (int) deltasLen, ptype); + DType dtype = ctx.dtype(); + long basesLen = (deltasLen / FastLanes.CHUNK) * lanes; + MemorySegment basesSeg = ctx.decodeChildSegment(0, dtype, basesLen); + MemorySegment deltasSeg = ctx.decodeChildSegment(1, dtype, deltasLen); + int elemBytes = ptype.byteSize(); + long basesCap = SegmentBroadcast.capacity(basesSeg, elemBytes); + long deltasCap = SegmentBroadcast.capacity(deltasSeg, elemBytes); - int numChunks = (int) (deltasLen / FastLanes.CHUNK); - long[] decoded = new long[(int) deltasLen]; - long[] untransposedChunk = new long[FastLanes.CHUNK]; + MemorySegment out = ctx.arena().allocate(rowCount * elemBytes); long[] chunkBases = new long[lanes]; long[] chunkDeltas = new long[FastLanes.CHUNK]; long[] chunkUndelta = new long[FastLanes.CHUNK]; + long[] untransposed = new long[FastLanes.CHUNK]; - for (int chunk = 0; chunk < numChunks; chunk++) { - int basesOff = chunk * lanes; - int deltaOff = chunk * FastLanes.CHUNK; - - System.arraycopy(basesAll, basesOff, chunkBases, 0, lanes); - System.arraycopy(deltasAll, deltaOff, chunkDeltas, 0, FastLanes.CHUNK); - + // Each chunk carries its own lane bases, so chunks are independent and only those + // overlapping the requested window are reconstructed — reading the tail of a long + // column no longer walks every chunk before it. + long numChunks = deltasLen / FastLanes.CHUNK; + long firstChunk = offset / FastLanes.CHUNK; + long lastChunk = Math.min(numChunks - 1, (offset + rowCount - 1) / FastLanes.CHUNK); + for (long chunk = firstChunk; chunk <= lastChunk; chunk++) { + readElements(basesSeg, ptype, basesCap, chunk * lanes, lanes, chunkBases); + readElements(deltasSeg, ptype, deltasCap, chunk * FastLanes.CHUNK, FastLanes.CHUNK, chunkDeltas); undeltaChunk(chunkDeltas, chunkBases, lanes, typeBits, mask, chunkUndelta); - for (int i = 0; i < FastLanes.CHUNK; i++) { - untransposedChunk[FastLanes.transposeIndex(i)] = chunkUndelta[i]; + untransposed[FastLanes.transposeIndex(i)] = chunkUndelta[i]; } - System.arraycopy(untransposedChunk, 0, decoded, deltaOff, FastLanes.CHUNK); + // The window clips only the first and last chunk; the ones between copy whole. + long chunkStart = chunk * FastLanes.CHUNK; + int from = (int) Math.max(0, offset - chunkStart); + int to = (int) Math.min(FastLanes.CHUNK, offset + rowCount - chunkStart); + writeElements(out, ptype, chunkStart + from - offset, untransposed, from, to - from); } + return array(ctx, ptype, rowCount, out); + } - long[] result = new long[(int) rowCount]; - System.arraycopy(decoded, offset, result, 0, (int) rowCount); - - MemorySegment seg = PrimitiveArrays.fromLongs(result, ptype, ctx.arena()); + /// Wraps a decoded segment in the `Materialized*Array` matching `ptype`. + /// + /// @param ctx decode context, source of the logical dtype + /// @param ptype physical type of the values in `seg` + /// @param length row count + /// @param seg the decoded values, little-endian at `ptype`'s width + /// @return the typed array view over `seg` + /// @throws VortexException if `ptype` is not an integer ptype + private static Array array(DecodeContext ctx, PType ptype, long length, MemorySegment seg) { return switch (ptype) { - case I64, U64 -> new MaterializedLongArray(ctx.dtype(), rowCount, seg); - case I32, U32 -> new MaterializedIntArray(ctx.dtype(), rowCount, seg); - case I16, U16 -> new MaterializedShortArray(ctx.dtype(), rowCount, seg); - case I8, U8 -> new MaterializedByteArray(ctx.dtype(), rowCount, seg); + case I64, U64 -> new MaterializedLongArray(ctx.dtype(), length, seg); + case I32, U32 -> new MaterializedIntArray(ctx.dtype(), length, seg); + case I16, U16 -> new MaterializedShortArray(ctx.dtype(), length, seg); + case I8, U8 -> new MaterializedByteArray(ctx.dtype(), length, seg); default -> throw new VortexException(EncodingId.FASTLANES_DELTA, "unsupported ptype: " + ptype); }; } @@ -113,24 +137,133 @@ private static void undeltaChunk(long[] deltas, long[] bases, int lanes, int typ } } - private static long[] readLongs(MemorySegment buf, int count, PType ptype) { - long[] out = new long[count]; - int elemSize = ptype.byteSize(); - long cap = SegmentBroadcast.capacity(buf, elemSize); + /// Reads `count` consecutive elements starting at logical index `firstIdx`, widened to + /// `long`. + /// + /// Branch-split on whether the segment physically holds the range (CLAUDE.md hot-loop + /// rule): the fast path is a uniform, modulo-free loop per ptype, and the wrap-around + /// arithmetic stays on the cold path, where it is only ever reached by a + /// `vortex.constant` child that stores one element for the whole array. + /// + /// @param buf the child segment + /// @param ptype element type + /// @param cap elements physically present in `buf` + /// @param firstIdx logical index of the first element to read + /// @param count number of elements to read + /// @param out destination scratch, at least `count` long + /// @throws VortexException if `buf` holds no elements at all + private static void readElements(MemorySegment buf, PType ptype, long cap, long firstIdx, + int count, long[] out) { + if (firstIdx + count <= cap) { + readContiguous(buf, ptype, firstIdx * ptype.byteSize(), count, out); + return; + } + if (cap == 0) { + throw new VortexException(EncodingId.FASTLANES_DELTA, + "empty child segment for " + count + " element(s) of " + ptype); + } + readBroadcast(buf, ptype, cap, firstIdx, count, out); + } + + private static void readContiguous(MemorySegment buf, PType ptype, long base, int count, long[] out) { + switch (ptype) { + case I8 -> { + for (int i = 0; i < count; i++) { + out[i] = buf.get(ValueLayout.JAVA_BYTE, base + i); + } + } + case U8 -> { + for (int i = 0; i < count; i++) { + out[i] = Byte.toUnsignedLong(buf.get(ValueLayout.JAVA_BYTE, base + i)); + } + } + case I16 -> { + for (int i = 0; i < count; i++) { + out[i] = buf.get(VortexFormat.LE_SHORT, base + i * 2L); + } + } + case U16 -> { + for (int i = 0; i < count; i++) { + out[i] = Short.toUnsignedLong(buf.get(VortexFormat.LE_SHORT, base + i * 2L)); + } + } + case I32 -> { + for (int i = 0; i < count; i++) { + out[i] = buf.get(VortexFormat.LE_INT, base + i * 4L); + } + } + case U32 -> { + for (int i = 0; i < count; i++) { + out[i] = Integer.toUnsignedLong(buf.get(VortexFormat.LE_INT, base + i * 4L)); + } + } + case I64, U64 -> { + for (int i = 0; i < count; i++) { + out[i] = buf.get(VortexFormat.LE_LONG, base + i * 8L); + } + } + default -> throw new VortexException(EncodingId.FASTLANES_DELTA, "unsupported ptype: " + ptype); + } + } + + /// Cold path of [#readElements]: the child holds fewer elements than the range asks for, + /// which only a `vortex.constant` child does, so this wraps around it one element at a time. + private static void readBroadcast(MemorySegment buf, PType ptype, long cap, long firstIdx, + int count, long[] out) { + int elemBytes = ptype.byteSize(); for (int i = 0; i < count; i++) { - long off = (i % cap) * elemSize; - out[i] = switch (ptype) { - case I8 -> buf.get(ValueLayout.JAVA_BYTE, off); - case U8 -> Byte.toUnsignedLong(buf.get(ValueLayout.JAVA_BYTE, off)); - case I16 -> buf.get(VortexFormat.LE_SHORT, off); - case U16 -> Short.toUnsignedLong(buf.get(VortexFormat.LE_SHORT, off)); - case I32 -> buf.get(VortexFormat.LE_INT, off); - case U32 -> Integer.toUnsignedLong(buf.get(VortexFormat.LE_INT, off)); - case I64, U64 -> buf.get(VortexFormat.LE_LONG, off); - default -> throw new VortexException(EncodingId.FASTLANES_DELTA, "unsupported ptype: " + ptype); - }; + out[i] = readOne(buf, ptype, ((firstIdx + i) % cap) * elemBytes); + } + } + + private static long readOne(MemorySegment buf, PType ptype, long off) { + return switch (ptype) { + case I8 -> buf.get(ValueLayout.JAVA_BYTE, off); + case U8 -> Byte.toUnsignedLong(buf.get(ValueLayout.JAVA_BYTE, off)); + case I16 -> buf.get(VortexFormat.LE_SHORT, off); + case U16 -> Short.toUnsignedLong(buf.get(VortexFormat.LE_SHORT, off)); + case I32 -> buf.get(VortexFormat.LE_INT, off); + case U32 -> Integer.toUnsignedLong(buf.get(VortexFormat.LE_INT, off)); + case I64, U64 -> buf.get(VortexFormat.LE_LONG, off); + default -> throw new VortexException(EncodingId.FASTLANES_DELTA, "unsupported ptype: " + ptype); + }; + } + + /// Writes `src[from, from + count)` into `out` at element index `dstIdx`, narrowed to + /// `ptype`'s width. The ptype switch is hoisted out of the loop so each body stays uniform. + /// + /// @param out destination segment, at `ptype`'s width + /// @param ptype element type + /// @param dstIdx element index in `out` to write the first value at + /// @param src reconstructed values, widened to `long` + /// @param from first index in `src` to write + /// @param count number of elements to write + /// @throws VortexException if `ptype` is not an integer ptype + private static void writeElements(MemorySegment out, PType ptype, long dstIdx, long[] src, + int from, int count) { + switch (ptype) { + case I8, U8 -> { + for (int i = 0; i < count; i++) { + out.set(ValueLayout.JAVA_BYTE, dstIdx + i, (byte) src[from + i]); + } + } + case I16, U16 -> { + for (int i = 0; i < count; i++) { + out.setAtIndex(VortexFormat.LE_SHORT, dstIdx + i, (short) src[from + i]); + } + } + case I32, U32 -> { + for (int i = 0; i < count; i++) { + out.setAtIndex(VortexFormat.LE_INT, dstIdx + i, (int) src[from + i]); + } + } + case I64, U64 -> { + for (int i = 0; i < count; i++) { + out.setAtIndex(VortexFormat.LE_LONG, dstIdx + i, src[from + i]); + } + } + default -> throw new VortexException(EncodingId.FASTLANES_DELTA, "unsupported ptype: " + ptype); } - return out; } } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.java index 1fd7b1ea7..55a1f8774 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.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.DType; import io.github.dfa1.vortex.core.model.PType; import io.github.dfa1.vortex.core.model.EncodingId; @@ -11,12 +12,14 @@ import io.github.dfa1.vortex.reader.array.LongArray; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.EnumSource; import java.lang.foreign.Arena; import java.lang.foreign.MemorySegment; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class DeltaEncodingDecoderTest { @@ -76,6 +79,47 @@ void decode_constantChildren_broadcastsAcrossChunk() { } } + /// `deltas_len` and `offset` are untrusted metadata that together name the window of + /// reconstructed elements the rows come from. A window running past that many elements + /// reached a `System.arraycopy` as a raw ArrayIndexOutOfBoundsException, and a negative + /// `deltas_len` sized a heap `long[]` — a NegativeArraySizeException. Both must be a + /// VortexException (ADR 0003). + @ParameterizedTest + @CsvSource({ + "1024, 0, 2000", // more rows than the chunks reconstruct + "1024, 900, 200", // window starts inside the chunk but runs off its end + "1024, -1, 4", // negative start position + "-1024, 0, 4" // negative element count + }) + void decode_windowOutsideDeltas_throws(long deltasLen, int offset, long rowCount) { + // Given — metadata whose row window is not covered by `deltasLen` elements + DecodeContext ctx = deltaContext(deltasLen, offset, rowCount); + + // When / Then + assertThatThrownBy(() -> SUT.decode(ctx)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("row window"); + } + + /// The other end of the same untrusted field: a `deltas_len` of `Long.MAX_VALUE` with a few + /// rows asked for is a *legal* window — the rows are inside it — over an absurd declared + /// length. It used to allocate `new long[(int) deltasLen]` twice before touching a buffer + /// (OutOfMemoryError, or a silently truncated cast). Nothing is sized from `deltasLen` any + /// more, and only the chunks the window touches are reconstructed, so this now decodes. + @Test + void decode_absurdDeltasLenButSmallWindow_decodesWithoutAllocatingForIt() { + // Given + DecodeContext ctx = deltaContext(Long.MAX_VALUE, 0, 4); + + // When + LongArray result = (LongArray) SUT.decode(ctx); + + // Then — zero bases and zero deltas, so every row is zero + assertThat(result.length()).isEqualTo(4); + assertThat(result.getLong(0)).isZero(); + assertThat(result.getLong(3)).isZero(); + } + @Test void decode_constantBases_nonZeroOffsetAndBase() { // Given a constant base of 5 broadcast across all 16 lanes with zero deltas: @@ -99,4 +143,17 @@ void decode_constantBases_nonZeroOffsetAndBase() { MemorySegment seg = result.materialize(Arena.ofAuto()); assertThat(seg.get(VortexFormat.LE_LONG, 0)).isEqualTo(5L); } + + /// An I64 delta node over single-element (constant) bases and deltas children, so the + /// metadata under test is the only variable. + private static DecodeContext deltaContext(long deltasLen, int offset, long rowCount) { + MemorySegment meta = MemorySegment.ofArray(new ProtoDeltaMetadata(deltasLen, offset).encode()); + ArrayNode bases = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{0}); + ArrayNode deltas = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{1}); + ArrayNode node = new ArrayNode(EncodingId.FASTLANES_DELTA, meta, + new ArrayNode[]{bases, deltas}, new int[0]); + MemorySegment[] segs = {TestSegments.leLongs(0L), TestSegments.leLongs(0L)}; + return new DecodeContext(node, new DType.Primitive(PType.I64, false), rowCount, + segs, REGISTRY, Arena.ofAuto()); + } } From ea7b933de062f58d44f43227e33f3a7e91277f97 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Fri, 7 Aug 2026 07:22:44 +0200 Subject: [PATCH 2/2] fix(reader): reject a negative delta element count on its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review follow-up on #338. `rowCount > deltasLen - offset` was the only thing standing between a negative `deltas_len` and the chunk loop, and the subtraction wraps: `Long.MIN_VALUE - 1` is positive, so the window check passed, the chunk range came out empty, and decode handed back a zero-filled array of the requested length. A malformed file answered instead of rejected — no raw exception, so ADR 0003 held, but the file is still garbage and should say so. Co-Authored-By: Claude Opus 5 --- .../dfa1/vortex/reader/decode/DeltaEncodingDecoder.java | 6 +++++- .../dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.java | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoder.java index 1f29fcad6..91f3437d3 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoder.java @@ -65,7 +65,11 @@ public Array decode(DecodeContext ctx) { // it — NegativeArraySizeException or OutOfMemoryError, neither a VortexException // (ADR 0003). Checked before any child decode, so a bogus length never drives an // allocation. - if (offset < 0 || rowCount > deltasLen - offset) { + // `deltasLen < 0` is checked on its own rather than left to the subtraction: a + // sufficiently negative one makes `deltasLen - offset` wrap positive, which passes a + // window check it should fail, and the chunk loop then simply does nothing and hands + // back a zero-filled array — a malformed file answered instead of rejected. + if (offset < 0 || deltasLen < 0 || rowCount > deltasLen - offset) { throw new VortexException(EncodingId.FASTLANES_DELTA, "row window [" + offset + ", " + (offset + rowCount) + ") outside the " + deltasLen + " delta element(s)"); diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.java index 55a1f8774..bfb0de9ce 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DeltaEncodingDecoderTest.java @@ -89,7 +89,8 @@ void decode_constantChildren_broadcastsAcrossChunk() { "1024, 0, 2000", // more rows than the chunks reconstruct "1024, 900, 200", // window starts inside the chunk but runs off its end "1024, -1, 4", // negative start position - "-1024, 0, 4" // negative element count + "-1024, 0, 4", // negative element count + "-9223372036854775808, 1, 4" // negative enough that `deltasLen - offset` wraps positive }) void decode_windowOutsideDeltas_throws(long deltasLen, int offset, long rowCount) { // Given — metadata whose row window is not covered by `deltasLen` elements