Skip to content

Commit 604586f

Browse files
committed
fix(reader): resolve non-offset dict values pools lazily, not by expansion
DictLayoutDecoder fell through to an eager per-row string expansion for any dict values pool that wasn't already a flat VarBinOffsetArray (a vortex.constant pool, a StringView pool, or a VarBin-backed extension dtype), and blindly cast to VarBinArray for pool shapes that aren't VarBin at all (e.g. a dict-encoded vortex.uuid), raising a raw ClassCastException instead of VortexException. Non-offset pools now normalize into the existing lazy VarBinDictArray path instead. The normalization walk is bounded by validating codes against the pool first, since a pool's declared length is untrusted and some shapes (vortex.constant) report an arbitrary length in a tiny file; a vortex.constant pool resolves in O(1) without a walk at all. The same fallback was also dropping pool/codes validity for any pool it reached, silently un-nulling rows — fixed as part of the same merge, since both bugs shared the one code path. Closes #341
1 parent 15d02a2 commit 604586f

4 files changed

Lines changed: 313 additions & 81 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414
### Fixed
1515

1616
- A malformed `fastlanes.rle` column now fails as `VortexException` where an absurd declared length, an empty or undersized child segment, or an out-of-range chunk offset previously escaped as `OutOfMemoryError`, `NegativeArraySizeException`, `ArithmeticException`, or `IndexOutOfBoundsException`. ([#342](https://github.com/dfa1/vortex-java/issues/342))
17+
- A `vortex.dict` layout over a StringView or `vortex.constant` values pool no longer expands every row into a fresh buffer, and no longer silently drops that pool's validity. ([#341](https://github.com/dfa1/vortex-java/issues/341))
18+
- A `vortex.dict` layout whose values pool declares more entries than it stores — a `vortex.constant` pool can claim any length in a few hundred bytes — no longer allocates against that claim; codes are bounded against the pool first, and an out-of-pool code fails as `VortexException`. ([#341](https://github.com/dfa1/vortex-java/issues/341))
19+
- A `vortex.dict` layout over a values pool that is neither VarBin- nor primitive-shaped (e.g. a dict-encoded `vortex.uuid`) now fails as `VortexException` instead of `ClassCastException`. ([#341](https://github.com/dfa1/vortex-java/issues/341))
1720

1821
## [0.13.2] — 2026-08-07
1922

docs/compatibility.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ only the built-in decoders in `reader`; no encoder class is loaded.
4040
| `vortex.onpair` experimental string encoding | Rust 0.74.0 | ❌ Not registered. Files using it fail to decode unless `ReadRegistry.builder().allowUnknown()` is enabled. |
4141
| `vortex.variant` arbitrary nested objects | Rust (`vortex.parquet.variant`) | ⚠️ Java encodes/decodes variant columns of **typed scalar** values (constant / chunked-of-constants core, optional shredded child); Java↔Rust round-trip verified. Arbitrary nested JSON objects and real path-based shredding need the `vortex.parquet.variant` physical encoding — deferred ([ADR 0014](../adr/0014-variant-encoding-strategy.md)). |
4242
| Arrow extension array import affecting Variant shape | Rust 0.74.0 (#8125) | Untested. Re-run integration fixtures against v0.74.0 once published. |
43+
| `vortex.dict` **layout** over a values pool that is neither VarBin- nor primitive-shaped (e.g. a dict-encoded `vortex.uuid`, whose storage is `FixedSizeList(U8, 16)`) | Rust's dict layout accepts any dtype | ❌ No lazy dict carrier exists for that pool shape, so decode throws `VortexException("unsupported dict values shape: …")`. The `vortex.dict` *encoding* is unaffected. |
4344
| Duplicate struct field names | Rust writer rejects ("StructLayout must have unique field names"); Rust reader tolerates foreign files (first-match access) | ⚠️ Deliberate divergence on read: Java rejects such files with `VortexException("duplicate field name in file schema")` instead of tolerating them — the name-keyed `Chunk` API cannot represent both columns, and silent column loss is worse than a loud failure on a file the reference writer refuses to produce. Java's writer mirrors the Rust writer's rejection. |
4445
| Blank / control-character field names | Wire-legal; the Rust writer produces `""` and whitespace-only names. NUL (`U+0000`) additionally aborts the Rust toolchain: Arrow FFI schema export hits a panic-cannot-unwind in `arrow-rs` (`ffi_stream::get_schema`) and SIGABRTs the process (measured against vortex-jni 0.75.0) | ⚠️ Deliberate strictness BOTH ways: vortex-java's writer refuses blank and control-character field names (`IllegalArgumentException`), and its reader rejects files carrying them (`VortexException` naming the producing pipeline as the likely bug) — the JSON-`""`-key principle: wire-legal is a floor, not a policy. Printable names of any shape (`$`-runs, spaces inside, emoji) are legal and round-trip intact both directions (measured; pinned by `ColumnNameEdgeCasesIntegrationTest`). |
4546

@@ -143,7 +144,7 @@ decoder falls into one of three shapes:
143144
| `vortex.varbinview` | Lazy | Lazy | `VarBinViewArray` — keeps views + data buffers as mmap slices |
144145
| `vortex.alp` | Lazy | Lazy | `LazyAlpXxxArray`; broadcast → `LazyConstantXxxArray`; patched stays Materialized, ADR 0010 + 0015 |
145146
| `vortex.alprd` | Lazy | Lazy | `LazyAlpRdDoubleArray`/`LazyAlpRdFloatArray` — left/right + patches on access |
146-
| `vortex.dict` | Lazy | Lazy | `DictXxxArray` (numeric) + `VarBinDictArray` (string), ADR 0012 |
147+
| `vortex.dict` | Lazy | Lazy | `DictXxxArray` (numeric) + `VarBinDictArray` (string), ADR 0012. The `vortex.dict` **layout** resolves the same way for any VarBin-shaped values pool — a StringView pool, or any VarBin-backed extension dtype — normalizing only the pool entries the codes reach, never the rows; a `vortex.constant` pool collapses to `VarBinConstantArray` |
147148
| `vortex.sparse` | Lazy | Lazy | `LazySparseXxxArray` (primitive + bool) + `VarBinSparseArray` (Utf8/Binary); fill broadcast, patch resolved per access; patch-free range → `VarBinConstantArray`, ADR 0015 |
148149
| `vortex.sequence` | Lazy | Lazy | `LazySequenceXxxArray`; `base + i * multiplier` per access, no buffer, ADR 0015 |
149150
| `vortex.struct` | Zero-copy | Zero-copy | `StructArray` wraps fields |

reader/src/main/java/io/github/dfa1/vortex/reader/layout/DictLayoutDecoder.java

Lines changed: 127 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package io.github.dfa1.vortex.reader.layout;
22

3-
import static io.github.dfa1.vortex.core.io.VortexFormat.LE_SHORT;
43
import static io.github.dfa1.vortex.core.io.VortexFormat.LE_INT;
54
import static io.github.dfa1.vortex.core.io.VortexFormat.LE_LONG;
5+
import static io.github.dfa1.vortex.core.io.VortexFormat.LE_SHORT;
66

77
import io.github.dfa1.vortex.core.error.VortexException;
88
import io.github.dfa1.vortex.core.model.DType;
@@ -26,6 +26,7 @@
2626
import io.github.dfa1.vortex.reader.array.MaskedArray;
2727
import io.github.dfa1.vortex.reader.array.MaterializedBoolArray;
2828
import io.github.dfa1.vortex.reader.array.VarBinArray;
29+
import io.github.dfa1.vortex.reader.array.VarBinConstantArray;
2930
import io.github.dfa1.vortex.reader.array.VarBinOffsetArray;
3031

3132
import java.lang.foreign.MemorySegment;
@@ -34,8 +35,9 @@
3435
import java.util.Optional;
3536

3637
/// Built-in decoder for the `vortex.dict` layout — a low-cardinality column stored as dictionary
37-
/// values plus per-row codes. Extracted verbatim from `ScanIterator.decodeDictLayout` and its
38-
/// private helpers.
38+
/// values plus per-row codes. Both supported pool shapes decode lazily: a VarBin-shaped pool
39+
/// (Utf8, Binary, or a VarBin-backed extension dtype) resolves through `VarBinDictArray`, a
40+
/// primitive pool through the matching `DictXxxArray`. Nothing is expanded per row.
3941
final class DictLayoutDecoder implements LayoutDecoder {
4042

4143
@Override
@@ -64,19 +66,18 @@ public Array decode(LayoutDecodeContext ctx, Layout dictLayout, DType dtype) {
6466
Array values = ctx.decodeChild(valuesLayout, dtype);
6567
Array codes = ctx.decodeChild(codesLayout, new DType.Primitive(codesPType, false));
6668

67-
// VarBin (string) dict: VarBinArray is a sealed interface; ofDict returns the
68-
// lazy VarBinDictArray record (no eager expansion into per-row offsets/bytes).
69-
// Unwrap a masked (nullable) codes/values child so the string expansion sees the raw
70-
// payload; the row-level validity is re-applied by wrapping the result below. This mirrors
71-
// the primitive path (buildLazyDictPrimitive) and is the shape a nullable global-dict Utf8
72-
// column produces (masked codes + non-nullable pool).
69+
// VarBin (string) dict: ofDict returns the lazy VarBinDictArray record (no eager
70+
// expansion into per-row offsets/bytes). Unwrap a masked (nullable) codes/values child so
71+
// the dict carrier sees the raw payload; the row-level validity is re-applied by wrapping
72+
// the result below. This mirrors the primitive path (buildLazyDictPrimitive) and is the
73+
// shape a nullable global-dict Utf8 column produces (masked codes + non-nullable pool).
7374
BoolArray poolValidity = values instanceof MaskedArray mv ? mv.validity() : null;
7475
Array valuesData = values instanceof MaskedArray mv ? mv.inner() : values;
7576
BoolArray codesValidity = codes instanceof MaskedArray mc ? mc.validity() : null;
7677
Array codesData = codes instanceof MaskedArray mc ? mc.inner() : codes;
77-
if (valuesData instanceof VarBinOffsetArray vb) {
78+
if (valuesData instanceof VarBinArray vb) {
7879
// Zip-bomb guard: read the codes as a segment so we can validate the buffer
79-
// before allocating the expansion output. For direct-mapped encodings (e.g.
80+
// before building the dict carrier. For direct-mapped encodings (e.g.
8081
// vortex.primitive), the codes buffer is mmap-bounded and can be much smaller
8182
// than the claimed rowCount. Full-decode encodings (e.g. bitpacked) already
8283
// wrote n * elemBytes to the arena during decodeChild above, so their buffer
@@ -87,10 +88,7 @@ public Array decode(LayoutDecodeContext ctx, Layout dictLayout, DType dtype) {
8788
throw new VortexException(EncodingId.VORTEX_DICT,
8889
"dict codes: layout row_count=" + n + " exceeds buffer capacity=" + bufferCodes);
8990
}
90-
MemorySegment valOffsets = vb.offsetsSegment();
91-
PType valOffPType = vb.offsetsPtype();
92-
Array dict = VarBinArray.ofDict(dtype, n, vb.bytesSegment(), valOffsets, valOffPType,
93-
codesSeg, codesPType);
91+
Array dict = buildLazyDictVarBin(dtype, n, vb, codesSeg, codesPType, arena);
9492
if (poolValidity == null && codesValidity == null) {
9593
return dict;
9694
}
@@ -107,16 +105,121 @@ public Array decode(LayoutDecodeContext ctx, Layout dictLayout, DType dtype) {
107105
validateDictCodesCapacity(codes, codesPType, n);
108106
return buildLazyDictPrimitive(pDtype, n, values, codes, arena);
109107
}
110-
// Non-Utf8, non-Primitive dict — e.g. extension types backed by VarBin. Fall through
111-
// to the existing string expansion for compatibility.
112-
MemorySegment codesSegFallback = codes.materialize(arena);
113-
long bufferCodesFallback = codesSegFallback.byteSize() / codesPType.byteSize();
114-
if (bufferCodesFallback < n) {
115-
throw new VortexException(EncodingId.VORTEX_DICT,
116-
"dict codes: layout row_count=" + n + " exceeds buffer capacity=" + bufferCodesFallback);
108+
// Neither VarBin- nor primitive-shaped: no lazy dict carrier exists for this pool — e.g.
109+
// a dict-encoded FixedSizeList-backed extension such as vortex.uuid. Untrusted input can
110+
// also reach here with an arbitrary child encoding, so reject rather than blindly casting
111+
// (a raw ClassCastException would break the security contract).
112+
throw new VortexException(EncodingId.VORTEX_DICT, "unsupported dict values shape: "
113+
+ valuesData.getClass().getSimpleName() + " for dtype "
114+
+ dtype.getClass().getSimpleName());
115+
}
116+
117+
/// Builds the lazy carrier for a VarBin-shaped dictionary — Utf8, Binary, or any
118+
/// VarBin-backed extension dtype (#341). No row is ever expanded: rows resolve through
119+
/// their code on access.
120+
///
121+
/// Only [VarBinOffsetArray] is directly indexable by code, so any other pool shape has to
122+
/// be walked entry by entry to become one. That walk is bounded by the codes, not by the
123+
/// pool's own `length()`: shapes that carry no per-entry storage report whatever length
124+
/// the file declares (a `vortex.constant` pool a few hundred bytes long can claim 2^40
125+
/// entries), and normalizing that claim would allocate until the reader dies. The codes
126+
/// buffer is the one bound the file must physically back, and it has already been checked
127+
/// to hold `n` entries by the caller.
128+
///
129+
/// @param dtype logical dtype of the column
130+
/// @param n logical row count
131+
/// @param values the decoded values pool, mask already unwrapped
132+
/// @param codesSeg per-row codes into `values`
133+
/// @param codesPType physical type of the codes
134+
/// @param arena allocator for a normalized pool, when one is needed
135+
/// @return a lazy `VarBinDictArray`, or a `VarBinConstantArray` when the pool holds a
136+
/// single broadcast value
137+
private static Array buildLazyDictVarBin(DType dtype, long n, VarBinArray values,
138+
MemorySegment codesSeg, PType codesPType, SegmentAllocator arena) {
139+
if (values instanceof VarBinOffsetArray flat) {
140+
// The common Utf8/Binary dict: already bytes-plus-offsets, so nothing is walked or
141+
// copied and an out-of-pool code is caught by the carrier on access.
142+
return VarBinArray.ofDict(dtype, n, flat.bytesSegment(), flat.offsetsSegment(),
143+
flat.offsetsPtype(), codesSeg, codesPType);
144+
}
145+
long poolExtent = checkedPoolExtent(codesSeg, codesPType, n, values.length());
146+
if (values instanceof VarBinConstantArray constant) {
147+
// A `vortex.constant` pool holds one distinct value broadcast across its entries,
148+
// so every in-range code resolves to the same bytes and the dict collapses to that
149+
// constant — O(1), no pool walk, no allocation whatever length the pool claims.
150+
return new VarBinConstantArray(dtype, n, constant.bytes());
151+
}
152+
// Remaining shapes (StringView, chunked, run-end, sparse, sliced pools) are walked, but
153+
// only up to the highest entry a code actually reaches.
154+
VarBinOffsetArray pool = VarBinArray.toOffsetMode(values.limited(poolExtent), arena);
155+
return VarBinArray.ofDict(dtype, n, pool.bytesSegment(), pool.offsetsSegment(),
156+
pool.offsetsPtype(), codesSeg, codesPType);
157+
}
158+
159+
/// Values-side bounds guard: validates every per-row code against the values pool and
160+
/// returns how many pool entries the codes can actually reach.
161+
///
162+
/// The codes-side guard in [#decode(LayoutDecodeContext, Layout, DType)] has no values-side
163+
/// analogue, because a pool's `length()` is a claim rather than a measurement for the shapes
164+
/// with no per-entry storage. Reading the codes bounds the pool by something the file must
165+
/// physically contain. Runs only when the pool is not already flat, so the common dict path
166+
/// pays nothing; the ptype dispatch is hoisted out of the loop (CLAUDE.md hot-loop rule) and
167+
/// the range test is a single check on the reduced min/max rather than one per row.
168+
///
169+
/// @param codesSeg per-row codes, already checked to hold at least `n` entries
170+
/// @param codesPType physical type of the codes
171+
/// @param n logical row count
172+
/// @param poolLength number of entries the values pool claims to hold
173+
/// @return `maxCode + 1`, in `[0, poolLength]`; `0` when there are no rows
174+
/// @throws VortexException if any code is negative or outside the pool
175+
private static long checkedPoolExtent(MemorySegment codesSeg, PType codesPType, long n,
176+
long poolLength) {
177+
long max = -1;
178+
long min = 0;
179+
switch (codesPType) {
180+
case U8 -> {
181+
for (long i = 0; i < n; i++) {
182+
long code = Byte.toUnsignedLong(codesSeg.get(ValueLayout.JAVA_BYTE, i));
183+
max = Math.max(max, code);
184+
}
185+
}
186+
case U16 -> {
187+
for (long i = 0; i < n; i++) {
188+
long code = Short.toUnsignedLong(codesSeg.getAtIndex(LE_SHORT, i));
189+
max = Math.max(max, code);
190+
}
191+
}
192+
case U32 -> {
193+
for (long i = 0; i < n; i++) {
194+
long code = Integer.toUnsignedLong(codesSeg.getAtIndex(LE_INT, i));
195+
max = Math.max(max, code);
196+
}
197+
}
198+
case I32 -> {
199+
for (long i = 0; i < n; i++) {
200+
long code = codesSeg.getAtIndex(LE_INT, i);
201+
max = Math.max(max, code);
202+
min = Math.min(min, code);
203+
}
204+
}
205+
// A u64 code past Long.MAX_VALUE reads back negative, which the min test rejects —
206+
// it is out of range for any pool a file can actually hold anyway.
207+
case I64, U64 -> {
208+
for (long i = 0; i < n; i++) {
209+
long code = codesSeg.getAtIndex(LE_LONG, i);
210+
max = Math.max(max, code);
211+
min = Math.min(min, code);
212+
}
213+
}
214+
default -> throw new VortexException(EncodingId.VORTEX_DICT,
215+
"layout: unsupported codes ptype: " + codesPType);
216+
}
217+
if (min < 0 || max >= poolLength) {
218+
throw new VortexException(EncodingId.VORTEX_DICT, "layout: dict code "
219+
+ (min < 0 ? min : max) + " out of range for a values pool of "
220+
+ poolLength + " entries");
117221
}
118-
return expandDictStrings(VarBinArray.toOffsetMode((VarBinArray) values, arena),
119-
codesSegFallback, codesPType, dtype, n, arena);
222+
return max + 1;
120223
}
121224

122225
/// Lazy-path zip-bomb guard. Inspects `codes`'s primary segment when available
@@ -236,53 +339,4 @@ private static PType readDictLayoutCodesPType(MemorySegment rawMeta) {
236339
}
237340
return PType.U8;
238341
}
239-
240-
private static Array expandDictStrings(
241-
VarBinOffsetArray values, MemorySegment codesSegs,
242-
PType codesPType, DType dtype,
243-
long n, SegmentAllocator arena
244-
) {
245-
MemorySegment valBytes = values.bytesSegment();
246-
MemorySegment valOffsets = values.offsetsSegment();
247-
PType valOffPType = values.offsetsPtype();
248-
249-
// First pass: total output byte length
250-
long totalBytes = 0L;
251-
for (long i = 0; i < n; i++) {
252-
long code = readUnsigned(codesSegs, i, codesPType);
253-
long start = readUnsigned(valOffsets, code, valOffPType);
254-
long end = readUnsigned(valOffsets, code + 1, valOffPType);
255-
totalBytes += end - start;
256-
}
257-
258-
MemorySegment outBytes = arena.allocate(totalBytes > 0 ? totalBytes : 1);
259-
MemorySegment outOffsets = arena.allocate((n + 1) * 4L, 4);
260-
outOffsets.setAtIndex(LE_INT, 0, 0);
261-
262-
long bytePos = 0L;
263-
for (long i = 0; i < n; i++) {
264-
long code = readUnsigned(codesSegs, i, codesPType);
265-
long start = readUnsigned(valOffsets, code, valOffPType);
266-
long end = readUnsigned(valOffsets, code + 1, valOffPType);
267-
long strLen = end - start;
268-
if (strLen > 0) {
269-
MemorySegment.copy(valBytes, start, outBytes, bytePos, strLen);
270-
bytePos += strLen;
271-
}
272-
outOffsets.setAtIndex(LE_INT, i + 1, (int) bytePos);
273-
}
274-
275-
return new VarBinOffsetArray(dtype, n, outBytes.asReadOnly(), outOffsets.asReadOnly(), PType.I32);
276-
}
277-
278-
private static long readUnsigned(MemorySegment seg, long idx, PType ptype) {
279-
return switch (ptype) {
280-
case U8 -> Byte.toUnsignedLong(seg.get(ValueLayout.JAVA_BYTE, idx));
281-
case U16 -> Short.toUnsignedLong(seg.get(LE_SHORT, idx * 2));
282-
case U32 -> Integer.toUnsignedLong(seg.getAtIndex(LE_INT, idx));
283-
case I32 -> seg.getAtIndex(LE_INT, idx);
284-
case I64, U64 -> seg.getAtIndex(LE_LONG, idx);
285-
default -> throw new VortexException(EncodingId.VORTEX_DICT, "layout: unsupported ptype " + ptype);
286-
};
287-
}
288342
}

0 commit comments

Comments
 (0)