diff --git a/CHANGELOG.md b/CHANGELOG.md index 914ee95e..ccde6a27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `fastlanes.rle` decodes F64/F32 value pools (`LazyRleDoubleArray`, `LazyRleFloatArray`) — files from the Python bindings RLE-encode double columns with long constant runs, which previously failed to scan with `unsupported ptype F64`. ([#209](https://github.com/dfa1/vortex-java/issues/209)) - Lazy dict decode now covers I8/U8/I16/U16 value columns (`DictByteArray`, `DictShortArray`) — files from the Python bindings dict-encode narrow-integer columns, which previously failed to scan with `unsupported ptype for lazy dict`. ([#206](https://github.com/dfa1/vortex-java/issues/206)) - CSV export handles all-null (`DType.Null`) columns as empty fields instead of throwing `unsupported array type: NullArray`. ([#211](https://github.com/dfa1/vortex-java/issues/211)) +- String-dict columns whose values are FSST-compressed no longer fail to scan with `IndexOutOfBoundsException` — the dictionary value offsets are now read at their true ptype width instead of a hardcoded 8-byte stride (uci-magic-gamma-telescope's `class` column decompressed to 4-byte offsets). ([#215](https://github.com/dfa1/vortex-java/issues/215)) - Null rows no longer silently decode as values: wrapper decoders now propagate the row validity that files from the Python bindings carry deep in the encoding tree. Three representations were dropped — a trailing validity child on `fastlanes.bitpacked` (reached through `vortex.alp`/`vortex.zigzag`/`fastlanes.for`, which delegate validity to their encoded child per the Rust `ValidityChild` contract), dict pools with invalid slots, and dict codes with their own validity — across both the eager `vortex.dict` decoder and the lazy dict layout path. Found on real data: penguins and kepler exported invented values (`32.1`, `0.0`) for thousands of null cells. ([#210](https://github.com/dfa1/vortex-java/issues/210)) ### Added diff --git a/integration/src/test/resources/raincloud/expected-status.csv b/integration/src/test/resources/raincloud/expected-status.csv index 337115a9..1a0cb4d5 100644 --- a/integration/src/test/resources/raincloud/expected-status.csv +++ b/integration/src/test/resources/raincloud/expected-status.csv @@ -217,7 +217,7 @@ uci-heart-failure-clinical-records,untriaged uci-human-activity-recognition-using-smartphones,untriaged uci-individual-household-electric-power-consumption,untriaged uci-iris,ok -uci-magic-gamma-telescope,gap:215 +uci-magic-gamma-telescope,ok uci-mushroom,ok uci-online-retail,untriaged uci-online-retail-ii,untriaged diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/array/VarBinArray.java b/reader/src/main/java/io/github/dfa1/vortex/reader/array/VarBinArray.java index ec7b79f4..1c2e6320 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/array/VarBinArray.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/array/VarBinArray.java @@ -326,11 +326,37 @@ private long dictReadCode(long i) { }; } + /// Reads dictionary-value offset `i` at the true width of [#dictValOffPType]. + /// + /// Offsets can arrive at any integer width — FSST decompresses its values to a + /// child with I32 offsets, legacy dicts use I64, and narrow sequence-encoded + /// offsets keep their U8/U16 ptype on the wire. Reading at the wrong width (e.g. + /// an 8-byte read against a 4-byte-stride buffer) walks off the segment. The + /// buffer bounds and ptype are untrusted input, so both are checked here and a + /// [VortexException] is thrown rather than a bare `IndexOutOfBoundsException` + /// (ADR 0003). + /// + /// @param i zero-based offset index (in `[0, dictSize]`) + /// @return the offset value widened to a signed long private long dictReadOff(long i) { - if (dictValOffPType == PType.I32 || dictValOffPType == PType.U32) { - return dictValOffsets.getAtIndex(VortexFormat.LE_INT, i); + int width = dictValOffPType.byteSize(); + long byteOffset = i * width; + if (i < 0 || byteOffset + width > dictValOffsets.byteSize()) { + throw new VortexException("dict value offset index " + i + " (" + dictValOffPType + + ", " + width + "-byte) out of range for offsets segment of " + + dictValOffsets.byteSize() + " bytes"); } - return dictValOffsets.getAtIndex(VortexFormat.LE_LONG, i); + return switch (dictValOffPType) { + case U8 -> Byte.toUnsignedLong(dictValOffsets.get(ValueLayout.JAVA_BYTE, byteOffset)); + case I8 -> dictValOffsets.get(ValueLayout.JAVA_BYTE, byteOffset); + case U16 -> Short.toUnsignedLong(dictValOffsets.get(VortexFormat.LE_SHORT, byteOffset)); + case I16 -> dictValOffsets.get(VortexFormat.LE_SHORT, byteOffset); + case U32 -> Integer.toUnsignedLong(dictValOffsets.get(VortexFormat.LE_INT, byteOffset)); + case I32 -> dictValOffsets.get(VortexFormat.LE_INT, byteOffset); + case I64, U64 -> dictValOffsets.get(VortexFormat.LE_LONG, byteOffset); + default -> throw new VortexException( + "unsupported dict value offset ptype: " + dictValOffPType); + }; } } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DictEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DictEncodingDecoder.java index 7789eb82..63aacf9e 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DictEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DictEncodingDecoder.java @@ -254,8 +254,13 @@ private static Array decodeUtf8DictProto(DecodeContext ctx, MemorySegment metaBu VarBinArray.OffsetMode dictValues = VarBinArray.toOffsetMode(valuesArr, ctx.arena()); BoolArray rowValidity = rowValidity(ctx, codesBuf, codePType, codesValidity, poolValidity, n); + // Carry the offsets ptype that `dictValues` actually materialized. `toOffsetMode` + // only builds fresh I64 offsets on its slow path; on the fast path it returns the + // decoded values array unchanged, keeping its own ptype (e.g. FSST decompresses to + // I32 offsets). Hardcoding I64 here made the DictMode carrier disagree with its + // buffer width (4-byte stride read as 8) and threw IOOBE — see #215. Array dict = VarBinArray.ofDict(ctx.dtype(), n, - dictValues.bytesSegment(), dictValues.offsetsSegment(), PType.I64, + dictValues.bytesSegment(), dictValues.offsetsSegment(), dictValues.offsetsPtype(), codesBuf, codePType); return rowValidity == null ? dict : new MaskedArray(dict, rowValidity); } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/array/VarBinArrayTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/array/VarBinArrayTest.java index c883166c..50705b19 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/array/VarBinArrayTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/array/VarBinArrayTest.java @@ -1,9 +1,12 @@ package io.github.dfa1.vortex.reader.array; +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 org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import java.lang.foreign.MemorySegment; import java.nio.ByteBuffer; @@ -13,6 +16,7 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class VarBinArrayTest { @@ -182,5 +186,72 @@ void forEachByteLength_resolvesViaDict() { assertThat(lengths).containsExactly(3, 1, 3); } + /// The dict-value offsets can arrive at any integer width: FSST-decompressed + /// values carry I32 offsets, legacy dicts use I64, and narrow sequence-encoded + /// offsets keep their U8/U16 ptype. `dictReadOff` must read each at its true + /// stride — reading a 4-byte-stride buffer as 8 bytes was the #215 crash. + @ParameterizedTest + @EnumSource(value = PType.class, names = {"U8", "I8", "U16", "I16", "I32", "U32", "I64", "U64"}) + void getString_resolvesOffsetsAtTrueWidth(PType offsetsPtype) { + // Given — dict=["foo","bar"] with offsets [0,3,6] packed at this ptype's width, + // codes=[1,0,1]. Small offsets fit every width including U8/I8, exercising the + // exact narrow-ptype shape from uci-magic-gamma-telescope's FSST dict. + VarBinArray sut = ofDictWithOffsets(new String[]{"foo", "bar"}, offsetsPtype, new int[]{1, 0, 1}); + + // When + String[] result = {sut.getString(0), sut.getString(1), sut.getString(2)}; + + // Then + assertThat(result).containsExactly("bar", "foo", "bar"); + assertThat(sut.getByteLength(0)).isEqualTo(3); + } + + /// The #215 shape exactly: an I32-width offsets buffer (4-byte stride) mislabeled + /// with an 8-byte ptype. The read must fail loudly with a helpful [VortexException], + /// never a bare `IndexOutOfBoundsException` (ADR 0003 bounds discipline). + @Test + void getString_widthOvershootsBuffer_throwsVortexException() { + // Given — 3 offsets packed as I32 (12-byte buffer) but the carrier claims I64. + // Reading index 1 as 8 bytes needs offset 8..16 against a 12-byte segment. + MemorySegment dictValBytes = MemorySegment.ofArray("foobar".getBytes(StandardCharsets.UTF_8)); + MemorySegment offsetsI32Width = leInts(new int[]{0, 3, 6}); + MemorySegment codes = leInts(new int[]{1}); + VarBinArray sut = VarBinArray.ofDict(UTF8, 1, + dictValBytes, offsetsI32Width, PType.I64, codes, PType.I32); + + // When / Then + assertThatThrownBy(() -> sut.getString(0)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("out of range"); + } + + private static VarBinArray ofDictWithOffsets(String[] dictValues, PType offsetsPtype, int[] codes) { + byte[] allBytes = String.join("", dictValues).getBytes(StandardCharsets.UTF_8); + MemorySegment dictValBytes = MemorySegment.ofArray(allBytes); + + int[] offs = new int[dictValues.length + 1]; + for (int i = 0; i < dictValues.length; i++) { + offs[i + 1] = offs[i] + dictValues[i].getBytes(StandardCharsets.UTF_8).length; + } + MemorySegment dictValOffsets = packOffsets(offs, offsetsPtype); + MemorySegment dictCodes = leInts(codes); + return VarBinArray.ofDict(UTF8, codes.length, + dictValBytes, dictValOffsets, offsetsPtype, dictCodes, PType.I32); + } + + private static MemorySegment packOffsets(int[] offs, PType ptype) { + int width = ptype.byteSize(); + ByteBuffer bb = ByteBuffer.allocate(offs.length * width).order(ByteOrder.LITTLE_ENDIAN); + for (int o : offs) { + switch (width) { + case 1 -> bb.put((byte) o); + case 2 -> bb.putShort((short) o); + case 4 -> bb.putInt(o); + default -> bb.putLong(o); + } + } + return MemorySegment.ofArray(bb.array()); + } + } }