Skip to content

Commit 2ff7e2c

Browse files
dfa1claude
andcommitted
fix: scan nested struct columns as full-range chunk sources (#207)
`countries-of-the-world` (vortex-data 0.69.0) has a top-level struct column `data` that is itself a `vortex.struct` layout. Scan threw `VortexException: scan: column 'data' has 0 flats but the widest column has 1` from `ScanIterator.buildChunks`; `schema`/`count`/`inspect` were unaffected. Root cause: `ScanIterator.collectFlats` recognized flat, dict, zoned, and chunked layouts but not struct. A nested struct column's top layout is `vortex.struct`, so it collected zero leaves and chunk planning saw a 0-vs-N width it refused. Fix: treat a nested `vortex.struct` layout as a single full-range chunk source (like a dict leaf) rather than descending into its per-field children — those children are separate physical columns, not row chunks, and span the same row range as the parent. This mirrors the Rust reference (`vortex-layout/src/layouts/struct_/reader.rs`: `StructReader` decodes all field children over the parent's full range and inserts a leading `Bool` validity child only when the struct dtype is nullable). Decode routes through the layout registry's new `StructLayoutDecoder`, which reassembles a `StructArray` from the field children (masking each field with the struct-level validity when present). `sliceArray` gains a `StructArray` arm so the shared full-range struct column slices per chunk. Evidence on the real file: `count` = 262, `select region country_code` renders correct values, and the #207 chunk-planning exception is gone — the scan decodes the struct column into a `StructArray`. CSV export of the struct column itself is still unsupported (a separate rendering limitation, not chunk planning): `CsvExporter.cellValue` throws its clear `unsupported array type: StructArray`. Closes #207 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1636234 commit 2ff7e2c

7 files changed

Lines changed: 250 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- Scanning a struct whose columns include a nested struct no longer fails chunk planning. A nested `vortex.struct` layout column (e.g. Raincloud `countries-of-the-world`'s `data` column) is now treated as a single full-range chunk source and decoded through the layout registry's new struct decoder into a `StructArray`, matching the Rust reference (a nested struct spans its parent's row range, not an independent chunking). `schema`/`count`/`inspect` already worked; plain scans and `select` of sibling columns now work too. CSV export of the struct column itself remains unsupported (a separate rendering limitation). ([#207](https://github.com/dfa1/vortex-java/issues/207))
1213
- CSV export renders unsigned integer columns (U8–U64) with their unsigned values — high-half values previously printed as two's-complement negatives (uci-wine `magnesium` U8 132 exported as -124), silent corruption found by the Raincloud conformance suite. ([#208](https://github.com/dfa1/vortex-java/issues/208))
1314
- `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))
1415
- 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))

docs/reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ loudly (`VortexException`); there is no allow-unknown mode for layouts (Rust def
240240

241241
| Method | Notes |
242242
|-----------------------------|--------------------------------------------------------------------|
243-
| `static defaults()` | The four built-ins: flat, chunked, zoned (both aliases), dict |
243+
| `static defaults()` | The built-ins: flat, chunked, zoned (both aliases), dict, struct |
244244
| `static builder()` | Returns a fresh `Builder` |
245245
| `hasDecoder(LayoutId)` | Lookup |
246246
| `decode(ctx, layout, dtype)`| Dispatch by the layout's typed id |

reader/src/main/java/io/github/dfa1/vortex/reader/ScanIterator.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,13 @@ private static void collectFlats(Layout layout, List<Layout> out) {
112112
for (int i = start; i < layout.children().size(); i++) {
113113
collectFlats(layout.children().get(i), out);
114114
}
115+
} else if (layout.isStruct()) {
116+
// A nested struct column (a vortex.struct under the root struct) spans the same full
117+
// row range as its parent — its per-field children are separate physical columns, not
118+
// row chunks. Treat the whole subtree as one chunk source (like a dict leaf) so chunk
119+
// planning sees a single full-range chunk; decode routes through the registry's
120+
// StructLayoutDecoder, which reassembles a StructArray from the field children.
121+
out.add(layout);
115122
}
116123
}
117124

@@ -695,6 +702,17 @@ private static Array sliceArray(Array full, long offset, long length, DType dtyp
695702
case ByteArray a -> new OffsetByteArray(dtype, length, a, offset);
696703
case BoolArray a -> new OffsetBoolArray(dtype, length, a, offset);
697704
case VarBinArray a -> new VarBinArray.SlicedMode(dtype, length, a, offset);
705+
case StructArray s -> {
706+
// A shared nested struct column is decoded once over the full range, then sliced
707+
// per chunk by slicing each field into the same window. Field dtypes come from the
708+
// struct dtype so each field slices with its own offset-array type.
709+
DType.Struct sd = (DType.Struct) dtype;
710+
var slicedFields = new ArrayList<Array>(s.fieldCount());
711+
for (int i = 0; i < s.fieldCount(); i++) {
712+
slicedFields.add(sliceArray(s.field(i), offset, length, sd.fieldTypes().get(i)));
713+
}
714+
yield new StructArray(sd, length, slicedFields);
715+
}
698716
default -> throw new VortexException(
699717
"scan: cannot slice shared array of type " + full.getClass().getSimpleName());
700718
};

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ private LayoutRegistry(Map<LayoutId, LayoutDecoder> decoders) {
3333
this.decoders = Collections.unmodifiableMap(sorted);
3434
}
3535

36-
/// Returns a registry populated with the four built-in layout decoders (flat, chunked,
37-
/// zoned/stats, dict).
36+
/// Returns a registry populated with the built-in layout decoders (flat, chunked,
37+
/// zoned/stats, dict, struct).
3838
///
3939
/// @return an immutable [LayoutRegistry] with the built-in decoders registered
4040
public static LayoutRegistry defaults() {
@@ -99,14 +99,15 @@ public Builder register(LayoutDecoder decoder) {
9999
return this;
100100
}
101101

102-
/// Registers the four built-in layout decoders (flat, chunked, zoned/stats, dict).
102+
/// Registers the built-in layout decoders (flat, chunked, zoned/stats, dict, struct).
103103
///
104104
/// @return this builder, for chaining
105105
public Builder registerDefaults() {
106106
return register(new FlatLayoutDecoder())
107107
.register(new ChunkedLayoutDecoder())
108108
.register(new ZonedLayoutDecoder())
109-
.register(new DictLayoutDecoder());
109+
.register(new DictLayoutDecoder())
110+
.register(new StructLayoutDecoder());
110111
}
111112

112113
/// Builds an immutable [LayoutRegistry].
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package io.github.dfa1.vortex.reader.layout;
2+
3+
import io.github.dfa1.vortex.core.error.VortexException;
4+
import io.github.dfa1.vortex.core.model.DType;
5+
import io.github.dfa1.vortex.core.model.LayoutId;
6+
import io.github.dfa1.vortex.reader.array.Array;
7+
import io.github.dfa1.vortex.reader.array.BoolArray;
8+
import io.github.dfa1.vortex.reader.array.MaskedArray;
9+
import io.github.dfa1.vortex.reader.array.StructArray;
10+
11+
import java.util.ArrayList;
12+
import java.util.List;
13+
14+
/// Built-in decoder for the `vortex.struct` layout — one child per column, all spanning the same
15+
/// full row range. Used to decode a struct column nested under the root struct (the root itself is
16+
/// expanded column-by-column by the scan and is never routed here).
17+
///
18+
/// A struct layout carries a leading `validity` child when its dtype is nullable, mirroring the
19+
/// Rust reference (`StructReader::try_new` inserts a `Bool` validity child at index 0 when
20+
/// `layout.dtype.is_nullable()`). Field children then follow one-to-one with the struct's fields;
21+
/// when validity is present it masks every field array so per-row nullness of the struct as a whole
22+
/// is honored, matching [io.github.dfa1.vortex.reader.decode.StructEncodingDecoder].
23+
final class StructLayoutDecoder implements LayoutDecoder {
24+
25+
@Override
26+
public LayoutId layoutId() {
27+
return LayoutId.STRUCT;
28+
}
29+
30+
@Override
31+
public Array decode(LayoutDecodeContext ctx, Layout layout, DType dtype) {
32+
if (!(dtype instanceof DType.Struct structDtype)) {
33+
throw new VortexException("vortex.struct layout requires a struct dtype, got " + dtype);
34+
}
35+
List<DType> fieldTypes = structDtype.fieldTypes();
36+
int nfields = fieldTypes.size();
37+
List<Layout> children = layout.children();
38+
boolean hasValidity = children.size() == nfields + 1;
39+
if (children.size() != nfields && !hasValidity) {
40+
throw new VortexException("vortex.struct layout has " + children.size()
41+
+ " children but the struct dtype has " + nfields + " fields");
42+
}
43+
int fieldOffset = hasValidity ? 1 : 0;
44+
45+
BoolArray validity = null;
46+
if (hasValidity) {
47+
Array decoded = ctx.decodeChild(children.getFirst(), DType.BOOL);
48+
if (!(decoded instanceof BoolArray boolArray)) {
49+
throw new VortexException("vortex.struct validity decoded to unexpected type: "
50+
+ decoded.getClass().getSimpleName());
51+
}
52+
validity = boolArray;
53+
}
54+
55+
List<Array> fields = new ArrayList<>(nfields);
56+
for (int i = 0; i < nfields; i++) {
57+
Array field = ctx.decodeChild(children.get(fieldOffset + i), fieldTypes.get(i));
58+
fields.add(validity != null ? new MaskedArray(field, validity) : field);
59+
}
60+
return new StructArray(structDtype, layout.rowCount(), fields);
61+
}
62+
}

reader/src/test/java/io/github/dfa1/vortex/reader/layout/LayoutRegistryTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,16 @@
3232
class LayoutRegistryTest {
3333

3434
@ParameterizedTest
35-
@ValueSource(strings = {"vortex.flat", "vortex.chunked", "vortex.zoned", "vortex.stats", "vortex.dict"})
36-
void defaults_containTheFourBuiltins_bothZonedAliasesResolve(String wireId) {
37-
// Given — the registry populated with the four built-in decoders
35+
@ValueSource(strings = {"vortex.flat", "vortex.chunked", "vortex.zoned", "vortex.stats", "vortex.dict", "vortex.struct"})
36+
void defaults_containTheBuiltins_bothZonedAliasesResolve(String wireId) {
37+
// Given — the registry populated with the built-in decoders
3838
LayoutRegistry sut = LayoutRegistry.defaults();
3939

4040
// When
4141
boolean result = sut.hasDecoder(LayoutId.parse(wireId));
4242

43-
// Then — flat, chunked, dict, and BOTH zoned aliases (vortex.zoned + legacy vortex.stats)
44-
// resolve to a decoder
43+
// Then — flat, chunked, dict, struct, and BOTH zoned aliases (vortex.zoned + legacy
44+
// vortex.stats) resolve to a decoder
4545
assertThat(result).isTrue();
4646
}
4747

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
package io.github.dfa1.vortex.reader.layout;
2+
3+
import io.github.dfa1.vortex.core.error.VortexException;
4+
import io.github.dfa1.vortex.core.model.ColumnName;
5+
import io.github.dfa1.vortex.core.model.DType;
6+
import io.github.dfa1.vortex.core.model.EncodingId;
7+
import io.github.dfa1.vortex.core.model.LayoutId;
8+
import io.github.dfa1.vortex.core.model.PType;
9+
import io.github.dfa1.vortex.reader.array.Array;
10+
import io.github.dfa1.vortex.reader.array.BoolArray;
11+
import io.github.dfa1.vortex.reader.array.MaskedArray;
12+
import io.github.dfa1.vortex.reader.array.StructArray;
13+
import io.github.dfa1.vortex.reader.array.UnknownArray;
14+
import org.junit.jupiter.api.Test;
15+
16+
import java.lang.foreign.MemorySegment;
17+
import java.util.List;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
21+
import static org.mockito.BDDMockito.given;
22+
import static org.mockito.Mockito.mock;
23+
24+
/// Drives [StructLayoutDecoder]: it reassembles a [StructArray] from the per-field children of a
25+
/// nested `vortex.struct` layout (issue #207), threading a leading validity child through as a
26+
/// per-field mask when the struct dtype is nullable, and failing loudly on shape mismatches.
27+
class StructLayoutDecoderTest {
28+
29+
private final StructLayoutDecoder sut = new StructLayoutDecoder();
30+
31+
@Test
32+
void decode_nonNullableStruct_reassemblesFieldsInOrder() {
33+
// Given — a two-field, non-nullable struct layout: one flat child per field, no validity.
34+
// The context returns a distinct sentinel per child so field ordering is observable.
35+
DType.Struct dtype = struct(false, "a", intType(), "b", intType());
36+
Layout child0 = flat(0);
37+
Layout child1 = flat(1);
38+
Layout layout = new Layout(LayoutId.STRUCT, 3L, null, List.of(child0, child1), List.of());
39+
Array field0 = sentinel();
40+
Array field1 = sentinel();
41+
LayoutDecodeContext ctx = mock(LayoutDecodeContext.class);
42+
given(ctx.decodeChild(child0, intType())).willReturn(field0);
43+
given(ctx.decodeChild(child1, intType())).willReturn(field1);
44+
45+
// When
46+
Array result = sut.decode(ctx, layout, dtype);
47+
48+
// Then — a StructArray carrying exactly the two field arrays, in schema order, at the
49+
// layout's row count
50+
assertThat(result).isInstanceOfSatisfying(StructArray.class, sa -> {
51+
assertThat(sa.length()).isEqualTo(3L);
52+
assertThat(sa.fieldCount()).isEqualTo(2);
53+
assertThat(sa.field(0)).isSameAs(field0);
54+
assertThat(sa.field(1)).isSameAs(field1);
55+
});
56+
}
57+
58+
@Test
59+
void decode_nullableStruct_masksEveryFieldWithLeadingValidity() {
60+
// Given — a nullable struct: child[0] is the Bool validity, matching the Rust reference
61+
// (StructReader inserts a validity child when the layout dtype is nullable). Each field
62+
// must come back wrapped in a MaskedArray sharing that struct-level validity.
63+
DType.Struct dtype = struct(true, "a", intType(), "b", intType());
64+
Layout validityChild = flat(0);
65+
Layout fieldChild0 = flat(1);
66+
Layout fieldChild1 = flat(2);
67+
Layout layout = new Layout(LayoutId.STRUCT, 4L, null,
68+
List.of(validityChild, fieldChild0, fieldChild1), List.of());
69+
BoolArray validity = constantValidity();
70+
Array field0 = sentinel();
71+
Array field1 = sentinel();
72+
LayoutDecodeContext ctx = mock(LayoutDecodeContext.class);
73+
given(ctx.decodeChild(validityChild, DType.BOOL)).willReturn(validity);
74+
given(ctx.decodeChild(fieldChild0, intType())).willReturn(field0);
75+
given(ctx.decodeChild(fieldChild1, intType())).willReturn(field1);
76+
77+
// When
78+
Array result = sut.decode(ctx, layout, dtype);
79+
80+
// Then — both fields are masked by the same validity, so struct-level nulls propagate
81+
assertThat(result).isInstanceOfSatisfying(StructArray.class, sa -> {
82+
assertThat(sa.field(0)).isInstanceOfSatisfying(MaskedArray.class, m -> {
83+
assertThat(m.inner()).isSameAs(field0);
84+
assertThat(m.validity()).isSameAs(validity);
85+
});
86+
assertThat(sa.field(1)).isInstanceOfSatisfying(MaskedArray.class, m ->
87+
assertThat(m.inner()).isSameAs(field1));
88+
});
89+
}
90+
91+
@Test
92+
void decode_nonStructDtype_throwsVortexException() {
93+
// Given — a struct layout handed a primitive dtype: a corrupt file could pair them
94+
Layout layout = new Layout(LayoutId.STRUCT, 1L, null, List.of(flat(0)), List.of());
95+
LayoutDecodeContext ctx = mock(LayoutDecodeContext.class);
96+
97+
// When / Then — fail fast rather than reassemble a struct from a non-struct type
98+
assertThatThrownBy(() -> sut.decode(ctx, layout, intType()))
99+
.isInstanceOf(VortexException.class)
100+
.hasMessageContaining("requires a struct dtype");
101+
}
102+
103+
@Test
104+
void decode_childCountMismatch_throwsVortexException() {
105+
// Given — a two-field struct but only one child (neither nfields nor nfields+1)
106+
DType.Struct dtype = struct(false, "a", intType(), "b", intType());
107+
Layout layout = new Layout(LayoutId.STRUCT, 1L, null, List.of(flat(0)), List.of());
108+
LayoutDecodeContext ctx = mock(LayoutDecodeContext.class);
109+
110+
// When / Then — the shape is unrecoverable, so it fails loudly
111+
assertThatThrownBy(() -> sut.decode(ctx, layout, dtype))
112+
.isInstanceOf(VortexException.class)
113+
.hasMessageContaining("1 children")
114+
.hasMessageContaining("2 fields");
115+
}
116+
117+
// ── helpers ───────────────────────────────────────────────────────────────
118+
119+
private static DType intType() {
120+
return new DType.Primitive(PType.I32, false);
121+
}
122+
123+
private static DType.Struct struct(boolean nullable, String n0, DType t0, String n1, DType t1) {
124+
return new DType.Struct(
125+
List.of(ColumnName.of(n0), ColumnName.of(n1)), List.of(t0, t1), nullable);
126+
}
127+
128+
private static Layout flat(int segment) {
129+
return new Layout(LayoutId.FLAT, 0L, null, List.of(), List.of(segment));
130+
}
131+
132+
/// A concrete stand-in [Array] — the sealed interface cannot be mocked, and these tests only
133+
/// need object identity.
134+
private static Array sentinel() {
135+
return new UnknownArray(EncodingId.parse("stub"), intType(), 0L, null,
136+
new MemorySegment[0], new Array[0]);
137+
}
138+
139+
/// A minimal all-valid [BoolArray]; only its identity is asserted, never its contents.
140+
private static BoolArray constantValidity() {
141+
return new BoolArray() {
142+
@Override
143+
public boolean getBoolean(long i) {
144+
return true;
145+
}
146+
147+
@Override
148+
public long length() {
149+
return 4L;
150+
}
151+
152+
@Override
153+
public DType dtype() {
154+
return DType.BOOL;
155+
}
156+
};
157+
}
158+
}

0 commit comments

Comments
 (0)