diff --git a/CHANGELOG.md b/CHANGELOG.md index ccde6a27..3dba4d57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- 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)) - 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)) - `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)) diff --git a/docs/reference.md b/docs/reference.md index 538b289f..9179a4fc 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -240,7 +240,7 @@ loudly (`VortexException`); there is no allow-unknown mode for layouts (Rust def | Method | Notes | |-----------------------------|--------------------------------------------------------------------| -| `static defaults()` | The four built-ins: flat, chunked, zoned (both aliases), dict | +| `static defaults()` | The built-ins: flat, chunked, zoned (both aliases), dict, struct | | `static builder()` | Returns a fresh `Builder` | | `hasDecoder(LayoutId)` | Lookup | | `decode(ctx, layout, dtype)`| Dispatch by the layout's typed id | diff --git a/integration/src/test/resources/raincloud/expected-status.csv b/integration/src/test/resources/raincloud/expected-status.csv index 1a0cb4d5..f5978797 100644 --- a/integration/src/test/resources/raincloud/expected-status.csv +++ b/integration/src/test/resources/raincloud/expected-status.csv @@ -81,7 +81,7 @@ cohere-wikipedia-simple-embed,untriaged coig,untriaged coronahack-chest-xraydataset,untriaged cosmopedia-stanford,untriaged -countries-of-the-world,gap:207 +countries-of-the-world,gap:217 covid-world-vaccination-progress,untriaged covid19-data-from-john-hopkins-university,untriaged crimes-in-boston,untriaged diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/ScanIterator.java b/reader/src/main/java/io/github/dfa1/vortex/reader/ScanIterator.java index 95ed9e33..a13e48a7 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/ScanIterator.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/ScanIterator.java @@ -112,6 +112,13 @@ private static void collectFlats(Layout layout, List out) { for (int i = start; i < layout.children().size(); i++) { collectFlats(layout.children().get(i), out); } + } else if (layout.isStruct()) { + // A nested struct column (a vortex.struct under the root struct) spans the same full + // row range as its parent — its per-field children are separate physical columns, not + // row chunks. Treat the whole subtree as one chunk source (like a dict leaf) so chunk + // planning sees a single full-range chunk; decode routes through the registry's + // StructLayoutDecoder, which reassembles a StructArray from the field children. + out.add(layout); } } @@ -695,6 +702,17 @@ private static Array sliceArray(Array full, long offset, long length, DType dtyp case ByteArray a -> new OffsetByteArray(dtype, length, a, offset); case BoolArray a -> new OffsetBoolArray(dtype, length, a, offset); case VarBinArray a -> new VarBinArray.SlicedMode(dtype, length, a, offset); + case StructArray s -> { + // A shared nested struct column is decoded once over the full range, then sliced + // per chunk by slicing each field into the same window. Field dtypes come from the + // struct dtype so each field slices with its own offset-array type. + DType.Struct sd = (DType.Struct) dtype; + var slicedFields = new ArrayList(s.fieldCount()); + for (int i = 0; i < s.fieldCount(); i++) { + slicedFields.add(sliceArray(s.field(i), offset, length, sd.fieldTypes().get(i))); + } + yield new StructArray(sd, length, slicedFields); + } default -> throw new VortexException( "scan: cannot slice shared array of type " + full.getClass().getSimpleName()); }; diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/layout/LayoutRegistry.java b/reader/src/main/java/io/github/dfa1/vortex/reader/layout/LayoutRegistry.java index 7efb4179..6cd23bb3 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/layout/LayoutRegistry.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/layout/LayoutRegistry.java @@ -33,8 +33,8 @@ private LayoutRegistry(Map decoders) { this.decoders = Collections.unmodifiableMap(sorted); } - /// Returns a registry populated with the four built-in layout decoders (flat, chunked, - /// zoned/stats, dict). + /// Returns a registry populated with the built-in layout decoders (flat, chunked, + /// zoned/stats, dict, struct). /// /// @return an immutable [LayoutRegistry] with the built-in decoders registered public static LayoutRegistry defaults() { @@ -99,14 +99,15 @@ public Builder register(LayoutDecoder decoder) { return this; } - /// Registers the four built-in layout decoders (flat, chunked, zoned/stats, dict). + /// Registers the built-in layout decoders (flat, chunked, zoned/stats, dict, struct). /// /// @return this builder, for chaining public Builder registerDefaults() { return register(new FlatLayoutDecoder()) .register(new ChunkedLayoutDecoder()) .register(new ZonedLayoutDecoder()) - .register(new DictLayoutDecoder()); + .register(new DictLayoutDecoder()) + .register(new StructLayoutDecoder()); } /// Builds an immutable [LayoutRegistry]. diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/layout/StructLayoutDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/layout/StructLayoutDecoder.java new file mode 100644 index 00000000..a67f85c5 --- /dev/null +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/layout/StructLayoutDecoder.java @@ -0,0 +1,62 @@ +package io.github.dfa1.vortex.reader.layout; + +import io.github.dfa1.vortex.core.error.VortexException; +import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.core.model.LayoutId; +import io.github.dfa1.vortex.reader.array.Array; +import io.github.dfa1.vortex.reader.array.BoolArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; +import io.github.dfa1.vortex.reader.array.StructArray; + +import java.util.ArrayList; +import java.util.List; + +/// Built-in decoder for the `vortex.struct` layout — one child per column, all spanning the same +/// full row range. Used to decode a struct column nested under the root struct (the root itself is +/// expanded column-by-column by the scan and is never routed here). +/// +/// A struct layout carries a leading `validity` child when its dtype is nullable, mirroring the +/// Rust reference (`StructReader::try_new` inserts a `Bool` validity child at index 0 when +/// `layout.dtype.is_nullable()`). Field children then follow one-to-one with the struct's fields; +/// when validity is present it masks every field array so per-row nullness of the struct as a whole +/// is honored, matching [io.github.dfa1.vortex.reader.decode.StructEncodingDecoder]. +final class StructLayoutDecoder implements LayoutDecoder { + + @Override + public LayoutId layoutId() { + return LayoutId.STRUCT; + } + + @Override + public Array decode(LayoutDecodeContext ctx, Layout layout, DType dtype) { + if (!(dtype instanceof DType.Struct structDtype)) { + throw new VortexException("vortex.struct layout requires a struct dtype, got " + dtype); + } + List fieldTypes = structDtype.fieldTypes(); + int nfields = fieldTypes.size(); + List children = layout.children(); + boolean hasValidity = children.size() == nfields + 1; + if (children.size() != nfields && !hasValidity) { + throw new VortexException("vortex.struct layout has " + children.size() + + " children but the struct dtype has " + nfields + " fields"); + } + int fieldOffset = hasValidity ? 1 : 0; + + BoolArray validity = null; + if (hasValidity) { + Array decoded = ctx.decodeChild(children.getFirst(), DType.BOOL); + if (!(decoded instanceof BoolArray boolArray)) { + throw new VortexException("vortex.struct validity decoded to unexpected type: " + + decoded.getClass().getSimpleName()); + } + validity = boolArray; + } + + List fields = new ArrayList<>(nfields); + for (int i = 0; i < nfields; i++) { + Array field = ctx.decodeChild(children.get(fieldOffset + i), fieldTypes.get(i)); + fields.add(validity != null ? new MaskedArray(field, validity) : field); + } + return new StructArray(structDtype, layout.rowCount(), fields); + } +} diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/LayoutRegistryTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/LayoutRegistryTest.java index 5211085f..29636fd1 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/LayoutRegistryTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/LayoutRegistryTest.java @@ -32,16 +32,16 @@ class LayoutRegistryTest { @ParameterizedTest - @ValueSource(strings = {"vortex.flat", "vortex.chunked", "vortex.zoned", "vortex.stats", "vortex.dict"}) - void defaults_containTheFourBuiltins_bothZonedAliasesResolve(String wireId) { - // Given — the registry populated with the four built-in decoders + @ValueSource(strings = {"vortex.flat", "vortex.chunked", "vortex.zoned", "vortex.stats", "vortex.dict", "vortex.struct"}) + void defaults_containTheBuiltins_bothZonedAliasesResolve(String wireId) { + // Given — the registry populated with the built-in decoders LayoutRegistry sut = LayoutRegistry.defaults(); // When boolean result = sut.hasDecoder(LayoutId.parse(wireId)); - // Then — flat, chunked, dict, and BOTH zoned aliases (vortex.zoned + legacy vortex.stats) - // resolve to a decoder + // Then — flat, chunked, dict, struct, and BOTH zoned aliases (vortex.zoned + legacy + // vortex.stats) resolve to a decoder assertThat(result).isTrue(); } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/StructLayoutDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/StructLayoutDecoderTest.java new file mode 100644 index 00000000..e9ef402e --- /dev/null +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/StructLayoutDecoderTest.java @@ -0,0 +1,158 @@ +package io.github.dfa1.vortex.reader.layout; + +import io.github.dfa1.vortex.core.error.VortexException; +import io.github.dfa1.vortex.core.model.ColumnName; +import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.core.model.EncodingId; +import io.github.dfa1.vortex.core.model.LayoutId; +import io.github.dfa1.vortex.core.model.PType; +import io.github.dfa1.vortex.reader.array.Array; +import io.github.dfa1.vortex.reader.array.BoolArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; +import io.github.dfa1.vortex.reader.array.StructArray; +import io.github.dfa1.vortex.reader.array.UnknownArray; +import org.junit.jupiter.api.Test; + +import java.lang.foreign.MemorySegment; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/// Drives [StructLayoutDecoder]: it reassembles a [StructArray] from the per-field children of a +/// nested `vortex.struct` layout (issue #207), threading a leading validity child through as a +/// per-field mask when the struct dtype is nullable, and failing loudly on shape mismatches. +class StructLayoutDecoderTest { + + private final StructLayoutDecoder sut = new StructLayoutDecoder(); + + @Test + void decode_nonNullableStruct_reassemblesFieldsInOrder() { + // Given — a two-field, non-nullable struct layout: one flat child per field, no validity. + // The context returns a distinct sentinel per child so field ordering is observable. + DType.Struct dtype = struct(false, "a", intType(), "b", intType()); + Layout child0 = flat(0); + Layout child1 = flat(1); + Layout layout = new Layout(LayoutId.STRUCT, 3L, null, List.of(child0, child1), List.of()); + Array field0 = sentinel(); + Array field1 = sentinel(); + LayoutDecodeContext ctx = mock(LayoutDecodeContext.class); + given(ctx.decodeChild(child0, intType())).willReturn(field0); + given(ctx.decodeChild(child1, intType())).willReturn(field1); + + // When + Array result = sut.decode(ctx, layout, dtype); + + // Then — a StructArray carrying exactly the two field arrays, in schema order, at the + // layout's row count + assertThat(result).isInstanceOfSatisfying(StructArray.class, sa -> { + assertThat(sa.length()).isEqualTo(3L); + assertThat(sa.fieldCount()).isEqualTo(2); + assertThat(sa.field(0)).isSameAs(field0); + assertThat(sa.field(1)).isSameAs(field1); + }); + } + + @Test + void decode_nullableStruct_masksEveryFieldWithLeadingValidity() { + // Given — a nullable struct: child[0] is the Bool validity, matching the Rust reference + // (StructReader inserts a validity child when the layout dtype is nullable). Each field + // must come back wrapped in a MaskedArray sharing that struct-level validity. + DType.Struct dtype = struct(true, "a", intType(), "b", intType()); + Layout validityChild = flat(0); + Layout fieldChild0 = flat(1); + Layout fieldChild1 = flat(2); + Layout layout = new Layout(LayoutId.STRUCT, 4L, null, + List.of(validityChild, fieldChild0, fieldChild1), List.of()); + BoolArray validity = constantValidity(); + Array field0 = sentinel(); + Array field1 = sentinel(); + LayoutDecodeContext ctx = mock(LayoutDecodeContext.class); + given(ctx.decodeChild(validityChild, DType.BOOL)).willReturn(validity); + given(ctx.decodeChild(fieldChild0, intType())).willReturn(field0); + given(ctx.decodeChild(fieldChild1, intType())).willReturn(field1); + + // When + Array result = sut.decode(ctx, layout, dtype); + + // Then — both fields are masked by the same validity, so struct-level nulls propagate + assertThat(result).isInstanceOfSatisfying(StructArray.class, sa -> { + assertThat(sa.field(0)).isInstanceOfSatisfying(MaskedArray.class, m -> { + assertThat(m.inner()).isSameAs(field0); + assertThat(m.validity()).isSameAs(validity); + }); + assertThat(sa.field(1)).isInstanceOfSatisfying(MaskedArray.class, m -> + assertThat(m.inner()).isSameAs(field1)); + }); + } + + @Test + void decode_nonStructDtype_throwsVortexException() { + // Given — a struct layout handed a primitive dtype: a corrupt file could pair them + Layout layout = new Layout(LayoutId.STRUCT, 1L, null, List.of(flat(0)), List.of()); + LayoutDecodeContext ctx = mock(LayoutDecodeContext.class); + + // When / Then — fail fast rather than reassemble a struct from a non-struct type + assertThatThrownBy(() -> sut.decode(ctx, layout, intType())) + .isInstanceOf(VortexException.class) + .hasMessageContaining("requires a struct dtype"); + } + + @Test + void decode_childCountMismatch_throwsVortexException() { + // Given — a two-field struct but only one child (neither nfields nor nfields+1) + DType.Struct dtype = struct(false, "a", intType(), "b", intType()); + Layout layout = new Layout(LayoutId.STRUCT, 1L, null, List.of(flat(0)), List.of()); + LayoutDecodeContext ctx = mock(LayoutDecodeContext.class); + + // When / Then — the shape is unrecoverable, so it fails loudly + assertThatThrownBy(() -> sut.decode(ctx, layout, dtype)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("1 children") + .hasMessageContaining("2 fields"); + } + + // ── helpers ─────────────────────────────────────────────────────────────── + + private static DType intType() { + return new DType.Primitive(PType.I32, false); + } + + private static DType.Struct struct(boolean nullable, String n0, DType t0, String n1, DType t1) { + return new DType.Struct( + List.of(ColumnName.of(n0), ColumnName.of(n1)), List.of(t0, t1), nullable); + } + + private static Layout flat(int segment) { + return new Layout(LayoutId.FLAT, 0L, null, List.of(), List.of(segment)); + } + + /// A concrete stand-in [Array] — the sealed interface cannot be mocked, and these tests only + /// need object identity. + private static Array sentinel() { + return new UnknownArray(EncodingId.parse("stub"), intType(), 0L, null, + new MemorySegment[0], new Array[0]); + } + + /// A minimal all-valid [BoolArray]; only its identity is asserted, never its contents. + private static BoolArray constantValidity() { + return new BoolArray() { + @Override + public boolean getBoolean(long i) { + return true; + } + + @Override + public long length() { + return 4L; + } + + @Override + public DType dtype() { + return DType.BOOL; + } + }; + } +}