Skip to content

Commit 7d3e6c5

Browse files
dfa1claude
andcommitted
fix: read string-dict offsets at their true ptype width (#215)
A `vortex.dict` string column whose values are FSST-compressed crashed the scan with a raw `IndexOutOfBoundsException` in `VarBinArray$DictMode.dictReadOff`. Root cause: `DictEncodingDecoder.decodeUtf8DictProto` ran the decoded values through `VarBinArray.toOffsetMode` and then hardcoded `PType.I64` as the offsets ptype of the resulting `DictMode`. But `toOffsetMode` only builds fresh I64 offsets on its slow path — on the fast path it returns the already-decoded `OffsetMode` unchanged, keeping its own ptype. FSST decompresses its values to a child with I32 offsets (Rust: FSST's `codes_offsets_ptype` is a narrow wire ptype read during decompression — see encodings/fsst/src/array.rs — while the canonicalized value offsets are freshly built and independent of it). The DictMode carrier thus claimed an 8-byte stride over a 4-byte-stride buffer (12 bytes for 3 offsets), so reading offset 1 as an 8-byte long walked off the segment. Fix: carry `dictValues.offsetsPtype()` so the carrier matches the buffer it actually materialized (parse-don't-validate). Additionally harden `dictReadOff` to read every signed/unsigned offset width at its true stride and to throw a helpful `VortexException` on out-of-range indices rather than a bare `IndexOutOfBoundsException` (ADR 0003 bounds discipline). Corpus evidence: uci-magic-gamma-telescope (Raincloud, vortex-data 0.69.0) — its utf8 `class` column is a `vortex.dict` over `vortex.fsst` values. Export now completes and matches the Parquet sibling on all 19020 rows x 11 cols. Closes #215 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 26df9ab commit 7d3e6c5

5 files changed

Lines changed: 108 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- `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))
1414
- 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))
1515
- 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))
16+
- 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))
1617
- 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))
1718

1819
### Added

integration/src/test/resources/raincloud/expected-status.csv

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ uci-heart-failure-clinical-records,untriaged
217217
uci-human-activity-recognition-using-smartphones,untriaged
218218
uci-individual-household-electric-power-consumption,untriaged
219219
uci-iris,ok
220-
uci-magic-gamma-telescope,gap:215
220+
uci-magic-gamma-telescope,ok
221221
uci-mushroom,ok
222222
uci-online-retail,untriaged
223223
uci-online-retail-ii,untriaged

reader/src/main/java/io/github/dfa1/vortex/reader/array/VarBinArray.java

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,37 @@ private long dictReadCode(long i) {
326326
};
327327
}
328328

329+
/// Reads dictionary-value offset `i` at the true width of [#dictValOffPType].
330+
///
331+
/// Offsets can arrive at any integer width — FSST decompresses its values to a
332+
/// child with I32 offsets, legacy dicts use I64, and narrow sequence-encoded
333+
/// offsets keep their U8/U16 ptype on the wire. Reading at the wrong width (e.g.
334+
/// an 8-byte read against a 4-byte-stride buffer) walks off the segment. The
335+
/// buffer bounds and ptype are untrusted input, so both are checked here and a
336+
/// [VortexException] is thrown rather than a bare `IndexOutOfBoundsException`
337+
/// (ADR 0003).
338+
///
339+
/// @param i zero-based offset index (in `[0, dictSize]`)
340+
/// @return the offset value widened to a signed long
329341
private long dictReadOff(long i) {
330-
if (dictValOffPType == PType.I32 || dictValOffPType == PType.U32) {
331-
return dictValOffsets.getAtIndex(VortexFormat.LE_INT, i);
342+
int width = dictValOffPType.byteSize();
343+
long byteOffset = i * width;
344+
if (i < 0 || byteOffset + width > dictValOffsets.byteSize()) {
345+
throw new VortexException("dict value offset index " + i + " (" + dictValOffPType
346+
+ ", " + width + "-byte) out of range for offsets segment of "
347+
+ dictValOffsets.byteSize() + " bytes");
332348
}
333-
return dictValOffsets.getAtIndex(VortexFormat.LE_LONG, i);
349+
return switch (dictValOffPType) {
350+
case U8 -> Byte.toUnsignedLong(dictValOffsets.get(ValueLayout.JAVA_BYTE, byteOffset));
351+
case I8 -> dictValOffsets.get(ValueLayout.JAVA_BYTE, byteOffset);
352+
case U16 -> Short.toUnsignedLong(dictValOffsets.get(VortexFormat.LE_SHORT, byteOffset));
353+
case I16 -> dictValOffsets.get(VortexFormat.LE_SHORT, byteOffset);
354+
case U32 -> Integer.toUnsignedLong(dictValOffsets.get(VortexFormat.LE_INT, byteOffset));
355+
case I32 -> dictValOffsets.get(VortexFormat.LE_INT, byteOffset);
356+
case I64, U64 -> dictValOffsets.get(VortexFormat.LE_LONG, byteOffset);
357+
default -> throw new VortexException(
358+
"unsupported dict value offset ptype: " + dictValOffPType);
359+
};
334360
}
335361
}
336362

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,13 @@ private static Array decodeUtf8DictProto(DecodeContext ctx, MemorySegment metaBu
254254
VarBinArray.OffsetMode dictValues = VarBinArray.toOffsetMode(valuesArr, ctx.arena());
255255

256256
BoolArray rowValidity = rowValidity(ctx, codesBuf, codePType, codesValidity, poolValidity, n);
257+
// Carry the offsets ptype that `dictValues` actually materialized. `toOffsetMode`
258+
// only builds fresh I64 offsets on its slow path; on the fast path it returns the
259+
// decoded values array unchanged, keeping its own ptype (e.g. FSST decompresses to
260+
// I32 offsets). Hardcoding I64 here made the DictMode carrier disagree with its
261+
// buffer width (4-byte stride read as 8) and threw IOOBE — see #215.
257262
Array dict = VarBinArray.ofDict(ctx.dtype(), n,
258-
dictValues.bytesSegment(), dictValues.offsetsSegment(), PType.I64,
263+
dictValues.bytesSegment(), dictValues.offsetsSegment(), dictValues.offsetsPtype(),
259264
codesBuf, codePType);
260265
return rowValidity == null ? dict : new MaskedArray(dict, rowValidity);
261266
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package io.github.dfa1.vortex.reader.array;
22

3+
import io.github.dfa1.vortex.core.error.VortexException;
34
import io.github.dfa1.vortex.core.model.DType;
45
import io.github.dfa1.vortex.core.model.PType;
56
import org.junit.jupiter.api.Nested;
67
import org.junit.jupiter.api.Test;
8+
import org.junit.jupiter.params.ParameterizedTest;
9+
import org.junit.jupiter.params.provider.EnumSource;
710

811
import java.lang.foreign.MemorySegment;
912
import java.nio.ByteBuffer;
@@ -13,6 +16,7 @@
1316
import java.util.List;
1417

1518
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1620

1721
class VarBinArrayTest {
1822

@@ -182,5 +186,72 @@ void forEachByteLength_resolvesViaDict() {
182186
assertThat(lengths).containsExactly(3, 1, 3);
183187
}
184188

189+
/// The dict-value offsets can arrive at any integer width: FSST-decompressed
190+
/// values carry I32 offsets, legacy dicts use I64, and narrow sequence-encoded
191+
/// offsets keep their U8/U16 ptype. `dictReadOff` must read each at its true
192+
/// stride — reading a 4-byte-stride buffer as 8 bytes was the #215 crash.
193+
@ParameterizedTest
194+
@EnumSource(value = PType.class, names = {"U8", "I8", "U16", "I16", "I32", "U32", "I64", "U64"})
195+
void getString_resolvesOffsetsAtTrueWidth(PType offsetsPtype) {
196+
// Given — dict=["foo","bar"] with offsets [0,3,6] packed at this ptype's width,
197+
// codes=[1,0,1]. Small offsets fit every width including U8/I8, exercising the
198+
// exact narrow-ptype shape from uci-magic-gamma-telescope's FSST dict.
199+
VarBinArray sut = ofDictWithOffsets(new String[]{"foo", "bar"}, offsetsPtype, new int[]{1, 0, 1});
200+
201+
// When
202+
String[] result = {sut.getString(0), sut.getString(1), sut.getString(2)};
203+
204+
// Then
205+
assertThat(result).containsExactly("bar", "foo", "bar");
206+
assertThat(sut.getByteLength(0)).isEqualTo(3);
207+
}
208+
209+
/// The #215 shape exactly: an I32-width offsets buffer (4-byte stride) mislabeled
210+
/// with an 8-byte ptype. The read must fail loudly with a helpful [VortexException],
211+
/// never a bare `IndexOutOfBoundsException` (ADR 0003 bounds discipline).
212+
@Test
213+
void getString_widthOvershootsBuffer_throwsVortexException() {
214+
// Given — 3 offsets packed as I32 (12-byte buffer) but the carrier claims I64.
215+
// Reading index 1 as 8 bytes needs offset 8..16 against a 12-byte segment.
216+
MemorySegment dictValBytes = MemorySegment.ofArray("foobar".getBytes(StandardCharsets.UTF_8));
217+
MemorySegment offsetsI32Width = leInts(new int[]{0, 3, 6});
218+
MemorySegment codes = leInts(new int[]{1});
219+
VarBinArray sut = VarBinArray.ofDict(UTF8, 1,
220+
dictValBytes, offsetsI32Width, PType.I64, codes, PType.I32);
221+
222+
// When / Then
223+
assertThatThrownBy(() -> sut.getString(0))
224+
.isInstanceOf(VortexException.class)
225+
.hasMessageContaining("out of range");
226+
}
227+
228+
private static VarBinArray ofDictWithOffsets(String[] dictValues, PType offsetsPtype, int[] codes) {
229+
byte[] allBytes = String.join("", dictValues).getBytes(StandardCharsets.UTF_8);
230+
MemorySegment dictValBytes = MemorySegment.ofArray(allBytes);
231+
232+
int[] offs = new int[dictValues.length + 1];
233+
for (int i = 0; i < dictValues.length; i++) {
234+
offs[i + 1] = offs[i] + dictValues[i].getBytes(StandardCharsets.UTF_8).length;
235+
}
236+
MemorySegment dictValOffsets = packOffsets(offs, offsetsPtype);
237+
MemorySegment dictCodes = leInts(codes);
238+
return VarBinArray.ofDict(UTF8, codes.length,
239+
dictValBytes, dictValOffsets, offsetsPtype, dictCodes, PType.I32);
240+
}
241+
242+
private static MemorySegment packOffsets(int[] offs, PType ptype) {
243+
int width = ptype.byteSize();
244+
ByteBuffer bb = ByteBuffer.allocate(offs.length * width).order(ByteOrder.LITTLE_ENDIAN);
245+
for (int o : offs) {
246+
switch (width) {
247+
case 1 -> bb.put((byte) o);
248+
case 2 -> bb.putShort((short) o);
249+
case 4 -> bb.putInt(o);
250+
default -> bb.putLong(o);
251+
}
252+
}
253+
return MemorySegment.ofArray(bb.array());
254+
}
255+
185256
}
186257
}

0 commit comments

Comments
 (0)