From cf5e28868b124f332042795fbe739fa9be65e593 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sun, 1 Nov 2026 09:34:44 +0100 Subject: [PATCH 1/6] refactor: extension metadata is a typed record, parsed and serialized at the wire DType.Extension held its metadata as a MemorySegment: a buffer view inside a value type. It needed a hand-written byte-comparing equals/hashCode, a copy off the file mapping so a dtype could outlive its reader, and every consumer re-parsed the bytes (TimestampDtype, TimeDtype, ExtensionStorage, the datetimeparts encoder and decoder). A heap segment from a caller was not copied, so changing its array changed the dtype and its hash. Rust holds an extension's ExtVTable::Metadata, typed, and converts to bytes only in the vtable's serialize_metadata / deserialize_metadata. So does Java now: - core keeps only the types: ExtensionMetadata (an open interface, an extension point; DType.Extension requires a record so dtypes compare by value), TimeUnitMetadata for date and time, TimestampMetadata(TimeUnit, ZoneId) for timestamp, and OpaqueMetadata for bytes nothing parses (Rust's ForeignExtMetadata: uuid's version byte, the st.* SpatialMetadata, foreign ids), which keeps them so a rewritten file loses nothing. - the reader parses: ExtensionMetadataParser for the well-known ids, straight from the segment with no copy, and ExtensionDecoder#parseMetadata for any other registered id, run by ReadRegistry.resolveExtensions (was validateExtensions) when a file opens. - the writer serializes: DTypeFbsSerializer for the built-ins and opaque bytes, ExtensionEncoder#serializeMetadata for other records, checked at VortexWriter.create so metadata no encoder can serialize fails before any data is written. A timestamp zone java.time.ZoneId cannot parse now refuses the file at open. Calcite's Temporal read the unit of date and time columns through TimestampDtype.readUnit, which only worked while it read raw bytes; it uses ExtensionStorage.readUnit, which takes both. CustomExtensionIntegrationTest round-trips example.vector's dimension as a typed record through its own encoder and decoder; ExtensionMetadataParserTest takes over the wire-format cases that left ExtensionIdTest. Co-Authored-By: Claude Sonnet 5 --- AGENTS.md | 13 +- CHANGELOG.md | 1 + .../github/dfa1/vortex/calcite/Temporal.java | 6 +- .../dfa1/vortex/cli/SchemaCommandTest.java | 5 +- .../github/dfa1/vortex/cli/VortexCliTest.java | 3 +- .../dfa1/vortex/cli/tui/GridRenderTest.java | 5 +- .../vortex/cli/tui/InspectorRenderTest.java | 3 +- .../github/dfa1/vortex/core/model/DType.java | 72 ++--------- .../dfa1/vortex/core/model/ExtensionId.java | 53 ++++---- .../vortex/core/model/ExtensionMetadata.java | 12 ++ .../vortex/core/model/OpaqueMetadata.java | 65 ++++++++++ .../dfa1/vortex/core/model/TimeDtype.java | 21 ++- .../vortex/core/model/TimeUnitMetadata.java | 16 +++ .../vortex/core/model/TimestampDtype.java | 54 +++----- .../vortex/core/model/TimestampMetadata.java | 20 +++ .../vortex/core/model/DTypeExtensionTest.java | 121 +++++++++++++----- .../core/model/DTypeWithNullableTest.java | 6 +- .../vortex/core/model/ExtensionIdTest.java | 64 ++++----- docs/compatibility.md | 5 +- docs/how-to.md | 60 +++++---- docs/reference.md | 12 +- .../CustomExtensionIntegrationTest.java | 76 ++++++++--- .../DateInteropIntegrationTest.java | 11 +- .../JavaWritesRustReadsIntegrationTest.java | 18 ++- ...KnownExtensionsInteropIntegrationTest.java | 4 +- .../vortex/parquet/ColumnBuilderTest.java | 3 +- .../vortex/parquet/ParquetExporterTest.java | 3 +- .../dfa1/vortex/reader/PostscriptParser.java | 6 +- .../dfa1/vortex/reader/ReadRegistry.java | 73 +++++++---- .../dfa1/vortex/reader/VortexHttpReader.java | 4 +- .../dfa1/vortex/reader/VortexReader.java | 4 +- .../decode/DateTimePartsEncodingDecoder.java | 9 +- .../reader/decode/VariantEncodingDecoder.java | 17 ++- .../extension/DateExtensionDecoder.java | 4 +- .../reader/extension/ExtensionDecoder.java | 17 +++ .../extension/ExtensionMetadataParser.java | 82 ++++++++++++ .../reader/extension/ExtensionStorage.java | 19 +-- .../extension/UuidExtensionDecoder.java | 5 +- .../dfa1/vortex/reader/ReadRegistryTest.java | 100 +++++++++++---- .../vortex/reader/compute/ValuesTest.java | 2 +- .../decode/ConstantEncodingDecoderTest.java | 19 ++- .../DateTimePartsEncodingDecoderTest.java | 8 +- .../decode/VariantEncodingDecoderTest.java | 7 +- .../extension/DateExtensionDecoderTest.java | 6 +- .../ExtensionMetadataParserTest.java | 77 +++++++++++ .../extension/ExtensionStorageTest.java | 38 +++--- .../extension/ExtensionTestSupport.java | 4 +- .../extension/TimeExtensionDecoderTest.java | 4 +- .../TimestampExtensionDecoderTest.java | 20 ++- .../extension/UuidExtensionDecoderTest.java | 3 +- .../reader/layout/DictLayoutDecoderTest.java | 5 +- .../reader/layout/ZonedStatsSchemaTest.java | 2 +- .../vortex/writer/DTypeFbsSerializer.java | 61 ++++++++- .../dfa1/vortex/writer/ExtensionEncoder.java | 19 +++ .../dfa1/vortex/writer/VortexWriter.java | 5 +- .../writer/encode/DateExtensionEncoder.java | 6 +- .../encode/DateTimePartsEncodingEncoder.java | 17 ++- .../writer/encode/UuidExtensionEncoder.java | 7 +- .../dfa1/vortex/writer/WriterZoneMapTest.java | 2 +- .../vortex/writer/ZoneMapStatCodecTest.java | 5 +- .../DateTimePartsEncodingEncoderTest.java | 7 +- .../writer/encode/ExtEncodingEncoderTest.java | 16 ++- .../encode/TimeExtensionEncoderTest.java | 5 +- .../encode/ZoneMapStatsFitnessTest.java | 3 +- 64 files changed, 954 insertions(+), 466 deletions(-) create mode 100644 core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java create mode 100644 core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java create mode 100644 core/src/main/java/io/github/dfa1/vortex/core/model/TimeUnitMetadata.java create mode 100644 core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java create mode 100644 reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java create mode 100644 reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java diff --git a/AGENTS.md b/AGENTS.md index c60be724b..c78ea9382 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -302,9 +302,16 @@ When stuck on encode/decode behavior, consult **in this order**: - **DType is pluggable only via `Extension`.** `DType` is a sealed interface; downstream code must not add variants. Use `new DType.Extension("ip.address", new DType.Primitive(PType.I32, false), - null, false)` and register an `ExtensionDecoder`/`ExtensionEncoder` on `ReadRegistry`/`WriteRegistry`. - Mirrors Rust's `ExtVTable` (`vortex.date`, `vortex.uuid`, …): an unregistered id fails the read - unless `allowUnknown()`, then reads as storage. No SPI for DType variants planned. + OpaqueMetadata.EMPTY, false)` and register an `ExtensionDecoder`/`ExtensionEncoder` on + `ReadRegistry`/`WriteRegistry`. Mirrors Rust's `ExtVTable` (`vortex.date`, `vortex.uuid`, …): an + unregistered id fails the read unless `allowUnknown()`, then reads as storage. No SPI for DType + variants planned. +- **Extension metadata is typed, bytes exist only at the wire.** `DType.Extension#metadata()` is an + `ExtensionMetadata`, an open interface whose implementations must be records (checked by + `DType.Extension`), so dtypes compare by value. As Rust's `ExtVTable::{de,}serialize_metadata`, + the reader parses (`ExtensionMetadataParser` for the well-known ids, `ExtensionDecoder#parseMetadata` + for the rest, at open) and the writer serializes (`DTypeFbsSerializer`, `ExtensionEncoder#serializeMetadata`); + `core` holds only the types. Bytes nothing parses stay `OpaqueMetadata` (Rust's `ForeignExtMetadata`). - **Encoding and layout decode are both pluggable via builder-only registries** — no `ServiceLoader`. Encodings register on `ReadRegistry`/`WriteRegistry` (`ReadRegistry.builder().registerDefaults().register(custom).build()`); layouts register on diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ad8bad26..c69906665 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking:** `ScanOptions.filter()` and `limit()` return `Optional` and `Optional`, as Rust's `ScanBuilder` holds them; `withLimit` takes a `ScanLimit`, and `NO_LIMIT`, `ScanOptions.limit(long)`, `rowFilter()`, `hasFilter()` and `hasLimit()` are gone. ([#598](https://github.com/dfa1/vortex-java/pull/598)) - **Breaking:** CSV import stores an empty cell in a number or boolean column as null instead of `0` or `false`, so inferred number and boolean columns are now nullable (read through `MaskedArray`), and an empty cell or a missing column fails the import where the schema declares the column non-nullable. ([#592](https://github.com/dfa1/vortex-java/issues/592)) - **Breaking:** the shared non-nullable dtypes move from `DType` to the new `DTypes` class: `DTypes.I64`, `DTypes.UTF8`, … ([#624](https://github.com/dfa1/vortex-java/pull/624)) +- **Breaking:** an extension dtype's metadata is a typed `ExtensionMetadata` record instead of a `MemorySegment`, parsed when a file opens and serialized when it is written, as Rust does: `TimeUnitMetadata` for date and time, `TimestampMetadata` with a `ZoneId` for timestamp, `OpaqueMetadata` for bytes nothing parses, and a third-party record through `ExtensionDecoder#parseMetadata` and `ExtensionEncoder#serializeMetadata`. ([#625](https://github.com/dfa1/vortex-java/pull/625)) ### Fixed - Two threads first using `DType` at the same time no longer deadlock the JVM, as they could when, for example, two Parquet imports started together. ([#624](https://github.com/dfa1/vortex-java/pull/624)) diff --git a/calcite/src/main/java/io/github/dfa1/vortex/calcite/Temporal.java b/calcite/src/main/java/io/github/dfa1/vortex/calcite/Temporal.java index 9b0c11ac1..1bf168b48 100644 --- a/calcite/src/main/java/io/github/dfa1/vortex/calcite/Temporal.java +++ b/calcite/src/main/java/io/github/dfa1/vortex/calcite/Temporal.java @@ -4,12 +4,12 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.ExtensionId; import io.github.dfa1.vortex.core.model.TimeUnit; -import io.github.dfa1.vortex.core.model.TimestampDtype; import io.github.dfa1.vortex.reader.array.ByteArray; import io.github.dfa1.vortex.reader.array.IntArray; import io.github.dfa1.vortex.reader.array.LongArray; import io.github.dfa1.vortex.reader.array.MaskedArray; import io.github.dfa1.vortex.reader.array.ShortArray; +import io.github.dfa1.vortex.reader.extension.ExtensionStorage; /// A `vortex.date`, `vortex.time` or `vortex.timestamp` column's conversion from its raw storage /// integer to Calcite's internal form: `DATE` as epoch days (`Integer`), `TIME` as milliseconds of @@ -36,8 +36,8 @@ static Temporal of(DType type) { && kind != ExtensionId.WellKnown.VORTEX_TIMESTAMP) { return null; } - // the unit byte leads every temporal extension's metadata - TimeUnit unit = TimestampDtype.readUnit(ext); + // date and time carry a TimeUnitMetadata, timestamp a TimestampMetadata: both have the unit + TimeUnit unit = ExtensionStorage.readUnit(ext); boolean valid = switch (kind) { case VORTEX_DATE -> unit == TimeUnit.Days || unit == TimeUnit.Milliseconds; default -> unit != TimeUnit.Days; diff --git a/cli/src/test/java/io/github/dfa1/vortex/cli/SchemaCommandTest.java b/cli/src/test/java/io/github/dfa1/vortex/cli/SchemaCommandTest.java index 1cdb80855..373f68bf8 100644 --- a/cli/src/test/java/io/github/dfa1/vortex/cli/SchemaCommandTest.java +++ b/cli/src/test/java/io/github/dfa1/vortex/cli/SchemaCommandTest.java @@ -3,6 +3,7 @@ import io.github.dfa1.vortex.core.model.ColumnName; import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.DTypes; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.core.model.PType; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -90,8 +91,8 @@ static Stream dtypeCases() { arguments(new DType.Map(DTypes.UTF8, i64, true, false), "map"), arguments(new DType.Map(DTypes.UTF8, new DType.Primitive(PType.I64, true), false, false), "map"), - arguments(new DType.Extension("vortex.uuid", i64, null, false), "ext"), - arguments(new DType.Extension("vortex.uuid", i64, null, true), "ext?"), + arguments(new DType.Extension("vortex.uuid", i64, OpaqueMetadata.EMPTY, false), "ext"), + arguments(new DType.Extension("vortex.uuid", i64, OpaqueMetadata.EMPTY, true), "ext?"), arguments(DTypes.VARIANT, "variant"), arguments(new DType.Variant(true), "variant?"), arguments(new DType.Struct(List.of(ColumnName.of("a"), ColumnName.of("b")), List.of(i64, DTypes.UTF8), false), diff --git a/cli/src/test/java/io/github/dfa1/vortex/cli/VortexCliTest.java b/cli/src/test/java/io/github/dfa1/vortex/cli/VortexCliTest.java index 17322915d..cbbce164c 100644 --- a/cli/src/test/java/io/github/dfa1/vortex/cli/VortexCliTest.java +++ b/cli/src/test/java/io/github/dfa1/vortex/cli/VortexCliTest.java @@ -161,7 +161,8 @@ private static Path writeCustomExtensionVortex(Path dir) throws IOException { Path file = dir.resolve("money.vortex"); DType.Struct schema = new DType.Struct(List.of(ColumnName.of("price")), List.of(new DType.Extension("acme.money", DTypes.I64, - java.lang.foreign.MemorySegment.ofArray("EUR".getBytes(StandardCharsets.US_ASCII)), false)), + new io.github.dfa1.vortex.core.model.OpaqueMetadata( + java.lang.foreign.MemorySegment.ofArray("EUR".getBytes(StandardCharsets.US_ASCII))), false)), false); try (FileChannel ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE); VortexWriter writer = VortexWriter.create(ch, schema, WriteOptions.defaults())) { diff --git a/cli/src/test/java/io/github/dfa1/vortex/cli/tui/GridRenderTest.java b/cli/src/test/java/io/github/dfa1/vortex/cli/tui/GridRenderTest.java index 835466564..93cd160a3 100644 --- a/cli/src/test/java/io/github/dfa1/vortex/cli/tui/GridRenderTest.java +++ b/cli/src/test/java/io/github/dfa1/vortex/cli/tui/GridRenderTest.java @@ -206,7 +206,8 @@ void extensionDecodeFailureRendersDiagnostic() { try (Arena arena = Arena.ofConfined()) { // Given — vortex.date declared but storage is F64, which epochInteger rejects DType dateExt = new DType.Extension("vortex.date", - DTypes.I32, null, false); + DTypes.I32, new io.github.dfa1.vortex.core.model.TimeUnitMetadata( + io.github.dfa1.vortex.core.model.TimeUnit.Days), false); Array badStorage = ArrayFixtures.doubles(arena, 1.5); // When — the decode throws and is caught @@ -222,7 +223,7 @@ void unknownExtensionIdFallsThroughToStorageRendering() { try (Arena arena = Arena.ofConfined()) { // Given — an extension id this reader does not know: render the raw storage value DType unknownExt = new DType.Extension("custom.thing", - DTypes.I32, null, false); + DTypes.I32, io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, false); IntArray storage = ArrayFixtures.ints(arena, 42); // When / Then — no extension formatting, falls to the IntArray branch diff --git a/cli/src/test/java/io/github/dfa1/vortex/cli/tui/InspectorRenderTest.java b/cli/src/test/java/io/github/dfa1/vortex/cli/tui/InspectorRenderTest.java index 70fa380df..8b8b483a9 100644 --- a/cli/src/test/java/io/github/dfa1/vortex/cli/tui/InspectorRenderTest.java +++ b/cli/src/test/java/io/github/dfa1/vortex/cli/tui/InspectorRenderTest.java @@ -147,7 +147,8 @@ void dateExtensionDecodeFailureFallsThroughToGeneric() { try (Arena arena = Arena.ofConfined()) { // Given — vortex.date declared but storage is F64; the decode throws and is swallowed DType dateExt = new DType.Extension("vortex.date", - DTypes.I32, null, false); + DTypes.I32, new io.github.dfa1.vortex.core.model.TimeUnitMetadata( + io.github.dfa1.vortex.core.model.TimeUnit.Days), false); Array badStorage = ArrayFixtures.doubles(arena, 1.5); // When / Then — falls through to the generic DoubleArray rendering diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/DType.java b/core/src/main/java/io/github/dfa1/vortex/core/model/DType.java index 0ac80e511..062b83128 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/DType.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/DType.java @@ -2,7 +2,6 @@ import io.github.dfa1.vortex.core.error.VortexException; -import java.lang.foreign.MemorySegment; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.Objects; @@ -338,78 +337,25 @@ public Struct entriesDtype() { /// /// @param extensionId unique string identifier for the extension type (e.g. `"vortex.timestamp"`) /// @param storageDType underlying storage dtype used for physical encoding - /// @param metadata opaque extension-specific metadata bytes, or `null` + /// @param metadata the extension's metadata, [OpaqueMetadata#EMPTY] for none /// @param nullable whether null values are permitted record Extension( String extensionId, DType storageDType, - MemorySegment metadata, + ExtensionMetadata metadata, boolean nullable ) implements DType { - /// Hard cap on extension metadata size. Spec-defined extensions use 0–3 bytes - /// (e.g. timestamp = TimeUnit tag + tz length + tz UTF-8); 64 KiB is generous - /// for custom extensions while blocking attacker-supplied multi-megabyte blobs - /// that would inflate parser allocations. - public static final MemorySize MAX_METADATA_SIZE = MemorySize.ofKiB(64); - - /// Bounds the metadata blob, which arrives from an untrusted file. + /// Requires metadata that is a record, so the dtype compares and hashes by value. /// - /// @throws VortexException if `metadata` carries more than - /// [#MAX_METADATA_SIZE] readable bytes + /// @throws NullPointerException if `metadata` is `null` + /// @throws IllegalArgumentException if `metadata` is not a record public Extension { - if (metadata != null && metadata.byteSize() > MAX_METADATA_SIZE.bytes()) { - throw new VortexException("extension metadata too large: " - + metadata.byteSize() + " > " + MAX_METADATA_SIZE.bytes()); - } - // A dtype outlives the reader that parsed it (callers keep reader.dtype()), but a slice - // of the file mapping fails once the reader closes: copy it onto the heap. Small, as - // the cap above bounds it. - if (metadata != null && metadata.isNative()) { - metadata = MemorySegment.ofArray(metadata.toArray(java.lang.foreign.ValueLayout.JAVA_BYTE)) - .asReadOnly(); - } - } - - /// Value equality with content-based metadata comparison. - /// - /// [MemorySegment] uses identity equality, so the default record `equals` would - /// treat a heap-built dtype and the same dtype read back from a mapped file as - /// unequal. Extension is a value type (compared in schema round-trips and as a - /// component of [Struct]/[List] dtypes), so metadata is compared by bytes — - /// matching the content semantics the previous `ByteBuffer` component had. - /// - /// @param o other object - /// @return whether `o` is an Extension with equal id, storage, nullability and metadata bytes - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof Extension(String id, DType dType, MemorySegment metadata1, boolean nullable1))) { - return false; - } - return nullable == nullable1 - && extensionId.equals(id) - && storageDType.equals(dType) - && metadataEquals(metadata, metadata1); - } - - /// @return a hash consistent with [#equals(Object)] (metadata hashed by content) - @Override - public int hashCode() { - int h = java.util.Objects.hash(extensionId, storageDType, nullable); - if (metadata != null) { - h = 31 * h + java.util.Arrays.hashCode(metadata.toArray(java.lang.foreign.ValueLayout.JAVA_BYTE)); - } - return h; - } - - private static boolean metadataEquals(MemorySegment a, MemorySegment b) { - if (a == null || b == null) { - return a == b; + Objects.requireNonNull(metadata, "metadata"); + if (!metadata.getClass().isRecord()) { + throw new IllegalArgumentException( + "extension metadata must be a record, got " + metadata.getClass().getName()); } - return a.byteSize() == b.byteSize() && a.mismatch(b) == -1; } } diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionId.java b/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionId.java index d182e58ca..52d9f4e2c 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionId.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionId.java @@ -3,10 +3,8 @@ import io.github.dfa1.vortex.core.error.VortexException; import java.io.Serializable; +import java.lang.foreign.MemorySegment; import java.lang.foreign.ValueLayout; -import java.nio.ByteBuffer; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.Objects; import java.util.function.Function; @@ -118,11 +116,10 @@ public String toString() { /// @param dtype an extension dtype with this id /// @throws VortexException if the metadata or storage dtype is invalid for this extension public void validate(DType.Extension dtype) { - byte[] meta = dtype.metadata() == null ? new byte[0] : dtype.metadata().toArray(ValueLayout.JAVA_BYTE); DType storage = dtype.storageDType(); switch (this) { case VORTEX_DATE -> { - TimeUnit unit = unit(meta); + TimeUnit unit = unit(dtype.metadata()); PType expected = switch (unit) { case Days -> PType.I32; case Milliseconds -> PType.I64; @@ -132,7 +129,7 @@ public void validate(DType.Extension dtype) { requirePtype(storage, expected, unit); } case VORTEX_TIME -> { - TimeUnit unit = unit(meta); + TimeUnit unit = unit(dtype.metadata()); PType expected = switch (unit) { case Seconds, Milliseconds -> PType.I32; case Microseconds, Nanoseconds -> PType.I64; @@ -141,31 +138,22 @@ public void validate(DType.Extension dtype) { requirePtype(storage, expected, unit); } case VORTEX_TIMESTAMP -> { - if (meta.length < 3) { - throw new VortexException(id + " metadata must have at least 3 bytes, got " + meta.length); - } - unit(meta); - int tzLen = Byte.toUnsignedInt(meta[1]) | Byte.toUnsignedInt(meta[2]) << 8; - if (meta.length < 3 + tzLen) { - throw new VortexException(id + " metadata is truncated: declared timezone length " - + tzLen + " but only " + (meta.length - 3) + " bytes available"); - } - try { - StandardCharsets.UTF_8.newDecoder().decode(ByteBuffer.wrap(meta, 3, tzLen)); - } catch (CharacterCodingException e) { - throw new VortexException(id + " timezone is not valid UTF-8", e); + if (!(dtype.metadata() instanceof TimestampMetadata)) { + throw new VortexException(id + " metadata must be TimestampMetadata, got " + dtype.metadata()); } if (!(storage instanceof DType.Primitive p && p.ptype() == PType.I64)) { throw new VortexException(id + " storage dtype must be I64, got " + storage); } } case VORTEX_UUID -> { - if (meta.length > 1) { - throw new VortexException(id + " metadata must be 0 or 1 bytes, got " + meta.length); + MemorySegment meta = opaque(dtype.metadata()); + if (meta.byteSize() > 1) { + throw new VortexException(id + " metadata must be 0 or 1 bytes, got " + meta.byteSize()); } // uuid crate versions 0-8, plus Max as 0x0f or the older 0xff - if (meta.length == 1 && !(meta[0] >= 0 && meta[0] <= 8 || meta[0] == 0x0f || meta[0] == (byte) 0xff)) { - throw new VortexException("unknown UUID version discriminant: " + Byte.toUnsignedInt(meta[0])); + byte version = meta.byteSize() == 1 ? meta.get(ValueLayout.JAVA_BYTE, 0) : 0; + if (!(version >= 0 && version <= 8 || version == 0x0f || version == (byte) 0xff)) { + throw new VortexException("unknown UUID version discriminant: " + Byte.toUnsignedInt(version)); } if (!(storage instanceof DType.FixedSizeList list && list.fixedSize() == 16 && list.elementType() instanceof DType.Primitive p && p.ptype() == PType.U8 && !p.nullable())) { @@ -173,7 +161,7 @@ public void validate(DType.Extension dtype) { } } case VORTEX_JSON -> { - if (meta.length != 0) { + if (opaque(dtype.metadata()).byteSize() != 0) { throw new VortexException(id + " metadata must be empty"); } if (!(storage instanceof DType.Utf8)) { @@ -225,15 +213,18 @@ private void requireF64Struct(DType dtype, java.util.List } } - private TimeUnit unit(byte[] meta) { - if (meta.length == 0) { - throw new VortexException(id + " metadata must not be empty"); + private MemorySegment opaque(ExtensionMetadata metadata) { + if (!(metadata instanceof OpaqueMetadata(MemorySegment bytes))) { + throw new VortexException(id + " metadata must be OpaqueMetadata, got " + metadata); } - try { - return TimeUnit.fromTag(meta[0]); - } catch (IllegalArgumentException e) { - throw new VortexException(id + " metadata has an invalid time unit", e); + return bytes; + } + + private TimeUnit unit(ExtensionMetadata metadata) { + if (!(metadata instanceof TimeUnitMetadata(TimeUnit unit))) { + throw new VortexException(id + " metadata must be TimeUnitMetadata, got " + metadata); } + return unit; } private void requirePtype(DType storage, PType expected, TimeUnit unit) { diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java new file mode 100644 index 000000000..7d05d2403 --- /dev/null +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java @@ -0,0 +1,12 @@ +package io.github.dfa1.vortex.core.model; + +/// The metadata of a [DType.Extension], as Rust holds an extension's `ExtVTable::Metadata` rather +/// than its bytes. The reader parses it from the bytes a file stores and the writer serializes it +/// back; nothing in between sees the bytes. +/// +/// An extension point: a third-party extension defines its own. It must be a **record**, so a +/// dtype compares and hashes by value ([DType.Extension] refuses anything else). The well-known +/// temporal extensions use [TimeUnitMetadata] and [TimestampMetadata]; every other id keeps its +/// bytes as [OpaqueMetadata] (Rust's `ForeignExtMetadata`). +public interface ExtensionMetadata { +} diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java new file mode 100644 index 000000000..9620b43a8 --- /dev/null +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java @@ -0,0 +1,65 @@ +package io.github.dfa1.vortex.core.model; + +import io.github.dfa1.vortex.core.error.VortexException; + +import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; +import java.util.Arrays; +import java.util.HexFormat; + +/// Extension metadata kept as the bytes a file stores, for an id nothing parses: Rust's +/// `ForeignExtMetadata`. +/// +/// Why bytes and not `null`: a file can carry metadata Java does not interpret, and a dtype read +/// from it must keep that metadata so writing it back loses nothing, as Rust keeps it. That covers +/// the `vortex.uuid` version byte, the `vortex.st.*` geometries' SpatialMetadata protobuf (CRS, +/// edges), any custom id read with `allowUnknown()` or by a decoder that does not parse its +/// metadata, and a writer with no encoder for the id, for which these bytes are the only metadata +/// it can supply. A `null` would drop them: the file would read back in Rust as a different dtype +/// (a geometry without its CRS). [#EMPTY] is the metadata of an extension that has none, written +/// as the empty field Rust requires. +/// +/// @param bytes the wire bytes, read-only, at most [#MAX_SIZE] +public record OpaqueMetadata(MemorySegment bytes) implements ExtensionMetadata { + + /// Hard cap on extension metadata size. Spec-defined extensions use 0–3 bytes (e.g. timestamp + /// = TimeUnit tag + tz length + tz UTF-8); 64 KiB is generous for custom extensions while + /// blocking attacker-supplied multi-megabyte blobs that would inflate parser allocations. + public static final MemorySize MAX_SIZE = MemorySize.ofKiB(64); + + /// No metadata. + public static final OpaqueMetadata EMPTY = new OpaqueMetadata(MemorySegment.ofArray(new byte[0])); + + /// Bounds the bytes, which may arrive from an untrusted file, and keeps them read-only. + /// + /// @param bytes the wire bytes + /// @throws VortexException if `bytes` is longer than [#MAX_SIZE] + public OpaqueMetadata { + if (bytes.byteSize() > MAX_SIZE.bytes()) { + throw new VortexException("extension metadata too large: " + bytes.byteSize() + " > " + MAX_SIZE.bytes()); + } + // A dtype outlives the reader that parsed it (callers keep reader.dtype()), but a slice of + // the file mapping fails once the reader closes: copy that one onto the heap. Small, as the + // cap above bounds it; a heap segment is kept as it is. + if (bytes.isNative()) { + bytes = MemorySegment.ofArray(bytes.toArray(ValueLayout.JAVA_BYTE)); + } + bytes = bytes.asReadOnly(); + } + + @Override + public boolean equals(Object o) { + return o instanceof OpaqueMetadata(MemorySegment other) + && bytes.byteSize() == other.byteSize() && bytes.mismatch(other) == -1; + } + + @Override + public int hashCode() { + return Arrays.hashCode(bytes.toArray(ValueLayout.JAVA_BYTE)); + } + + @Override + public String toString() { + return "OpaqueMetadata[" + HexFormat.of().formatHex(bytes.toArray(ValueLayout.JAVA_BYTE)) + "]"; + } +} diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/TimeDtype.java b/core/src/main/java/io/github/dfa1/vortex/core/model/TimeDtype.java index 2d92c7d8b..cb8307fe1 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/TimeDtype.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/TimeDtype.java @@ -2,13 +2,10 @@ import io.github.dfa1.vortex.core.error.VortexException; -import java.lang.foreign.MemorySegment; -import java.lang.foreign.ValueLayout; - /// Static factories and metadata accessors for `vortex.time` extension dtypes. /// -/// Wire format: a single byte holding the [TimeUnit] tag. Storage width is -/// derived from the unit (I32 for s/ms, I64 for μs/ns). +/// The metadata is a [TimeUnitMetadata]. Storage width is derived from the unit (I32 for s/ms, I64 +/// for μs/ns). public final class TimeDtype { private TimeDtype() { @@ -34,24 +31,22 @@ public static DType.Extension of(TimeUnit unit, boolean nullable) { case Microseconds, Nanoseconds -> PType.I64; case Days -> throw new IllegalArgumentException("Days unit not valid for vortex.time"); }; - MemorySegment meta = MemorySegment.ofArray(new byte[]{(byte) unit.ordinal()}); return new DType.Extension( ExtensionId.VORTEX_TIME.id(), new DType.Primitive(storage, nullable), - meta, + new TimeUnitMetadata(unit), nullable); } - /// Reads the [TimeUnit] tag from the metadata's first byte. + /// Returns the [TimeUnit] the dtype's metadata records. /// /// @param ext declared extension dtype /// @return the recorded time unit - /// @throws VortexException if metadata is missing or empty + /// @throws VortexException if the metadata is not a [TimeUnitMetadata] public static TimeUnit readUnit(DType.Extension ext) { - MemorySegment meta = ext.metadata(); - if (meta == null || meta.byteSize() == 0) { - throw new VortexException("missing TimeUnit metadata byte for " + ext.extensionId()); + if (!(ext.metadata() instanceof TimeUnitMetadata(TimeUnit unit))) { + throw new VortexException("missing TimeUnit metadata for " + ext.extensionId()); } - return TimeUnit.fromTag(meta.get(ValueLayout.JAVA_BYTE, 0)); + return unit; } } diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/TimeUnitMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/TimeUnitMetadata.java new file mode 100644 index 000000000..6bb457743 --- /dev/null +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/TimeUnitMetadata.java @@ -0,0 +1,16 @@ +package io.github.dfa1.vortex.core.model; + +import java.util.Objects; + +/// The metadata of `vortex.date` and `vortex.time`: the [TimeUnit] their storage counts in. +/// +/// @param unit the storage's time unit +public record TimeUnitMetadata(TimeUnit unit) implements ExtensionMetadata { + + /// Requires a unit. + /// + /// @param unit the storage's time unit + public TimeUnitMetadata { + Objects.requireNonNull(unit, "unit"); + } +} diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampDtype.java b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampDtype.java index 3d1983857..398283ee1 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampDtype.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampDtype.java @@ -2,19 +2,11 @@ import io.github.dfa1.vortex.core.error.VortexException; -import static io.github.dfa1.vortex.core.io.VortexFormat.LE_SHORT; - -import java.lang.foreign.MemorySegment; -import java.lang.foreign.ValueLayout; -import java.nio.charset.StandardCharsets; import java.time.ZoneId; import java.util.Optional; -/// Static factories and metadata accessors for `vortex.timestamp` extension dtypes. -/// -/// Wire format: `byte[0] = TimeUnit tag, bytes[1..3] = tz_len (u16 LE), -/// bytes[3..3+tz_len] = tz UTF-8`. Shared by reader and writer so the -/// encode/decode sides cannot drift. +/// Static factories and metadata accessors for `vortex.timestamp` extension dtypes, whose metadata +/// is a [TimestampMetadata]. public final class TimestampDtype { private TimestampDtype() { @@ -35,51 +27,35 @@ public static DType.Extension of(boolean nullable) { /// @param nullable whether the column allows nulls /// @return matching extension dtype public static DType.Extension of(TimeUnit unit, ZoneId zone, boolean nullable) { - byte[] tzBytes = zone == null ? new byte[0] : zone.getId().getBytes(StandardCharsets.UTF_8); - MemorySegment meta = MemorySegment.ofArray(new byte[3 + tzBytes.length]); - meta.set(ValueLayout.JAVA_BYTE, 0, (byte) unit.ordinal()); - meta.set(LE_SHORT, 1, (short) tzBytes.length); - MemorySegment.copy(tzBytes, 0, meta, ValueLayout.JAVA_BYTE, 3, tzBytes.length); return new DType.Extension( ExtensionId.VORTEX_TIMESTAMP.id(), new DType.Primitive(PType.I64, nullable), - meta, + new TimestampMetadata(unit, zone), nullable); } - /// Reads the [TimeUnit] tag from the metadata's first byte. + /// Returns the [TimeUnit] the dtype's metadata records. /// /// @param ext declared extension dtype /// @return the recorded time unit - /// @throws VortexException if metadata is missing or empty + /// @throws VortexException if the metadata is not a [TimestampMetadata] public static TimeUnit readUnit(DType.Extension ext) { - MemorySegment meta = ext.metadata(); - if (meta == null || meta.byteSize() == 0) { - throw new VortexException("missing TimeUnit metadata byte for " + ext.extensionId()); - } - return TimeUnit.fromTag(meta.get(ValueLayout.JAVA_BYTE, 0)); + return metadata(ext).unit(); } - /// Reads the optional IANA timezone from the metadata's UTF-8 suffix. + /// Returns the timezone the dtype's metadata records. /// /// @param ext declared extension dtype /// @return parsed zone id, or empty when no timezone is recorded - /// @throws VortexException if the metadata is truncated mid-string + /// @throws VortexException if the metadata is not a [TimestampMetadata] public static Optional timezone(DType.Extension ext) { - MemorySegment meta = ext.metadata(); - if (meta == null || meta.byteSize() < 3) { - return Optional.empty(); - } - int tzLen = Short.toUnsignedInt(meta.get(LE_SHORT, 1)); - if (tzLen == 0) { - return Optional.empty(); - } - if (meta.byteSize() < 3 + tzLen) { - throw new VortexException( - "timestamp metadata truncated: declared tz_len=" - + tzLen + " but only " + (meta.byteSize() - 3) + " bytes available"); + return Optional.ofNullable(metadata(ext).zone()); + } + + private static TimestampMetadata metadata(DType.Extension ext) { + if (!(ext.metadata() instanceof TimestampMetadata metadata)) { + throw new VortexException("missing timestamp metadata for " + ext.extensionId()); } - byte[] tzBytes = meta.asSlice(3, tzLen).toArray(ValueLayout.JAVA_BYTE); - return Optional.of(ZoneId.of(new String(tzBytes, StandardCharsets.UTF_8))); + return metadata; } } diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java new file mode 100644 index 000000000..e0ebc00cd --- /dev/null +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java @@ -0,0 +1,20 @@ +package io.github.dfa1.vortex.core.model; + +import java.time.ZoneId; +import java.util.Objects; + +/// The metadata of `vortex.timestamp`, Rust's `TimestampOptions`: the [TimeUnit] its `I64` storage +/// counts in and an optional timezone. +/// +/// @param unit the storage's time unit +/// @param zone the timezone, or `null` for none +public record TimestampMetadata(TimeUnit unit, ZoneId zone) implements ExtensionMetadata { + + /// Requires a unit. + /// + /// @param unit the storage's time unit + /// @param zone the timezone, or `null` for none + public TimestampMetadata { + Objects.requireNonNull(unit, "unit"); + } +} diff --git a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java index 7a9c303e0..03209b0aa 100644 --- a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java +++ b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java @@ -2,9 +2,12 @@ import io.github.dfa1.vortex.core.error.VortexException; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import java.lang.foreign.Arena; import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -13,48 +16,100 @@ class DTypeExtensionTest { private static final DType.Primitive I32 = DTypes.I32; - @Test - void metadataAtCap_accepted() { - // Given — exact boundary must pass so legitimate large-but-bounded metadata - // is not falsely rejected - MemorySegment meta = MemorySegment.ofArray(new byte[(int) DType.Extension.MAX_METADATA_SIZE.bytes()]); + @Nested + class Metadata { - // When / Then - assertThat(new DType.Extension("acme.big", I32, meta, false).metadata().byteSize()) - .isEqualTo(DType.Extension.MAX_METADATA_SIZE.bytes()); - } + @Test + void nullMetadata_rejected() { + // Given / When / Then — no metadata is OpaqueMetadata.EMPTY, so a dtype always has one + assertThatThrownBy(() -> new DType.Extension("vortex.date", I32, null, false)) + .isInstanceOf(NullPointerException.class); + } + + @Test + void nonRecordMetadata_rejected() { + // Given metadata whose equality is identity: a dtype holding it would not compare by + // value, so two reads of one file would give unequal schemas + ExtensionMetadata identity = new ExtensionMetadata() { + }; - @Test - void metadataOverCap_rejected() { - // Given — one byte past the cap; defends parser against attacker-supplied - // multi-megabyte metadata blobs that would inflate parse-time allocations - MemorySegment meta = MemorySegment.ofArray(new byte[(int) DType.Extension.MAX_METADATA_SIZE.bytes() + 1]); + // When / Then + assertThatThrownBy(() -> new DType.Extension("acme.custom", I32, identity, false)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must be a record"); + } + + @Test + void sameOpaqueBytes_equalDtypes() { + // Given the same bytes from a heap array and from off-heap memory, as a dtype built in + // code and one read back from a file hold them + MemorySegment offHeap = Arena.ofAuto().allocate(2); + offHeap.set(ValueLayout.JAVA_BYTE, 0, (byte) 1); + offHeap.set(ValueLayout.JAVA_BYTE, 1, (byte) 2); + var built = new DType.Extension("acme.custom", I32, new OpaqueMetadata(MemorySegment.ofArray(new byte[]{1, 2})), false); - // When / Then - assertThatThrownBy(() -> new DType.Extension("acme.big", I32, meta, false)) - .isInstanceOf(VortexException.class) - .hasMessageContaining("metadata too large"); + // When + var result = new DType.Extension("acme.custom", I32, new OpaqueMetadata(offHeap), false); + + // Then + assertThat(result).isEqualTo(built).hasSameHashCodeAs(built); + } } - @Test - void nativeMetadata_outlivesItsArena() { - // Given metadata sliced from off-heap memory, as a reader parses it out of the file mapping - DType.Extension result; - try (java.lang.foreign.Arena arena = java.lang.foreign.Arena.ofConfined()) { - MemorySegment meta = arena.allocate(2); - meta.set(java.lang.foreign.ValueLayout.JAVA_BYTE, 0, (byte) 3); + @Nested + class Opaque { + + @Test + void atCap_accepted() { + // Given — exact boundary must pass so legitimate large-but-bounded metadata + // is not falsely rejected + MemorySegment bytes = MemorySegment.ofArray(new byte[(int) OpaqueMetadata.MAX_SIZE.bytes()]); // When - result = new DType.Extension("vortex.timestamp", DTypes.I64, meta, false); + OpaqueMetadata result = new OpaqueMetadata(bytes); + + // Then + assertThat(result.bytes().byteSize()).isEqualTo(OpaqueMetadata.MAX_SIZE.bytes()); } - // Then — the dtype is still readable after the arena (the reader) is closed - assertThat(result.metadata().get(java.lang.foreign.ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) 3); - } + @Test + void overCap_rejected() { + // Given — one byte past the cap; defends parser against attacker-supplied + // multi-megabyte metadata blobs that would inflate parse-time allocations + MemorySegment bytes = MemorySegment.ofArray(new byte[(int) OpaqueMetadata.MAX_SIZE.bytes() + 1]); - @Test - void nullMetadata_accepted() { - // Given / When / Then — date/uuid extensions carry no metadata; null must remain valid - assertThat(new DType.Extension("vortex.date", I32, null, false).metadata()).isNull(); + // When / Then + assertThatThrownBy(() -> new OpaqueMetadata(bytes)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("metadata too large"); + } + + @Test + void nativeBytes_outliveTheirArena() { + // Given bytes sliced from off-heap memory, as a reader parses them out of the file mapping + OpaqueMetadata result; + try (Arena arena = Arena.ofConfined()) { + MemorySegment bytes = arena.allocate(2); + bytes.set(ValueLayout.JAVA_BYTE, 0, (byte) 3); + + // When + result = new OpaqueMetadata(bytes); + } + + // Then — still readable after the arena (the reader) is closed + assertThat(result.bytes().get(ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) 3); + } + + @Test + void bytes_areReadOnly() { + // Given + OpaqueMetadata sut = new OpaqueMetadata(MemorySegment.ofArray(new byte[]{1})); + + // When + boolean result = sut.bytes().isReadOnly(); + + // Then — a caller cannot change a dtype through the segment it reads back + assertThat(result).isTrue(); + } } } diff --git a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeWithNullableTest.java b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeWithNullableTest.java index f7a5344d5..3bcf2fbbe 100644 --- a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeWithNullableTest.java +++ b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeWithNullableTest.java @@ -27,7 +27,7 @@ static Stream nonNullableSamples() { Arguments.of("Struct", new DType.Struct(List.of(ColumnName.of("a"), ColumnName.of("b")), List.of(i32, DTypes.UTF8), false)), Arguments.of("List", new DType.List(i32, false)), Arguments.of("FixedSizeList", new DType.FixedSizeList(i32, 4, false)), - Arguments.of("Extension", new DType.Extension("ip.address", i32, null, false)), + Arguments.of("Extension", new DType.Extension("ip.address", i32, OpaqueMetadata.EMPTY, false)), Arguments.of("Variant", DTypes.VARIANT) ); } @@ -97,7 +97,7 @@ void withNullable_preservesCompoundPayload_struct() { void withNullable_preservesCompoundPayload_extensionAndFixedSizeList() { // Given DType storage = DTypes.I32; - DType.Extension ext = new DType.Extension("ip.address", storage, null, false); + DType.Extension ext = new DType.Extension("ip.address", storage, OpaqueMetadata.EMPTY, false); DType.FixedSizeList fsl = new DType.FixedSizeList(storage, 16, false); // When @@ -107,7 +107,7 @@ void withNullable_preservesCompoundPayload_extensionAndFixedSizeList() { // Then assertThat(extResult.extensionId()).isEqualTo("ip.address"); assertThat(extResult.storageDType()).isEqualTo(storage); - assertThat(extResult.metadata()).isNull(); + assertThat(extResult.metadata()).isEqualTo(OpaqueMetadata.EMPTY); assertThat(fslResult.fixedSize()).isEqualTo(16); assertThat(fslResult.elementType()).isEqualTo(storage); } diff --git a/core/src/test/java/io/github/dfa1/vortex/core/model/ExtensionIdTest.java b/core/src/test/java/io/github/dfa1/vortex/core/model/ExtensionIdTest.java index b97761fe3..f83700930 100644 --- a/core/src/test/java/io/github/dfa1/vortex/core/model/ExtensionIdTest.java +++ b/core/src/test/java/io/github/dfa1/vortex/core/model/ExtensionIdTest.java @@ -10,6 +10,7 @@ import org.junit.jupiter.params.provider.ValueSource; import java.lang.foreign.MemorySegment; +import java.time.ZoneId; import java.util.List; import java.util.stream.Stream; @@ -113,20 +114,20 @@ class Validate { static Stream valid() { return Stream.of( - Arguments.of("date days I32", ext(ExtensionId.VORTEX_DATE, DTypes.I32, 4)), - Arguments.of("date millis I64", ext(ExtensionId.VORTEX_DATE, DTypes.I64, 2)), - Arguments.of("time seconds I32", ext(ExtensionId.VORTEX_TIME, DTypes.I32, 3)), - Arguments.of("time millis I32", ext(ExtensionId.VORTEX_TIME, DTypes.I32, 2)), - Arguments.of("time micros I64", ext(ExtensionId.VORTEX_TIME, DTypes.I64, 1)), - Arguments.of("time nanos I64", ext(ExtensionId.VORTEX_TIME, DTypes.I64, 0)), - Arguments.of("timestamp no tz", ext(ExtensionId.VORTEX_TIMESTAMP, DTypes.I64, 2, 0, 0)), - Arguments.of("timestamp tz", ext(ExtensionId.VORTEX_TIMESTAMP, DTypes.I64, 2, 3, 0, 'U', 'T', 'C')), - Arguments.of("timestamp days", ext(ExtensionId.VORTEX_TIMESTAMP, DTypes.I64, 4, 0, 0)), + Arguments.of("date days I32", unit(ExtensionId.VORTEX_DATE, DTypes.I32, TimeUnit.Days)), + Arguments.of("date millis I64", unit(ExtensionId.VORTEX_DATE, DTypes.I64, TimeUnit.Milliseconds)), + Arguments.of("time seconds I32", unit(ExtensionId.VORTEX_TIME, DTypes.I32, TimeUnit.Seconds)), + Arguments.of("time millis I32", unit(ExtensionId.VORTEX_TIME, DTypes.I32, TimeUnit.Milliseconds)), + Arguments.of("time micros I64", unit(ExtensionId.VORTEX_TIME, DTypes.I64, TimeUnit.Microseconds)), + Arguments.of("time nanos I64", unit(ExtensionId.VORTEX_TIME, DTypes.I64, TimeUnit.Nanoseconds)), + Arguments.of("timestamp no tz", timestamp(DTypes.I64, TimeUnit.Milliseconds, null)), + Arguments.of("timestamp tz", timestamp(DTypes.I64, TimeUnit.Milliseconds, ZoneId.of("UTC"))), + Arguments.of("timestamp days", timestamp(DTypes.I64, TimeUnit.Days, null)), Arguments.of("uuid no version", ext(ExtensionId.VORTEX_UUID, UUID_STORAGE)), Arguments.of("uuid v4", ext(ExtensionId.VORTEX_UUID, UUID_STORAGE, 4)), Arguments.of("uuid max 0x0f", ext(ExtensionId.VORTEX_UUID, UUID_STORAGE, 0x0f)), Arguments.of("uuid max 0xff", ext(ExtensionId.VORTEX_UUID, UUID_STORAGE, 0xff)), - Arguments.of("date nullable storage", ext(ExtensionId.VORTEX_DATE, DTypes.I32.asNullable(), 4)), + Arguments.of("date nullable storage", unit(ExtensionId.VORTEX_DATE, DTypes.I32.asNullable(), TimeUnit.Days)), Arguments.of("json", ext(ExtensionId.VORTEX_JSON, DTypes.UTF8)), Arguments.of("wkb", ext(ExtensionId.VORTEX_ST_WKB, DTypes.BINARY)), Arguments.of("point xy", ext(ExtensionId.VORTEX_ST_POINT, coords("x", "y"))), @@ -141,17 +142,17 @@ static Stream valid() { static Stream invalid() { return Stream.of( - Arguments.of(ext(ExtensionId.VORTEX_DATE, DTypes.I32), "metadata must not be empty"), - Arguments.of(ext(ExtensionId.VORTEX_DATE, DTypes.I32, 9), "invalid time unit"), - Arguments.of(ext(ExtensionId.VORTEX_DATE, DTypes.I64, 4), "for Days must be I32"), - Arguments.of(ext(ExtensionId.VORTEX_DATE, DTypes.I64, 0), "does not support time unit Nanoseconds"), - Arguments.of(ext(ExtensionId.VORTEX_DATE, DTypes.UTF8, 4), "must be I32"), - Arguments.of(ext(ExtensionId.VORTEX_TIME, DTypes.I64, 2), "for Milliseconds must be I32"), - Arguments.of(ext(ExtensionId.VORTEX_TIME, DTypes.I32, 4), "does not support time unit Days"), - Arguments.of(ext(ExtensionId.VORTEX_TIMESTAMP, DTypes.I32, 2, 0, 0), "storage dtype must be I64"), - Arguments.of(ext(ExtensionId.VORTEX_TIMESTAMP, DTypes.I64, 2), "at least 3 bytes"), - Arguments.of(ext(ExtensionId.VORTEX_TIMESTAMP, DTypes.I64, 2, 5, 0, 'U'), "truncated"), - Arguments.of(ext(ExtensionId.VORTEX_TIMESTAMP, DTypes.I64, 2, 1, 0, 0xc3), "not valid UTF-8"), + // the wire-format errors (empty, bad tag, truncated, not UTF-8) are the reader's + // ExtensionMetadataParser's: here the metadata is already typed + Arguments.of(ext(ExtensionId.VORTEX_DATE, DTypes.I32, 4), "must be TimeUnitMetadata"), + Arguments.of(unit(ExtensionId.VORTEX_DATE, DTypes.I64, TimeUnit.Days), "for Days must be I32"), + Arguments.of(unit(ExtensionId.VORTEX_DATE, DTypes.I64, TimeUnit.Nanoseconds), "does not support time unit Nanoseconds"), + Arguments.of(unit(ExtensionId.VORTEX_DATE, DTypes.UTF8, TimeUnit.Days), "must be I32"), + Arguments.of(unit(ExtensionId.VORTEX_TIME, DTypes.I64, TimeUnit.Milliseconds), "for Milliseconds must be I32"), + Arguments.of(unit(ExtensionId.VORTEX_TIME, DTypes.I32, TimeUnit.Days), "does not support time unit Days"), + Arguments.of(timestamp(DTypes.I32, TimeUnit.Milliseconds, null), "storage dtype must be I64"), + Arguments.of(unit(ExtensionId.VORTEX_TIMESTAMP, DTypes.I64, TimeUnit.Milliseconds), "must be TimestampMetadata"), + Arguments.of(unit(ExtensionId.VORTEX_UUID, UUID_STORAGE, TimeUnit.Days), "must be OpaqueMetadata"), Arguments.of(ext(ExtensionId.VORTEX_UUID, UUID_STORAGE, 4, 0), "0 or 1 bytes"), Arguments.of(ext(ExtensionId.VORTEX_UUID, UUID_STORAGE, 9), "unknown UUID version"), Arguments.of(ext(ExtensionId.VORTEX_UUID, new DType.FixedSizeList(DTypes.U8, 8, false)), "FixedSizeList"), @@ -192,15 +193,6 @@ void validate_invalidDtype_throws(DType.Extension dtype, String message) { .hasMessageContaining(message); } - @Test - void validate_nullMetadata_isTreatedAsEmpty() { - // Given — Java callers may pass null; it means "no metadata", which uuid allows - DType.Extension dtype = new DType.Extension("vortex.uuid", UUID_STORAGE, null, false); - - // When / Then - ExtensionId.VORTEX_UUID.validate(dtype); - } - private static final DType UUID_STORAGE = new DType.FixedSizeList(DTypes.U8, 16, false); /// A GeoArrow coordinate (or box) struct of non-nullable F64 fields. @@ -222,7 +214,17 @@ private static DType.Extension ext(ExtensionId id, DType storage, int... meta) { for (int i = 0; i < meta.length; i++) { bytes[i] = (byte) meta[i]; } - return new DType.Extension(id.id(), storage, MemorySegment.ofArray(bytes), storage.nullable()); + return new DType.Extension(id.id(), storage, new OpaqueMetadata(MemorySegment.ofArray(bytes)), + storage.nullable()); + } + + private static DType.Extension unit(ExtensionId id, DType storage, TimeUnit unit) { + return new DType.Extension(id.id(), storage, new TimeUnitMetadata(unit), storage.nullable()); + } + + private static DType.Extension timestamp(DType storage, TimeUnit unit, ZoneId zone) { + return new DType.Extension(ExtensionId.VORTEX_TIMESTAMP.id(), storage, new TimestampMetadata(unit, zone), + storage.nullable()); } } } diff --git a/docs/compatibility.md b/docs/compatibility.md index 0826e9acd..95e5f7d0f 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -321,7 +321,10 @@ built-ins apply Rust's `deserialize_metadata` + `validate_dtype` rules `vortex.time` s/ms → `I32`, μs/ns → `I64`; `vortex.timestamp` metadata ≥ 3 bytes with a UTF-8 zone and `I64` storage; `vortex.uuid` 0–1 metadata bytes (a known version) over `FixedSizeList(non-null U8, 16)`. An extension dtype must carry its metadata field, empty when -there is none, as Rust requires; the writer always emits it. +there is none, as Rust requires; the writer always emits it. As in Rust, the metadata is parsed when +the file opens (a malformed one, or a timestamp zone `java.time.ZoneId` cannot parse, refuses the +file) and kept typed; an id nothing parses keeps its bytes (`OpaqueMetadata`, Rust's +`ForeignExtMetadata`). `ExtensionId` is a sealed `WellKnown`/`Custom` type like `EncodingId`: the spec ids (`VORTEX_DATE`, `VORTEX_TIME`, `VORTEX_TIMESTAMP`, `VORTEX_UUID`) plus any third-party id. diff --git a/docs/how-to.md b/docs/how-to.md index 2a0c12aac..26bf42aa6 100644 --- a/docs/how-to.md +++ b/docs/how-to.md @@ -1009,29 +1009,47 @@ own id, writes domain values through an `ExtensionEncoder` and reads them back t `validate_dtype`), run on every dtype with that id when the writer is created and when a file is opened. +An extension's metadata is a record implementing `ExtensionMetadata`, so a dtype compares by +value. The encoder's `serializeMetadata` writes it as the bytes a file stores and the decoder's +`parseMetadata` reads it back when the file opens (Rust's `serialize_metadata` and +`deserialize_metadata`); nothing else sees the bytes. Without a decoder (with `allowUnknown()`) the +metadata reads back as `OpaqueMetadata`, the raw bytes. + The example is a money column with one currency per column: amounts are `I64` minor units -(cents for EUR), and the ISO 4217 code is the extension metadata, as a timestamp keeps its zone. +(cents for EUR), and the currency is the extension metadata, as a timestamp keeps its zone. Never store money as a float. -**Write side:** - ```java import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; +import io.github.dfa1.vortex.reader.Chunk; +import io.github.dfa1.vortex.reader.array.LongArray; +import io.github.dfa1.vortex.reader.extension.ExtensionDecoder; import io.github.dfa1.vortex.writer.ExtensionEncoder; import java.lang.foreign.ValueLayout; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Collection; import java.util.Currency; +// the metadata: the column's currency, stored as its ISO 4217 code +record MoneyMetadata(Currency currency) implements ExtensionMetadata { +} + +// write side final class MoneyEncoder implements ExtensionEncoder { static final ExtensionId ID = ExtensionId.parse("acme.money"); static DType.Extension dtype(Currency currency) { - byte[] code = currency.getCurrencyCode().getBytes(StandardCharsets.US_ASCII); - return new DType.Extension(ID.id(), DTypes.I64, MemorySegment.ofArray(code), false); + return new DType.Extension(ID.id(), DTypes.I64, new MoneyMetadata(currency), false); + } + + @Override + public byte[] serializeMetadata(ExtensionMetadata metadata) { + return ((MoneyMetadata) metadata).currency().getCurrencyCode().getBytes(StandardCharsets.US_ASCII); } @Override @@ -1049,12 +1067,11 @@ final class MoneyEncoder implements ExtensionEncoder { if (!dtype.storageDType().equals(DTypes.I64)) { throw new VortexException("acme.money storage must be non-nullable I64"); } - currency(dtype); // rejects a code that is not ISO 4217 } @Override public Object encodeAll(DType.Extension dtype, Collection values) { - int digits = currency(dtype).getDefaultFractionDigits(); + int digits = ((MoneyMetadata) dtype.metadata()).currency().getDefaultFractionDigits(); long[] minorUnits = new long[values.size()]; int i = 0; for (Object value : values) { @@ -1063,11 +1080,6 @@ final class MoneyEncoder implements ExtensionEncoder { } return minorUnits; } - - static Currency currency(DType.Extension dtype) { - return Currency.getInstance(new String(dtype.metadata().toArray(ValueLayout.JAVA_BYTE), - StandardCharsets.US_ASCII)); - } } DType.Struct schema = new DType.Struct(List.of(ColumnName.of("price")), @@ -1079,17 +1091,8 @@ try (var ch = FileChannel.open(Path.of("prices.vortex"), StandardOpenOption.CREA writer.writeChunk(chunk -> chunk.put(ColumnName.of("price"), List.of(new BigDecimal("9.99"), new BigDecimal("120.50")))); } -``` - -**Read side:** - -```java -import io.github.dfa1.vortex.reader.Chunk; -import io.github.dfa1.vortex.reader.array.LongArray; -import io.github.dfa1.vortex.reader.extension.ExtensionDecoder; - -import java.util.ArrayList; +// read side final class MoneyDecoder implements ExtensionDecoder { static final ExtensionId ID = ExtensionId.parse("acme.money"); @@ -1100,8 +1103,14 @@ final class MoneyDecoder implements ExtensionDecoder { @Override public DType.Extension dtype(boolean nullable) { - byte[] code = "EUR".getBytes(StandardCharsets.US_ASCII); - return new DType.Extension(ID.id(), DTypes.I64, MemorySegment.ofArray(code), false); + return MoneyEncoder.dtype(Currency.getInstance("EUR")); + } + + @Override + public ExtensionMetadata parseMetadata(MemorySegment metadata) { + // an unknown code throws, refusing the file as Rust's deserialize_metadata would + return new MoneyMetadata(Currency.getInstance( + new String(metadata.toArray(ValueLayout.JAVA_BYTE), StandardCharsets.US_ASCII))); } @Override @@ -1111,8 +1120,7 @@ final class MoneyDecoder implements ExtensionDecoder { @Override public List decodeAll(DType.Extension dtype, Array storage) { - Currency currency = Currency.getInstance(new String(dtype.metadata().toArray(ValueLayout.JAVA_BYTE), - StandardCharsets.US_ASCII)); + Currency currency = ((MoneyMetadata) dtype.metadata()).currency(); LongArray minorUnits = (LongArray) storage; List out = new ArrayList<>(); for (long i = 0; i < minorUnits.length(); i++) { diff --git a/docs/reference.md b/docs/reference.md index 8956c214f..d345e2e2e 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -52,7 +52,7 @@ new code — the record constructors stay available for pattern matching and tes | `DType.List` | — | `new DType.List(elementType, nullable)` | | `DType.FixedSizeList` | — | `new DType.FixedSizeList(elementType, fixedSize, nullable)` | | `DType.Map` | — | `new DType.Map(keyType, valueType, keysSorted, nullable)` | -| `DType.Extension` | — | `new DType.Extension(id, storageDType, metadata, nullable)` | +| `DType.Extension` | — | `new DType.Extension(id, storageDType, metadata, nullable)`; `metadata` is an `ExtensionMetadata` record (`TimeUnitMetadata`, `TimestampMetadata`, a third-party record, or `OpaqueMetadata.EMPTY` for none) | | `DType.Union` | — (read only: no union is written) | `new DType.Union(names, variantTypes, typeIds, nullable)`; `variantIndex(typeId)` | Helpers: `nullable()` (boolean accessor on every record), `asNullable()` (fluent @@ -309,9 +309,13 @@ Immutable after construction. Build via `ReadRegistry.builder()` or the static c Register custom encoding and extension decoders programmatically via `register(...)` — there is no `ServiceLoader` discovery. `Chunk.as(name, Class)` decodes an extension column through the -`ExtensionDecoder` registered for its id. Opening a file runs each registered decoder's -`validate(DType.Extension)` on its dtypes, and refuses an extension id with no decoder unless -`allowUnknown()` is set — Rust's behavior for an id its session has no vtable for. +`ExtensionDecoder` registered for its id. Opening a file parses each extension's metadata +(`ExtensionDecoder#parseMetadata(MemorySegment)` for a registered id the reader does not parse +itself), runs each registered decoder's `validate(DType.Extension)` on its dtypes, and refuses an +extension id with no decoder unless `allowUnknown()` is set — Rust's behavior for an id its session +has no vtable for. The writer serializes metadata it does not know with the id's +`ExtensionEncoder#serializeMetadata(ExtensionMetadata)`, and refuses at `create` a record no +encoder serializes. ### `WriteRegistry` (`io.github.dfa1.vortex.writer.WriteRegistry`) diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java index 2f61fc620..f3f169b8d 100644 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java @@ -8,6 +8,8 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.DTypes; import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.core.model.PType; import io.github.dfa1.vortex.reader.Chunk; import io.github.dfa1.vortex.reader.ReadRegistry; @@ -107,13 +109,16 @@ void schema_keepsIdStorageAndMetadata(@TempDir Path tmp) throws IOException { DType.Extension written = vectorDtype(DIM, true); writeVectors(file, written); + DType result; try (VortexReader reader = VortexReader.open(file, READ_REGISTRY)) { - // When — the metadata is a view of the mapped file, so compare before closing - DType result = ((DType.Struct) reader.dtype()).fieldTypes().get(1); - - // Then — Extension.equals compares the metadata bytes, so the dimension survived too - assertThat(result).isEqualTo(written); + // When + result = ((DType.Struct) reader.dtype()).fieldTypes().get(1); } + + // Then — the encoder serialized the typed metadata and the decoder parsed it back, so + // the dimension survived as a VectorMetadata, still readable after the reader closed + assertThat(result).isEqualTo(written); + assertThat(((DType.Extension) result).metadata()).isEqualTo(new VectorMetadata(DIM)); } @Test @@ -152,10 +157,10 @@ void limitedScan_returnsTheFirstRows(@TempDir Path tmp) throws IOException { @Test void storageArray_withoutEncoder_isWrittenAsIs(@TempDir Path tmp) throws IOException { - // Given — no encoder registered: the caller passes the storage shape itself, as for any - // extension id the writer does not know + // Given — no encoder registered: the caller passes the storage shape itself and the + // metadata as the bytes a file stores, as for any extension id the writer does not know Path file = tmp.resolve("raw.vortex"); - DType.Struct schema = schema(vectorDtype(2, false)); + DType.Struct schema = schema(opaqueVectorDtype(2, 2)); float[] flat = {1f, 2f, 3f, 4f}; try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE); var sut = VortexWriter.create(ch, schema, WriteOptions.defaults())) { @@ -257,11 +262,25 @@ void vortexJni_withoutVtable_refusesTheFile(@TempDir Path tmp) throws IOExceptio @Nested class Validation { + @Test + void create_withoutEncoder_rejectsTypedMetadata(@TempDir Path tmp) throws IOException { + // Given — no encoder to serialize a VectorMetadata: refused when the writer is created, + // not when it closes after the data is written + Path file = tmp.resolve("unserializable.vortex"); + + try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) { + // When / Then + assertThatThrownBy(() -> VortexWriter.create(ch, schema(vectorDtype(DIM, false)), WriteOptions.defaults())) + .isInstanceOf(VortexException.class) + .hasMessageContaining("no extension encoder registered for example.vector"); + } + } + @Test void create_rejectsDtypeTheEncoderRefuses(@TempDir Path tmp) throws IOException { // Given — metadata says 4 dimensions, storage holds 3 DType.Extension invalid = new DType.Extension(VECTOR.id(), - new DType.FixedSizeList(DTypes.F32, DIM, false), dimensionMetadata(4), false); + new DType.FixedSizeList(DTypes.F32, DIM, false), new VectorMetadata(4), false); Path file = tmp.resolve("invalid.vortex"); try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) { @@ -275,8 +294,7 @@ void create_rejectsDtypeTheEncoderRefuses(@TempDir Path tmp) throws IOException @Test void open_rejectsDtypeTheDecoderRefuses(@TempDir Path tmp) throws IOException { // Given — a file from a writer that had no encoder, so nothing validated the dtype - DType.Extension invalid = new DType.Extension(VECTOR.id(), - new DType.FixedSizeList(DTypes.F32, 2, false), dimensionMetadata(5), false); + DType.Extension invalid = opaqueVectorDtype(2, 5); Path file = tmp.resolve("invalid.vortex"); try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE); var writer = VortexWriter.create(ch, schema(invalid), WriteOptions.defaults())) { @@ -306,13 +324,26 @@ void writeChunk_nullInNonNullableColumn_throws(@TempDir Path tmp) throws IOExcep // ── the custom extension type ──────────────────────────────────────────── + /// The `example.vector` metadata: the vector dimension, also the storage's fixed size. Its wire + /// form is a u32 LE, written by [VectorEncoder] and parsed back by [VectorDecoder]. + /// + /// @param dimension the number of elements per vector + record VectorMetadata(int dimension) implements ExtensionMetadata { + } + /// The `example.vector` dtype: `FixedSizeList` storage, dimension also in metadata. static DType.Extension vectorDtype(int dim, boolean nullable) { return new DType.Extension(VECTOR.id(), new DType.FixedSizeList(DTypes.F32, dim, nullable), - dimensionMetadata(dim), nullable); + new VectorMetadata(dim), nullable); + } + + /// The dtype as a writer without the encoder writes it: metadata as the bytes a file stores. + static DType.Extension opaqueVectorDtype(int storageDim, int metadataDim) { + return new DType.Extension(VECTOR.id(), new DType.FixedSizeList(DTypes.F32, storageDim, false), + new OpaqueMetadata(dimensionBytes(metadataDim)), false); } - static MemorySegment dimensionMetadata(int dim) { + static MemorySegment dimensionBytes(int dim) { MemorySegment metadata = MemorySegment.ofArray(new byte[4]); metadata.set(ValueLayout.JAVA_INT_UNALIGNED.withOrder(java.nio.ByteOrder.LITTLE_ENDIAN), 0, dim); return metadata; @@ -325,11 +356,9 @@ static void validateVector(DType.Extension dtype) { throw new VortexException("example.vector storage must be FixedSizeList, got " + dtype.storageDType()); } - MemorySegment metadata = dtype.metadata(); - if (metadata == null || metadata.byteSize() != 4) { - throw new VortexException("example.vector metadata must be a u32 dimension"); + if (!(dtype.metadata() instanceof VectorMetadata(int dim))) { + throw new VortexException("example.vector metadata must be a VectorMetadata, got " + dtype.metadata()); } - int dim = metadata.get(ValueLayout.JAVA_INT_UNALIGNED.withOrder(java.nio.ByteOrder.LITTLE_ENDIAN), 0); if (dim != list.fixedSize()) { throw new VortexException("example.vector metadata dimension " + dim + " != storage size " + list.fixedSize()); @@ -353,6 +382,11 @@ public void validate(DType.Extension dtype) { validateVector(dtype); } + @Override + public byte[] serializeMetadata(ExtensionMetadata metadata) { + return dimensionBytes(((VectorMetadata) metadata).dimension()).toArray(ValueLayout.JAVA_BYTE); + } + @Override public Object encodeAll(DType.Extension dtype, Collection values) { int dim = ((DType.FixedSizeList) dtype.storageDType()).fixedSize(); @@ -407,6 +441,14 @@ public void validate(DType.Extension dtype) { validateVector(dtype); } + @Override + public ExtensionMetadata parseMetadata(MemorySegment metadata) { + if (metadata.byteSize() != 4) { + throw new VortexException("example.vector metadata must be a u32 dimension"); + } + return new VectorMetadata(metadata.get(ValueLayout.JAVA_INT_UNALIGNED.withOrder(java.nio.ByteOrder.LITTLE_ENDIAN), 0)); + } + @Override public List decodeAll(DType.Extension dtype, Array storage) { MaskedArray.Unwrapped unwrapped = MaskedArray.unwrap(storage); diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/DateInteropIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/DateInteropIntegrationTest.java index c92e4efce..8f3340f8c 100644 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/DateInteropIntegrationTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/DateInteropIntegrationTest.java @@ -8,6 +8,7 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.DTypes; import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimeUnitMetadata; import io.github.dfa1.vortex.reader.Chunk; import io.github.dfa1.vortex.reader.ReadRegistry; import io.github.dfa1.vortex.reader.ScanFilter; @@ -32,7 +33,6 @@ import org.junit.jupiter.params.provider.EnumSource; import java.io.IOException; -import java.lang.foreign.ValueLayout; import java.nio.file.Path; import java.time.LocalDate; import java.util.LinkedHashMap; @@ -104,9 +104,9 @@ void rustWrites_dtypeRecordsTheUnit(DateUnit unit, @TempDir Path tmp) throws IOE // When DType.Extension result = (DType.Extension) ((DType.Struct) reader.dtype()).fieldTypes().get(1); - // Then — the unit byte and storage width Rust chose, which the decoder must honor + // Then — the unit and storage width Rust chose, which the decoder must honor TimeUnit expectedUnit = unit == DateUnit.DAY ? TimeUnit.Days : TimeUnit.Milliseconds; - assertThat(TimeUnit.fromTag(result.metadata().get(ValueLayout.JAVA_BYTE, 0))).isEqualTo(expectedUnit); + assertThat(result.metadata()).isEqualTo(new TimeUnitMetadata(expectedUnit)); assertThat(result.storageDType()).isEqualTo(unit == DateUnit.DAY ? DTypes.I32.asNullable() : DTypes.I64.asNullable()); } } @@ -124,9 +124,10 @@ void rustWrites_dtypeOutlivesTheReader(DateUnit unit, @TempDir Path tmp) throws result = (DType.Extension) ((DType.Struct) reader.dtype()).fieldTypes().get(1); } - // Then — the metadata was a slice of the closed file mapping, so this threw "Already closed" + // Then — the metadata was once a slice of the closed file mapping, and this threw + // "Already closed"; now it is parsed when the file opens TimeUnit expectedUnit = unit == DateUnit.DAY ? TimeUnit.Days : TimeUnit.Milliseconds; - assertThat(TimeUnit.fromTag(result.metadata().get(ValueLayout.JAVA_BYTE, 0))).isEqualTo(expectedUnit); + assertThat(result.metadata()).isEqualTo(new TimeUnitMetadata(expectedUnit)); } /// Every ninth row is null; the rest step 37 days from 1915, so a quarter of them precede the diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/JavaWritesRustReadsIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/JavaWritesRustReadsIntegrationTest.java index cea0aebcd..2e9bfb78d 100644 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/JavaWritesRustReadsIntegrationTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/JavaWritesRustReadsIntegrationTest.java @@ -2328,12 +2328,13 @@ private static boolean hasDictLayout(io.github.dfa1.vortex.inspect.InspectorTree @Test void javaWriter_rustReader_extensionWithNullMetadata(@TempDir Path tmp) throws IOException { - // Given — an extension dtype with null metadata, here vortex.json (Utf8 storage, no - // metadata), which the vortex-jni session registers. The writer used to omit the metadata - // field for null, and Rust refuses an absent field ("failed to parse extension metadata") - // for the whole file, even when only a sibling column is projected. + // Given — an extension dtype with no metadata, here vortex.json (Utf8 storage), which the + // vortex-jni session registers. The writer used to omit the metadata field when there was + // none, and Rust refuses an absent field ("failed to parse extension metadata") for the + // whole file, even when only a sibling column is projected. Path file = tmp.resolve("java_json_null_metadata.vtx"); - DType.Extension json = new DType.Extension("vortex.json", DTypes.UTF8, null, false); + DType.Extension json = new DType.Extension("vortex.json", DTypes.UTF8, + io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, false); DType.Struct schema = new DType.Struct(List.of(ColumnName.of("id"), ColumnName.of("j")), List.of(DTypes.I64, json), false); try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE); @@ -2367,7 +2368,8 @@ void invalidBuiltInExtensionDtype_refusedByJavaAndRust(@TempDir Path tmp) throws // Given — vortex.timestamp over I32 storage, which Rust's validate_dtype refuses ("Timestamp // storage dtype must be i64"). Java used to write and read it. DType.Extension invalid = new DType.Extension("vortex.timestamp", DTypes.I32, - MemorySegment.ofArray(new byte[]{2, 0, 0}), false); + new io.github.dfa1.vortex.core.model.TimestampMetadata(io.github.dfa1.vortex.core.model.TimeUnit.Milliseconds, null), + false); DType.Struct schema = new DType.Struct(List.of(ColumnName.of("ts")), List.of(invalid), false); Path file = tmp.resolve("timestamp_i32.vtx"); @@ -2382,7 +2384,9 @@ void invalidBuiltInExtensionDtype_refusedByJavaAndRust(@TempDir Path tmp) throws // No Java write path emits it any more, so write the column under a custom id of the same // length and rename it in place: a FlatBuffer string is stored verbatim, once. DType.Struct disguised = new DType.Struct(List.of(ColumnName.of("ts")), List.of(new DType.Extension( - "vortex.timestamq", DTypes.I32, MemorySegment.ofArray(new byte[]{2, 0, 0}), false)), false); + "vortex.timestamq", DTypes.I32, + new io.github.dfa1.vortex.core.model.OpaqueMetadata(MemorySegment.ofArray(new byte[]{2, 0, 0})), false)), + false); try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING); var writer = VortexWriter.create(ch, disguised, WriteOptions.defaults())) { diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/WellKnownExtensionsInteropIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/WellKnownExtensionsInteropIntegrationTest.java index 4f9e5b8c4..98c00aac1 100644 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/WellKnownExtensionsInteropIntegrationTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/WellKnownExtensionsInteropIntegrationTest.java @@ -10,6 +10,7 @@ import io.github.dfa1.vortex.core.model.ColumnName; import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.DTypes; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.reader.Chunk; import io.github.dfa1.vortex.reader.ReadRegistry; import io.github.dfa1.vortex.reader.VortexReader; @@ -30,7 +31,6 @@ import org.junit.jupiter.api.io.TempDir; import java.io.IOException; -import java.lang.foreign.MemorySegment; import java.nio.channels.FileChannel; import java.nio.charset.StandardCharsets; import java.nio.file.Path; @@ -50,7 +50,7 @@ class WellKnownExtensionsInteropIntegrationTest { private static final Session SESSION = Session.create(); private static final BufferAllocator ALLOCATOR = ArrowAllocation.rootAllocator(); - private static final MemorySegment NO_METADATA = MemorySegment.ofArray(new byte[0]); + private static final OpaqueMetadata NO_METADATA = OpaqueMetadata.EMPTY; private static final DType POINT_STORAGE = new DType.Struct( List.of(ColumnName.of("x"), ColumnName.of("y")), List.of(DTypes.F64, DTypes.F64), false); private static final DType.Struct SCHEMA = new DType.Struct( diff --git a/parquet/src/test/java/io/github/dfa1/vortex/parquet/ColumnBuilderTest.java b/parquet/src/test/java/io/github/dfa1/vortex/parquet/ColumnBuilderTest.java index 634da9284..6a9a3dcc4 100644 --- a/parquet/src/test/java/io/github/dfa1/vortex/parquet/ColumnBuilderTest.java +++ b/parquet/src/test/java/io/github/dfa1/vortex/parquet/ColumnBuilderTest.java @@ -201,7 +201,8 @@ void f16_rejectedAtBuild() { void extensionDtype_rejectedAtConstruction() { // Given — nested Extension (timestamp) columns are unsupported; see class javadoc DType.Extension extensionDtype = new DType.Extension( - "vortex.date", new DType.Primitive(PType.I32, false), null, false); + "vortex.date", new DType.Primitive(PType.I32, false), + new io.github.dfa1.vortex.core.model.TimeUnitMetadata(io.github.dfa1.vortex.core.model.TimeUnit.Days), false); // When / Then assertThatThrownBy(() -> ColumnBuilder.forType(extensionDtype)) diff --git a/parquet/src/test/java/io/github/dfa1/vortex/parquet/ParquetExporterTest.java b/parquet/src/test/java/io/github/dfa1/vortex/parquet/ParquetExporterTest.java index daa0faf18..4878f5a3c 100644 --- a/parquet/src/test/java/io/github/dfa1/vortex/parquet/ParquetExporterTest.java +++ b/parquet/src/test/java/io/github/dfa1/vortex/parquet/ParquetExporterTest.java @@ -208,7 +208,8 @@ void timestamp_secondsOrDaysResolution_throws(TimeUnit unit) { @Test void nonTimestampExtension_throws() { // Given — a made-up extension id this exporter has no mapping for - DType.Extension ext = new DType.Extension("vortex.uuid", new DType.Primitive(PType.I64, false), null, false); + DType.Extension ext = new DType.Extension("vortex.uuid", new DType.Primitive(PType.I64, false), + io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, false); // When / Then assertThatThrownBy(() -> schemaOf("u", ext)).isInstanceOf(UnsupportedOperationException.class); diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java b/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java index 84f10bba1..d52128048 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java @@ -20,6 +20,7 @@ import io.github.dfa1.vortex.core.fbs.FbsType; import io.github.dfa1.vortex.core.fbs.FbsUtf8; import io.github.dfa1.vortex.core.fbs.FbsVariant; +import io.github.dfa1.vortex.reader.extension.ExtensionMetadataParser; import io.github.dfa1.vortex.reader.layout.Layout; import java.lang.foreign.MemorySegment; @@ -339,10 +340,11 @@ private static DType convertDType(io.github.dfa1.vortex.core.fbs.FbsDType fbs) { if (metadata == null) { throw new VortexException("extension dtype missing metadata"); } + ExtensionId id = parseExtensionId(e.id()); yield new DType.Extension( - parseExtensionId(e.id()).id(), + id.id(), storage, - metadata, + ExtensionMetadataParser.parse(id, metadata), storage.nullable()); } case FbsType.FbsVariant -> new DType.Variant(fbs.type(new FbsVariant()).nullable()); diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/ReadRegistry.java b/reader/src/main/java/io/github/dfa1/vortex/reader/ReadRegistry.java index fc73a86c9..4afe1220c 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/ReadRegistry.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/ReadRegistry.java @@ -8,6 +8,7 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.EncodingId; import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.reader.decode.AlpEncodingDecoder; import io.github.dfa1.vortex.reader.decode.AlpRdEncodingDecoder; import io.github.dfa1.vortex.reader.decode.ArrayNode; @@ -142,47 +143,67 @@ public ExtensionDecoder lookup(ExtensionId extensionId) { return extensions.get(extensionId); } - /// Checks every extension dtype in `dtype`, nested ones included, as Rust does when it - /// deserializes one: a registered decoder validates it; a [ExtensionId.WellKnown] id without a - /// decoder is validated by [ExtensionId.WellKnown#validate(DType.Extension)] and reads as its - /// storage; any other id is refused unless [Builder#allowUnknown()] is set, when it reads as its - /// storage (Rust's `ForeignExtDType`). + /// Resolves every extension dtype in `dtype`, nested ones included, as Rust does when it + /// deserializes one: a registered decoder parses metadata the reader kept as [OpaqueMetadata] + /// ([ExtensionDecoder#parseMetadata(MemorySegment)]) and validates the dtype; a + /// [ExtensionId.WellKnown] id without a decoder is validated by + /// [ExtensionId.WellKnown#validate(DType.Extension)] and reads as its storage; any other id is + /// refused unless [Builder#allowUnknown()] is set, when it reads as its storage with its bytes + /// (Rust's `ForeignExtDType`). /// Run on a file's schema at open, and on the dtypes embedded in Variant metadata at decode. /// /// @param dtype a file's schema, or `null` when the file carries no dtype blob - /// @throws VortexException if a registered decoder rejects its dtype, or an extension id has no - /// decoder and unknown ids are not allowed - public void validateExtensions(DType dtype) { - switch (dtype) { - case null -> { - // a file without a dtype blob parses with no schema: nothing to check - } + /// @return `dtype` with each registered extension's metadata parsed by its decoder + /// @throws VortexException if a registered decoder rejects its metadata or dtype, or an + /// extension id has no decoder and unknown ids are not allowed + public DType resolveExtensions(DType dtype) { + return switch (dtype) { + case null -> null; case DType.Extension ext -> { ExtensionId id = parseId(ext.extensionId()); ExtensionDecoder decoder = extensions.get(id); + DType.Extension resolved = new DType.Extension(ext.extensionId(), + resolveExtensions(ext.storageDType()), ext.metadata(), ext.nullable()); if (decoder != null) { - validate(decoder, ext); + resolved = parseMetadata(decoder, resolved); + validate(decoder, resolved); } else if (id instanceof ExtensionId.WellKnown wellKnown) { // the reference session knows it: no decoder needed, the column reads as storage - wellKnown.validate(ext); + wellKnown.validate(resolved); } else if (!allowUnknown) { throw new VortexException("no extension decoder registered for " + ext.extensionId() + "; register one or enable ReadRegistry.Builder#allowUnknown()"); } - validateExtensions(ext.storageDType()); - } - case DType.Struct struct -> struct.fieldTypes().forEach(this::validateExtensions); - case DType.List list -> validateExtensions(list.elementType()); - case DType.FixedSizeList list -> validateExtensions(list.elementType()); - case DType.Map map -> { - validateExtensions(map.keyType()); - validateExtensions(map.valueType()); + yield resolved; } - case DType.Union union -> union.variantTypes().forEach(this::validateExtensions); + case DType.Struct(var names, var types, var nullable) -> + new DType.Struct(names, types.stream().map(this::resolveExtensions).toList(), nullable); + case DType.List(var element, var nullable) -> new DType.List(resolveExtensions(element), nullable); + case DType.FixedSizeList(var element, var size, var nullable) -> + new DType.FixedSizeList(resolveExtensions(element), size, nullable); + case DType.Map(var key, var value, var keysSorted, var nullable) -> + new DType.Map(resolveExtensions(key), resolveExtensions(value), keysSorted, nullable); + case DType.Union(var names, var types, var typeIds, var nullable) -> + new DType.Union(names, types.stream().map(this::resolveExtensions).toList(), typeIds, nullable); case DType.Null _, DType.Bool _, DType.Primitive _, DType.Decimal _, DType.Utf8 _, - DType.Binary _, DType.Variant _ -> { - // no nested dtype - } + DType.Binary _, DType.Variant _ -> dtype; + }; + } + + /// Parses metadata the reader kept as bytes with the extension's decoder. A third-party parser + /// may trip over malformed bytes with any runtime exception, or return metadata that is not a + /// record; reading a file must only ever fail with [VortexException]. + private static DType.Extension parseMetadata(ExtensionDecoder decoder, DType.Extension ext) { + if (!(ext.metadata() instanceof OpaqueMetadata(MemorySegment bytes))) { + return ext; + } + try { + return new DType.Extension(ext.extensionId(), ext.storageDType(), decoder.parseMetadata(bytes), + ext.nullable()); + } catch (VortexException e) { + throw e; + } catch (RuntimeException e) { + throw new VortexException("extension " + ext.extensionId() + " rejected its metadata", e); } } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/VortexHttpReader.java b/reader/src/main/java/io/github/dfa1/vortex/reader/VortexHttpReader.java index 7aef92985..50ab2f082 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/VortexHttpReader.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/VortexHttpReader.java @@ -158,11 +158,11 @@ public static VortexHttpReader open(URI uri, ReadRegistry registry, LayoutRegist // Range request from them. The local-file path runs this inside PostscriptParser.parse; // the HTTP path calls parseBlobs directly and must validate here too. PostscriptParser.validateSegmentSpecs(parsed.footer().segmentSpecs(), fileSize); - registry.validateExtensions(parsed.dtype()); + DType dtype = registry.resolveExtensions(parsed.dtype()); return new VortexHttpReader( uri, client, fileSize, trailer.version(), - parsed.footer(), parsed.dtype(), parsed.layout(), + parsed.footer(), dtype, parsed.layout(), registry, layoutRegistry ); } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/VortexReader.java b/reader/src/main/java/io/github/dfa1/vortex/reader/VortexReader.java index 7e049fcda..c2bcf64a7 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/VortexReader.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/VortexReader.java @@ -119,11 +119,11 @@ private static VortexReader parse( } catch (RuntimeException e) { throw new VortexException("malformed postscript", e); } - registry.validateExtensions(parsed.dtype()); + DType dtype = registry.resolveExtensions(parsed.dtype()); return new VortexReader( arena, seg, size, trailer.version(), - parsed.footer(), parsed.dtype(), parsed.layout(), + parsed.footer(), dtype, parsed.layout(), registry, layoutRegistry ); } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoder.java index 5a47e52b7..c3275adb3 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoder.java @@ -9,10 +9,10 @@ import io.github.dfa1.vortex.reader.array.Array; import io.github.dfa1.vortex.reader.array.LazyDateTimePartsLongArray; import io.github.dfa1.vortex.reader.array.MaskedArray; +import io.github.dfa1.vortex.reader.extension.ExtensionStorage; import java.io.IOException; import java.lang.foreign.MemorySegment; -import java.lang.foreign.ValueLayout; /// Read-only decoder for `vortex.datetimeparts`. /// @@ -77,12 +77,7 @@ public Array decode(DecodeContext ctx) { /// `1` when the unit is [TimeUnit#Days] (days carry no sub-second /// component; seconds and subseconds children are expected to be zero). private static long readUnitsPerSecond(DType.Extension ext) { - MemorySegment extMeta = ext.metadata(); - if (extMeta == null || extMeta.byteSize() == 0) { - throw new VortexException(EncodingId.VORTEX_DATETIMEPARTS, - "extension " + ext.extensionId() + " missing TimeUnit metadata byte"); - } - TimeUnit unit = TimeUnit.fromTag(extMeta.get(ValueLayout.JAVA_BYTE, 0)); + TimeUnit unit = ExtensionStorage.readUnit(ext); return unit == TimeUnit.Days ? 1L : unit.divisor(); } } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoder.java index 0c1dffe4e..adae98c8f 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoder.java @@ -6,8 +6,10 @@ import io.github.dfa1.vortex.reader.array.Array; import io.github.dfa1.vortex.reader.array.VariantArray; import io.github.dfa1.vortex.core.model.EncodingId; +import io.github.dfa1.vortex.core.model.ExtensionId; import io.github.dfa1.vortex.core.proto.ProtoVariantMetadata; import io.github.dfa1.vortex.reader.ReadRegistry; +import io.github.dfa1.vortex.reader.extension.ExtensionMetadataParser; import java.io.IOException; import java.lang.foreign.MemorySegment; @@ -71,9 +73,7 @@ private static DType parseShreddedDtype(MemorySegment rawMeta, ReadRegistry regi /// @throws VortexException if an extension id is unregistered (without allow-unknown) or its /// decoder rejects the dtype static DType dtypeFromProto(io.github.dfa1.vortex.core.proto.ProtoDType proto, ReadRegistry registry) { - DType dtype = dtypeFromProto(proto); - registry.validateExtensions(dtype); - return dtype; + return registry.resolveExtensions(dtypeFromProto(proto)); } static DType dtypeFromProto(io.github.dfa1.vortex.core.proto.ProtoDType proto) { @@ -126,10 +126,17 @@ static DType dtypeFromProto(io.github.dfa1.vortex.core.proto.ProtoDType proto) { if (proto.extension() != null) { // an extension's nullability is its storage's, as the file-schema parser has it DType storage = dtypeFromProto(proto.extension().storage_dtype()); + ExtensionId id; + try { + id = ExtensionId.parse(proto.extension().id()); + } catch (IllegalArgumentException | NullPointerException e) { + throw new VortexException("invalid extension id in variant metadata: " + e.getMessage()); + } + byte[] metadata = proto.extension().metadata() != null ? proto.extension().metadata() : new byte[0]; return new DType.Extension( - proto.extension().id(), + id.id(), storage, - MemorySegment.ofArray(proto.extension().metadata() != null ? proto.extension().metadata() : new byte[0]).asReadOnly(), + ExtensionMetadataParser.parse(id, MemorySegment.ofArray(metadata)), storage.nullable()); } if (proto.variant() != null) { diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/DateExtensionDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/DateExtensionDecoder.java index f88eba0e4..6816b963c 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/DateExtensionDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/DateExtensionDecoder.java @@ -43,12 +43,10 @@ public Class domainType() { @Override public DType.Extension dtype(boolean nullable) { - // Rust vortex.date metadata: 1 byte = TimeUnit tag (Days = 4), required by Rust reader. - java.lang.foreign.MemorySegment meta = java.lang.foreign.MemorySegment.ofArray(new byte[]{(byte) TimeUnit.Days.ordinal()}); return new DType.Extension( ExtensionId.VORTEX_DATE.id(), new DType.Primitive(PType.I32, nullable), - meta, + new io.github.dfa1.vortex.core.model.TimeUnitMetadata(TimeUnit.Days), nullable); } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionDecoder.java index abec400c9..8a3926501 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionDecoder.java @@ -2,8 +2,11 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.reader.array.Array; +import java.lang.foreign.MemorySegment; import java.util.List; /// Read-side contract for a Vortex extension type, the Java side of Rust's `ExtVTable`. @@ -43,6 +46,20 @@ public interface ExtensionDecoder { /// @return decoded values in row order, `null` at null rows List decodeAll(DType.Extension dtype, Array storage); + /// Parses this extension's metadata from the bytes a file stores, Rust's + /// `deserialize_metadata`: the inverse of the writer's `ExtensionEncoder.serializeMetadata`. + /// Called when a file is opened, for every extension dtype with this id in its schema whose + /// metadata the reader does not parse itself (it does for the well-known temporal ids). + /// + /// The default keeps the bytes as [OpaqueMetadata]. + /// + /// @param metadata the wire bytes, from an untrusted file + /// @return the metadata, a record + /// @throws io.github.dfa1.vortex.core.error.VortexException if the bytes are malformed + default ExtensionMetadata parseMetadata(MemorySegment metadata) { + return new OpaqueMetadata(metadata); + } + /// Rejects a dtype this extension cannot represent — Rust's `validate_dtype`. Called for every /// extension dtype with this id in a file's schema when the file is opened. /// diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java new file mode 100644 index 000000000..167eef2ef --- /dev/null +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java @@ -0,0 +1,82 @@ +package io.github.dfa1.vortex.reader.extension; + +import io.github.dfa1.vortex.core.error.VortexException; +import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; +import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimeUnitMetadata; +import io.github.dfa1.vortex.core.model.TimestampMetadata; + +import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.StandardCharsets; +import java.time.DateTimeException; +import java.time.ZoneId; + +import static io.github.dfa1.vortex.core.io.VortexFormat.LE_SHORT; + +/// Parses the metadata bytes a file stores for an extension dtype, as Rust's +/// `deserialize_metadata`: [TimeUnitMetadata] for `vortex.date` and `vortex.time`, +/// [TimestampMetadata] for `vortex.timestamp`, and the bytes as [OpaqueMetadata] for every other id. +/// The writer's `DTypeFbsSerializer` is the inverse. +public final class ExtensionMetadataParser { + + private ExtensionMetadataParser() { + } + + /// Parses the metadata of extension `id`. + /// + /// @param id the extension the bytes belong to + /// @param metadata the wire bytes, from an untrusted file + /// @return the parsed metadata + /// @throws VortexException if the bytes are malformed for `id`, or longer than + /// [OpaqueMetadata#MAX_SIZE] + public static ExtensionMetadata parse(ExtensionId id, MemorySegment metadata) { + if (id == ExtensionId.VORTEX_DATE || id == ExtensionId.VORTEX_TIME) { + return new TimeUnitMetadata(unit(id, metadata)); + } + if (id == ExtensionId.VORTEX_TIMESTAMP) { + return timestamp(id, metadata); + } + return new OpaqueMetadata(metadata); + } + + /// `byte[0] = TimeUnit tag, bytes[1..3] = tz_len (u16 LE), bytes[3..3+tz_len] = tz UTF-8`. + private static TimestampMetadata timestamp(ExtensionId id, MemorySegment metadata) { + if (metadata.byteSize() < 3) { + throw new VortexException(id + " metadata must have at least 3 bytes, got " + metadata.byteSize()); + } + TimeUnit unit = unit(id, metadata); + int tzLen = Short.toUnsignedInt(metadata.get(LE_SHORT, 1)); + if (tzLen == 0) { + return new TimestampMetadata(unit, null); + } + if (metadata.byteSize() < 3 + tzLen) { + throw new VortexException(id + " metadata is truncated: declared timezone length " + + tzLen + " but only " + (metadata.byteSize() - 3) + " bytes available"); + } + try { + String timezone = StandardCharsets.UTF_8.newDecoder() + .decode(metadata.asSlice(3, tzLen).asByteBuffer()).toString(); + return new TimestampMetadata(unit, ZoneId.of(timezone)); + } catch (CharacterCodingException e) { + throw new VortexException(id + " timezone is not valid UTF-8", e); + } catch (DateTimeException e) { + throw new VortexException(id + " timezone is not a valid zone id: " + e.getMessage(), e); + } + } + + /// The unit tag in the first byte. + private static TimeUnit unit(ExtensionId id, MemorySegment metadata) { + if (metadata.byteSize() == 0) { + throw new VortexException(id + " metadata must not be empty"); + } + try { + return TimeUnit.fromTag(metadata.get(ValueLayout.JAVA_BYTE, 0)); + } catch (IllegalArgumentException e) { + throw new VortexException(id + " metadata has an invalid time unit", e); + } + } +} diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionStorage.java b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionStorage.java index 3bf63bac8..a64843e65 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionStorage.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionStorage.java @@ -44,18 +44,19 @@ public static long epochInteger(Array storage, long i) { }; } - /// Reads the [TimeUnit] metadata byte at the buffer's current position; - /// throws if the buffer is null or empty. + /// Returns the [TimeUnit] a temporal extension's metadata records. /// /// @param ext declared extension dtype - /// @return decoded time unit - /// @throws VortexException if the metadata is missing + /// @return the time unit + /// @throws VortexException if the metadata is neither a + /// [io.github.dfa1.vortex.core.model.TimeUnitMetadata] nor a + /// [io.github.dfa1.vortex.core.model.TimestampMetadata] public static TimeUnit readUnit(DType.Extension ext) { - java.lang.foreign.MemorySegment meta = ext.metadata(); - if (meta == null || meta.byteSize() == 0) { - throw new VortexException("missing TimeUnit metadata byte for " + ext.extensionId()); - } - return TimeUnit.fromTag(meta.get(java.lang.foreign.ValueLayout.JAVA_BYTE, 0)); + return switch (ext.metadata()) { + case io.github.dfa1.vortex.core.model.TimeUnitMetadata(TimeUnit unit) -> unit; + case io.github.dfa1.vortex.core.model.TimestampMetadata(TimeUnit unit, var _) -> unit; + default -> throw new VortexException("missing TimeUnit metadata for " + ext.extensionId()); + }; } /// Constructs an [Instant] from a signed epoch count in the given unit. diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/UuidExtensionDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/UuidExtensionDecoder.java index a9b5a6549..8c7bc0ad3 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/UuidExtensionDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/UuidExtensionDecoder.java @@ -47,12 +47,11 @@ public List decodeAll(DType.Extension dtype, Array storage) { @Override public DType.Extension dtype(boolean nullable) { DType.Primitive u8 = DTypes.U8; - // Rust vortex.uuid metadata: 0 bytes means "no specific version" — but the field - // must be present in the flatbuffer or the Rust reader's ok_or_else() check fails. + // Rust vortex.uuid metadata: 0 bytes means "no specific version" return new DType.Extension( ExtensionId.VORTEX_UUID.id(), new DType.FixedSizeList(u8, 16, nullable), - java.lang.foreign.MemorySegment.ofArray(new byte[0]), + io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, nullable); } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/ReadRegistryTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/ReadRegistryTest.java index c3ca2f72a..1b7a3e305 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/ReadRegistryTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/ReadRegistryTest.java @@ -8,6 +8,8 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.EncodingId; import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.reader.extension.DateExtensionDecoder; import io.github.dfa1.vortex.reader.extension.ExtensionDecoder; import io.github.dfa1.vortex.reader.decode.ArrayNode; @@ -228,94 +230,148 @@ void registerExtension_duplicateId_throws() { } @Test - void validateExtensions_unregisteredNestedId_throwsUnlessAllowUnknown() { + void resolveExtensions_unregisteredNestedId_throwsUnlessAllowUnknown() { // Given — the unknown id sits inside a list inside a struct: Rust checks every extension // dtype it deserializes, not only top-level columns DType schema = new DType.Struct(List.of(ColumnName.of("c")), - List.of(new DType.List(new DType.Extension("acme.geopoint", DTypes.I32, null, false), false)), false); + List.of(new DType.List(geopoint(OpaqueMetadata.EMPTY), false)), false); ReadRegistry strict = ReadRegistry.loadAll(); ReadRegistry lenient = ReadRegistry.builder().registerDefaults().allowUnknown().build(); - // When / Then - assertThatThrownBy(() -> strict.validateExtensions(schema)) + // When + DType result = lenient.resolveExtensions(schema); + + // Then — allowed, an unknown id keeps its bytes (Rust's ForeignExtDType) + assertThat(result).isEqualTo(schema); + assertThatThrownBy(() -> strict.resolveExtensions(schema)) .isInstanceOf(VortexException.class) .hasMessageContaining("no extension decoder registered for acme.geopoint"); - lenient.validateExtensions(schema); } @Test - void validateExtensions_registeredDecoder_runsItsValidate() { + void resolveExtensions_registeredDecoder_parsesItsMetadata() { + // Given — a decoder that parses its bytes into a record, nested in a list in a struct so + // the resolved dtype has to be rebuilt around it + ExtensionDecoder decoder = extension("acme.geopoint"); + // will…given form: given(decoder.parseMetadata(any())) would run the previous stub with null + org.mockito.BDDMockito.willAnswer(inv -> new Srid(((MemorySegment) inv.getArgument(0)).get(java.lang.foreign.ValueLayout.JAVA_BYTE, 0))) + .given(decoder).parseMetadata(org.mockito.ArgumentMatchers.any()); + ReadRegistry sut = ReadRegistry.builder().register(decoder).build(); + DType schema = new DType.Struct(List.of(ColumnName.of("c")), List.of(new DType.List( + geopoint(new OpaqueMetadata(MemorySegment.ofArray(new byte[]{42}))), false)), false); + + // When + DType result = sut.resolveExtensions(schema); + + // Then — the column now carries the decoder's typed metadata + assertThat(result).isEqualTo(new DType.Struct(List.of(ColumnName.of("c")), + List.of(new DType.List(geopoint(new Srid(42)), false)), false)); + } + + @Test + void resolveExtensions_parserReturnsNonRecord_wrapsInVortexException() { + // Given — a third-party parser returning metadata without value equality + ExtensionDecoder decoder = extension("acme.geopoint"); + org.mockito.BDDMockito.willReturn(new ExtensionMetadata() { + }).given(decoder).parseMetadata(org.mockito.ArgumentMatchers.any()); + ReadRegistry sut = ReadRegistry.builder().register(decoder).build(); + + // When / Then — reading a file only ever fails with VortexException + assertThatThrownBy(() -> sut.resolveExtensions(geopoint(OpaqueMetadata.EMPTY))) + .isInstanceOf(VortexException.class) + .hasMessageContaining("acme.geopoint rejected its metadata") + .hasCauseInstanceOf(IllegalArgumentException.class); + } + + @Test + void resolveExtensions_registeredDecoder_runsItsValidate() { // Given — a decoder whose validate refuses every dtype ExtensionDecoder refusing = extension("acme.geopoint"); org.mockito.BDDMockito.willThrow(new VortexException("bad geopoint")).given(refusing).validate(org.mockito.ArgumentMatchers.any()); ReadRegistry sut = ReadRegistry.builder().register(refusing).build(); - DType schema = new DType.Struct(List.of(ColumnName.of("c")), - List.of(new DType.Extension("acme.geopoint", DTypes.I32, null, false)), false); + DType schema = new DType.Struct(List.of(ColumnName.of("c")), List.of(geopoint(OpaqueMetadata.EMPTY)), false); // When / Then - assertThatThrownBy(() -> sut.validateExtensions(schema)) + assertThatThrownBy(() -> sut.resolveExtensions(schema)) .isInstanceOf(VortexException.class) .hasMessage("bad geopoint"); } @Test - void validateExtensions_noDtype_passes() { + void resolveExtensions_noDtype_passes() { // Given — a file without a dtype blob parses with a null schema (PostscriptParserBlobBoundsTest); // the walk's switch used to throw NullPointerException on it ReadRegistry sut = ReadRegistry.loadAll(); - // When / Then - sut.validateExtensions(null); + // When + DType result = sut.resolveExtensions(null); + + // Then + assertThat(result).isNull(); } @Test - void validateExtensions_validateThrowsRuntimeException_wrapsInVortexException() { + void resolveExtensions_validateThrowsRuntimeException_wrapsInVortexException() { // Given — a third-party validate that trips over malformed metadata from the file ExtensionDecoder fragile = extension("acme.geopoint"); IndexOutOfBoundsException cause = new IndexOutOfBoundsException("metadata too short"); org.mockito.BDDMockito.willThrow(cause).given(fragile).validate(org.mockito.ArgumentMatchers.any()); ReadRegistry sut = ReadRegistry.builder().register(fragile).build(); - DType schema = new DType.Extension("acme.geopoint", DTypes.I32, null, false); // When / Then — reading a file only ever fails with VortexException - assertThatThrownBy(() -> sut.validateExtensions(schema)) + assertThatThrownBy(() -> sut.resolveExtensions(geopoint(OpaqueMetadata.EMPTY))) .isInstanceOf(VortexException.class) .hasMessageContaining("acme.geopoint") .hasCause(cause); } @Test - void validateExtensions_wellKnownIdWithoutDecoder_passesStrictMode() { + void resolveExtensions_wellKnownIdWithoutDecoder_passesStrictMode() { // Given — vortex.json is in the vortex-jni default session, so a strict reader must open it // (reading the column as its Utf8 storage) though Java registers no decoder for it - DType schema = new DType.Extension("vortex.json", DTypes.UTF8, MemorySegment.ofArray(new byte[0]), false); + DType schema = new DType.Extension("vortex.json", DTypes.UTF8, OpaqueMetadata.EMPTY, false); ReadRegistry sut = ReadRegistry.loadAll(); - // When / Then - sut.validateExtensions(schema); + // When + DType result = sut.resolveExtensions(schema); + + // Then + assertThat(result).isEqualTo(schema); } @Test - void validateExtensions_wellKnownIdWithoutDecoder_stillValidated() { + void resolveExtensions_wellKnownIdWithoutDecoder_stillValidated() { // Given — no decoder, but Rust's rules for the id still apply - DType schema = new DType.Extension("vortex.json", DTypes.I32, MemorySegment.ofArray(new byte[0]), false); + DType schema = new DType.Extension("vortex.json", DTypes.I32, OpaqueMetadata.EMPTY, false); ReadRegistry sut = ReadRegistry.loadAll(); // When / Then - assertThatThrownBy(() -> sut.validateExtensions(schema)) + assertThatThrownBy(() -> sut.resolveExtensions(schema)) .isInstanceOf(VortexException.class) .hasMessageContaining("storage dtype must be Utf8"); } + /// A third-party extension's typed metadata. + private record Srid(int srid) implements ExtensionMetadata { + } + + private static DType.Extension geopoint(ExtensionMetadata metadata) { + return new DType.Extension("acme.geopoint", DTypes.I32, metadata, false); + } + private static EncodingDecoder decoder(EncodingId id) { EncodingDecoder decoder = mock(EncodingDecoder.class); given(decoder.encodingId()).willReturn(id); return decoder; } + /// A mocked decoder; its `parseMetadata` keeps the bytes, as the interface's default does + /// (a mock would return `null`). private static ExtensionDecoder extension(String id) { ExtensionDecoder decoder = mock(ExtensionDecoder.class); given(decoder.extensionId()).willReturn(ExtensionId.parse(id)); + given(decoder.parseMetadata(org.mockito.ArgumentMatchers.any())) + .willAnswer(inv -> new OpaqueMetadata(inv.getArgument(0))); return decoder; } } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/compute/ValuesTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/compute/ValuesTest.java index 1f0a07375..b18fe9805 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/compute/ValuesTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/compute/ValuesTest.java @@ -67,7 +67,7 @@ void signedNarrowColumnsSignExtend() { void extensionDtypePassesIntegerThrough() { // Given an integer column whose dtype is an Extension (not a Primitive) — the unsigned // widening must pass the raw value through rather than assume a primitive ptype - DType extension = new DType.Extension("test.ext", DTypes.I32, null, false); + DType extension = new DType.Extension("test.ext", DTypes.I32, io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, false); MemorySegment seg = ARENA.allocate(4, 4); seg.setAtIndex(VortexFormat.LE_INT, 0, -7); Array column = new MaterializedIntArray(extension, 1, seg); diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/ConstantEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/ConstantEncodingDecoderTest.java index 8b9d51c88..2d0a07d05 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/ConstantEncodingDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/ConstantEncodingDecoderTest.java @@ -5,7 +5,11 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.DTypes; import io.github.dfa1.vortex.core.model.EncodingId; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.core.model.PType; +import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimeUnitMetadata; +import io.github.dfa1.vortex.core.model.TimestampMetadata; import io.github.dfa1.vortex.core.proto.ProtoNullValue; import io.github.dfa1.vortex.core.proto.ProtoScalarValue; import io.github.dfa1.vortex.reader.ReadRegistry; @@ -103,13 +107,14 @@ static Stream maskableTypes() { cases.add(Arguments.of(DTypes.UTF8.asNullable(), VarBinArray.class)); cases.add(Arguments.of(DTypes.BINARY.asNullable(), VarBinArray.class)); cases.add(Arguments.of(new DType.Decimal((byte) 10, (byte) 2, true), DecimalArray.class)); - cases.add(Arguments.of(new DType.Extension("vortex.date", DTypes.I32, MemorySegment.ofArray(new byte[]{0}), true), - IntArray.class)); - cases.add(Arguments.of(new DType.Extension("vortex.time", DTypes.I32, MemorySegment.ofArray(new byte[]{1}), true), - IntArray.class)); + cases.add(Arguments.of(new DType.Extension("vortex.date", DTypes.I32, + new TimeUnitMetadata(TimeUnit.Nanoseconds), true), IntArray.class)); + cases.add(Arguments.of(new DType.Extension("vortex.time", DTypes.I32, + new TimeUnitMetadata(TimeUnit.Microseconds), true), IntArray.class)); cases.add(Arguments.of(new DType.Extension("vortex.timestamp", DTypes.I64, - MemorySegment.ofArray(new byte[]{1, 0, 0}), true), LongArray.class)); - cases.add(Arguments.of(new DType.Extension("acme.custom", DTypes.U16, null, true), ShortArray.class)); + new TimestampMetadata(TimeUnit.Microseconds, null), true), LongArray.class)); + cases.add(Arguments.of(new DType.Extension("acme.custom", DTypes.U16, OpaqueMetadata.EMPTY, true), + ShortArray.class)); return cases.stream(); } @@ -150,7 +155,7 @@ static Stream unmaskableTypes() { new DType.Struct(List.of(ColumnName.of("a")), List.of(DTypes.I64), true), new DType.Map(DTypes.I64, DTypes.I64, false, true), new DType.Variant(true), - new DType.Extension("vortex.uuid", new DType.FixedSizeList(DTypes.U8, 16, false), null, true)); + new DType.Extension("vortex.uuid", new DType.FixedSizeList(DTypes.U8, 16, false), OpaqueMetadata.EMPTY, true)); } @Test diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java index dd10f84be..c16cb8128 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/DateTimePartsEncodingDecoderTest.java @@ -35,9 +35,9 @@ private static MemorySegment i64Meta() { } private static DType timestampDType(TimeUnit unit, boolean nullable) { - MemorySegment meta = MemorySegment.ofArray(new byte[]{(byte) unit.ordinal(), 0, 0}); return new DType.Extension("vortex.timestamp", - new DType.Primitive(PType.I64, nullable), meta, nullable); + new DType.Primitive(PType.I64, nullable), + new io.github.dfa1.vortex.core.model.TimestampMetadata(unit, null), nullable); } /// Builds a context with three I64 part-children backed by the given segments. @@ -110,9 +110,9 @@ void decode_nullableExtension_decodesNullableDaysChild() { @Test void decode_extensionMissingTimeUnitMetadata_throws() { - // Given an extension whose metadata byte is absent + // Given an extension whose metadata records no time unit DType noUnit = new DType.Extension("vortex.timestamp", - DTypes.I64, null, false); + DTypes.I64, io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, false); DecodeContext c = ctx(i64Meta(), noUnit, 1, TestSegments.leLongs(0L), TestSegments.leLongs(0L), TestSegments.leLongs(0L)); diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoderTest.java index eb025ff04..abf7de3ff 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoderTest.java @@ -273,12 +273,13 @@ void extension_withMetadata() { DType.Extension ext = (DType.Extension) result; assertThat(ext.extensionId()).isEqualTo("ip.address"); assertThat(ext.storageDType()).isEqualTo(DTypes.I32); - assertThat(ext.metadata().byteSize()).isEqualTo(3); + assertThat(ext.metadata()).isInstanceOf(io.github.dfa1.vortex.core.model.OpaqueMetadata.class); + assertThat(((io.github.dfa1.vortex.core.model.OpaqueMetadata) ext.metadata()).bytes().byteSize()).isEqualTo(3); } @Test void extension_nullMetadata_becomesEmptyBuffer() { - // Given null metadata — must not NPE, maps to an empty read-only buffer + // Given null metadata — must not NPE, maps to empty opaque metadata var proto = io.github.dfa1.vortex.core.proto.ProtoDType.ofExtension(new io.github.dfa1.vortex.core.proto.ProtoExtension( "uuid", prim(io.github.dfa1.vortex.core.proto.ProtoPType.I64, false), null)); @@ -286,7 +287,7 @@ void extension_nullMetadata_becomesEmptyBuffer() { DType.Extension result = (DType.Extension) VariantEncodingDecoder.dtypeFromProto(proto); // Then - assertThat(result.metadata().byteSize()).isZero(); + assertThat(result.metadata()).isEqualTo(io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY); } @Test diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/DateExtensionDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/DateExtensionDecoderTest.java index 0d44e14bf..2cee964e0 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/DateExtensionDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/DateExtensionDecoderTest.java @@ -50,9 +50,7 @@ void dtype_isI32StorageWithDaysMetadata() { // Then assertThat(dtype.extensionId()).isEqualTo("vortex.date"); assertThat(dtype.storageDType()).isEqualTo(DTypes.I32); - assertThat(dtype.metadata()).isNotNull(); - assertThat(dtype.metadata().byteSize()).isEqualTo(1); - assertThat(dtype.metadata().get(java.lang.foreign.ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) 4); // TimeUnit.Days ordinal + assertThat(dtype.metadata()).isEqualTo(new io.github.dfa1.vortex.core.model.TimeUnitMetadata(TimeUnit.Days)); } @Test @@ -164,6 +162,6 @@ void date_unsupportedUnit_throws() { private static DType.Extension dateDtype(TimeUnit unit, DType storage) { return new DType.Extension("vortex.date", storage, - MemorySegment.ofArray(new byte[]{(byte) unit.ordinal()}), false); + new io.github.dfa1.vortex.core.model.TimeUnitMetadata(unit), false); } } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java new file mode 100644 index 000000000..0fc374530 --- /dev/null +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java @@ -0,0 +1,77 @@ +package io.github.dfa1.vortex.reader.extension; + +import io.github.dfa1.vortex.core.error.VortexException; +import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; +import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimeUnitMetadata; +import io.github.dfa1.vortex.core.model.TimestampMetadata; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.lang.foreign.MemorySegment; +import java.time.ZoneId; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/// Rust's `deserialize_metadata` for the temporal extensions (vortex-array/src/extension/datetime), +/// and every other id's bytes kept as they are. +class ExtensionMetadataParserTest { + + static Stream valid() { + return Stream.of( + Arguments.of(ExtensionId.VORTEX_DATE, bytes(4), new TimeUnitMetadata(TimeUnit.Days)), + Arguments.of(ExtensionId.VORTEX_TIME, bytes(0), new TimeUnitMetadata(TimeUnit.Nanoseconds)), + Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 0, 0), new TimestampMetadata(TimeUnit.Milliseconds, null)), + Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(1, 3, 0, 'U', 'T', 'C'), + new TimestampMetadata(TimeUnit.Microseconds, ZoneId.of("UTC"))), + // trailing bytes past the declared timezone are ignored, as Rust slices to tz_len + Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(1, 3, 0, 'U', 'T', 'C', 0), + new TimestampMetadata(TimeUnit.Microseconds, ZoneId.of("UTC"))), + Arguments.of(ExtensionId.VORTEX_UUID, bytes(4), new OpaqueMetadata(bytes(4))), + Arguments.of(ExtensionId.parse("acme.geopoint"), bytes(1, 2, 3), new OpaqueMetadata(bytes(1, 2, 3)))); + } + + static Stream invalid() { + return Stream.of( + Arguments.of(ExtensionId.VORTEX_DATE, bytes(), "metadata must not be empty"), + Arguments.of(ExtensionId.VORTEX_DATE, bytes(9), "invalid time unit"), + Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2), "at least 3 bytes"), + Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 5, 0, 'U'), "truncated"), + Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 1, 0, 0xc3), "not valid UTF-8"), + Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 3, 0, 'X', '/', 'Y'), "not a valid zone id"), + Arguments.of(ExtensionId.parse("acme.big"), MemorySegment.ofArray(new byte[(int) OpaqueMetadata.MAX_SIZE.bytes() + 1]), + "metadata too large")); + } + + @ParameterizedTest + @MethodSource("valid") + void parse_validBytes(ExtensionId id, MemorySegment metadata, ExtensionMetadata expected) { + // When + ExtensionMetadata result = ExtensionMetadataParser.parse(id, metadata); + + // Then + assertThat(result).isEqualTo(expected); + } + + @ParameterizedTest + @MethodSource("invalid") + void parse_malformedBytes_throwsVortexException(ExtensionId id, MemorySegment metadata, String message) { + // When / Then — the bytes come from an untrusted file + assertThatThrownBy(() -> ExtensionMetadataParser.parse(id, metadata)) + .isInstanceOf(VortexException.class) + .hasMessageContaining(message); + } + + private static MemorySegment bytes(int... values) { + byte[] out = new byte[values.length]; + for (int i = 0; i < values.length; i++) { + out[i] = (byte) values[i]; + } + return MemorySegment.ofArray(out); + } +} diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionStorageTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionStorageTest.java index 7620a7b46..51e1780ca 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionStorageTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionStorageTest.java @@ -9,7 +9,6 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import java.lang.foreign.MemorySegment; import java.time.Instant; import static io.github.dfa1.vortex.reader.array.TestArrays.bools; @@ -69,36 +68,41 @@ void unsupportedStorage_throws() { class ReadUnit { @Test - void readsTagByte() { - // Given — tag 3 = Seconds (ordinal) - DType.Extension ext = ext(java.lang.foreign.MemorySegment.ofArray(new byte[]{3})); + void timeUnitMetadata_readsItsUnit() { + // Given + DType.Extension ext = ext(new io.github.dfa1.vortex.core.model.TimeUnitMetadata(TimeUnit.Seconds)); - // When / Then - assertThat(ExtensionStorage.readUnit(ext)).isEqualTo(TimeUnit.Seconds); + // When + TimeUnit result = ExtensionStorage.readUnit(ext); + + // Then + assertThat(result).isEqualTo(TimeUnit.Seconds); } @Test - void nullMetadata_throws() { - // Given - DType.Extension ext = ext(null); + void timestampMetadata_readsItsUnit() { + // Given — a timestamp carries its unit alongside the timezone + DType.Extension ext = ext(new io.github.dfa1.vortex.core.model.TimestampMetadata(TimeUnit.Microseconds, null)); - // When / Then - assertThatThrownBy(() -> ExtensionStorage.readUnit(ext)) - .isInstanceOf(VortexException.class).hasMessageContaining("missing TimeUnit"); + // When + TimeUnit result = ExtensionStorage.readUnit(ext); + + // Then + assertThat(result).isEqualTo(TimeUnit.Microseconds); } @Test - void emptyMetadata_throws() { - // Given - DType.Extension ext = ext(MemorySegment.ofArray(new byte[0])); + void opaqueMetadata_throws() { + // Given metadata that records no unit + DType.Extension ext = ext(io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY); // When / Then assertThatThrownBy(() -> ExtensionStorage.readUnit(ext)) .isInstanceOf(VortexException.class).hasMessageContaining("missing TimeUnit"); } - private DType.Extension ext(java.lang.foreign.MemorySegment meta) { - return new DType.Extension("vortex.timestamp", DTypes.I64, meta, false); + private DType.Extension ext(io.github.dfa1.vortex.core.model.ExtensionMetadata metadata) { + return new DType.Extension("vortex.timestamp", DTypes.I64, metadata, false); } } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionTestSupport.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionTestSupport.java index efb79759d..b700b390e 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionTestSupport.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionTestSupport.java @@ -2,6 +2,7 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.DTypes; +import io.github.dfa1.vortex.core.model.ExtensionId; import io.github.dfa1.vortex.reader.array.IntArray; import io.github.dfa1.vortex.reader.array.LongArray; import io.github.dfa1.vortex.reader.array.MaterializedIntArray; @@ -24,8 +25,9 @@ final class ExtensionTestSupport { private ExtensionTestSupport() { } + /// An extension dtype whose metadata is parsed from `meta` as a file's would be. static DType.Extension ext(String id, DType storage, MemorySegment meta) { - return new DType.Extension(id, storage, meta, false); + return new DType.Extension(id, storage, ExtensionMetadataParser.parse(ExtensionId.parse(id), meta), false); } static MemorySegment unitByte(byte tag) { diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimeExtensionDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimeExtensionDecoderTest.java index 206470e79..a9a4a8949 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimeExtensionDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimeExtensionDecoderTest.java @@ -46,7 +46,7 @@ void dtype_defaultsToMillisecondsI32() { // Then — storage width is I32 for s/ms, metadata byte 0 = TimeUnit.Milliseconds.ordinal() assertThat(dtype.storageDType()).isEqualTo(new DType.Primitive(PType.I32, true)); - assertThat(dtype.metadata().get(java.lang.foreign.ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) TimeUnit.Milliseconds.ordinal()); + assertThat(dtype.metadata()).isEqualTo(new io.github.dfa1.vortex.core.model.TimeUnitMetadata(TimeUnit.Milliseconds)); } @Test @@ -56,7 +56,7 @@ void dtype_nanosecondsForceI64() { // Then assertThat(dtype.storageDType()).isEqualTo(DTypes.I64); - assertThat(dtype.metadata().get(java.lang.foreign.ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) TimeUnit.Nanoseconds.ordinal()); + assertThat(dtype.metadata()).isEqualTo(new io.github.dfa1.vortex.core.model.TimeUnitMetadata(TimeUnit.Nanoseconds)); } @Test diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoderTest.java index b32f96455..59141af3b 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoderTest.java @@ -44,20 +44,17 @@ void dtype_defaultIsMsUtcless() { // Given — default factory yields ms storage with empty tz_len DType.Extension dtype = sut.dtype(false); - // Then — byte 0 = ms ordinal, bytes 1..3 = 0 (tz_len = 0) + // Then — milliseconds, no timezone assertThat(dtype.storageDType()).isEqualTo(DTypes.I64); - assertThat(dtype.metadata().get(java.lang.foreign.ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) TimeUnit.Milliseconds.ordinal()); - assertThat(dtype.metadata().get(io.github.dfa1.vortex.core.io.VortexFormat.LE_SHORT, 1)).isEqualTo((short) 0); + assertThat(dtype.metadata()).isEqualTo(new io.github.dfa1.vortex.core.model.TimestampMetadata(TimeUnit.Milliseconds, null)); } @Test - void dtype_withTimezoneEncodesIanaName() { - // Given — timezone bytes are appended after the 3-byte header so decode can pull them back + void dtype_withTimezoneRecordsIt() { + // Given DType.Extension dtype = sut.dtype(TimeUnit.Microseconds, ZoneId.of("Europe/Paris"), false); - // Then — header tz_len matches the UTF-8 length; the actual bytes follow - int tzLen = Short.toUnsignedInt(dtype.metadata().get(io.github.dfa1.vortex.core.io.VortexFormat.LE_SHORT, 1)); - assertThat(tzLen).isEqualTo("Europe/Paris".getBytes().length); + // Then — the zone the decoder reads back is the one the dtype was built with assertThat(sut.timezone(dtype)).contains(ZoneId.of("Europe/Paris")); } @@ -117,13 +114,12 @@ void decodeAll_maskedArray_yieldsNullAtInvalidPositions() { } @Test - void timezone_truncatedMetadata_throws() { + void truncatedMetadata_refusedWhenParsed() { // Given — declared tz_len longer than buffer can carry MemorySegment meta = MemorySegment.ofArray(new byte[]{2, 5, 0, (byte) 0x55, (byte) 0x54, (byte) 0x43}); - DType.Extension truncated = ext("vortex.timestamp", I64, meta); - // When / Then - assertThatThrownBy(() -> sut.timezone(truncated)) + // When / Then — refused when the file's dtype is parsed, before any decoder reads the zone + assertThatThrownBy(() -> ext("vortex.timestamp", I64, meta)) .isInstanceOf(VortexException.class) .hasMessageContaining("truncated"); } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/UuidExtensionDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/UuidExtensionDecoderTest.java index dbb28028e..bfb1fb0e5 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/UuidExtensionDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/UuidExtensionDecoderTest.java @@ -42,8 +42,7 @@ void dtype_isFixedSizeListOf16U8() { // Then DType.Primitive u8 = DTypes.U8; assertThat(dtype.storageDType()).isEqualTo(new DType.FixedSizeList(u8, 16, true)); - assertThat(dtype.metadata()).isNotNull(); - assertThat(dtype.metadata().byteSize()).isZero(); + assertThat(dtype.metadata()).isEqualTo(io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY); } @Test diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/DictLayoutDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/DictLayoutDecoderTest.java index 74e0d0ce8..1ec0da0ce 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/DictLayoutDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/DictLayoutDecoderTest.java @@ -4,6 +4,7 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.DTypes; import io.github.dfa1.vortex.core.model.LayoutId; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.core.io.VortexFormat; import io.github.dfa1.vortex.core.model.PType; import io.github.dfa1.vortex.core.testing.TestSegments; @@ -324,7 +325,7 @@ void fixedSizeListPool_throwsVortexException() { // neither VarBin- nor primitive-shaped. The fallback used to blind-cast it to VarBinArray, // which breaks the security contract (a raw ClassCastException on untrusted input). DType.FixedSizeList storage = new DType.FixedSizeList(DTypes.U8, 16, false); - DType uuid = new DType.Extension("vortex.uuid", storage, null, false); + DType uuid = new DType.Extension("vortex.uuid", storage, OpaqueMetadata.EMPTY, false); Array values = new FixedSizeListArray(storage, 2, bytePool(new int[32])); Array codes = byteCodes(0, 1, 0); @@ -337,7 +338,7 @@ void fixedSizeListPool_throwsVortexException() { // ── harness ───────────────────────────────────────────────────────────────── private static Stream varBinPools() { - DType utf8Extension = new DType.Extension("ip.address", DTypes.UTF8, null, false); + DType utf8Extension = new DType.Extension("ip.address", DTypes.UTF8, OpaqueMetadata.EMPTY, false); String[] byCode = {"ccc", "a", "bb", "a"}; String[] broadcast = {"zz", "zz", "zz", "zz"}; return Stream.of( diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchemaTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchemaTest.java index f0dffb828..048457d15 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchemaTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchemaTest.java @@ -207,7 +207,7 @@ void keepsNanCountForFloatColumn() { void resolvesExtensionViaStorageDType() { // Given — ext over i32 storage; sum should widen using storage dtype (i32 → i64) DType columnDtype = new DType.Extension("ip.address", - DTypes.I32, null, false); + DTypes.I32, io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, false); List present = List.of( ZonedStatsSchema.Stat.MIN, ZonedStatsSchema.Stat.SUM); diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/DTypeFbsSerializer.java b/writer/src/main/java/io/github/dfa1/vortex/writer/DTypeFbsSerializer.java index d89cf8ace..60830f96b 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/DTypeFbsSerializer.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/DTypeFbsSerializer.java @@ -16,30 +16,82 @@ import io.github.dfa1.vortex.core.fbs.FbsUtf8; import io.github.dfa1.vortex.core.fbs.FbsUnion; import io.github.dfa1.vortex.core.fbs.FbsVariant; +import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; +import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimeUnitMetadata; +import io.github.dfa1.vortex.core.model.TimestampMetadata; +import java.lang.foreign.MemorySegment; import java.lang.foreign.ValueLayout; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.nio.charset.StandardCharsets; +import java.time.ZoneId; /// Serializes a [DType] tree to its FlatBuffer wire encoding (the file's DType blob, pointed to /// by the postscript). final class DTypeFbsSerializer { - private DTypeFbsSerializer() { + private final WriteRegistry extensions; + + /// @param extensions the extension encoders, which serialize metadata of their own types + DTypeFbsSerializer(WriteRegistry extensions) { + this.extensions = extensions; } /// Serializes `dtype` to a standalone, finished FlatBuffer. /// /// @param dtype the schema (or nested type) to serialize /// @return the finished FlatBuffer bytes, little-endian - static ByteBuffer buildDType(DType dtype) { + ByteBuffer buildDType(DType dtype) { var fbb = new FbsBuilder(128); int off = serializeDType(fbb, dtype); FbsDType.finishFbsDTypeBuffer(fbb, off); return fbb.dataSegment().asByteBuffer().order(ByteOrder.LITTLE_ENDIAN); } + /// Serializes an extension's metadata, as Rust's `serialize_metadata`: the inverse of the + /// reader's `ExtensionMetadataParser`. The built-in records and [OpaqueMetadata] are written + /// here; any other record by the extension's registered encoder. + /// + /// @param ext an extension dtype + /// @return the metadata's wire bytes + /// @throws VortexException if the metadata is a record no registered encoder serializes, or a + /// timezone longer than the wire's u16 length + byte[] metadataBytes(DType.Extension ext) { + return switch (ext.metadata()) { + case TimeUnitMetadata(TimeUnit unit) -> new byte[]{(byte) unit.ordinal()}; + case TimestampMetadata(TimeUnit unit, ZoneId zone) -> timestampBytes(unit, zone); + case OpaqueMetadata(MemorySegment bytes) -> bytes.toArray(ValueLayout.JAVA_BYTE); + case ExtensionMetadata other -> { + ExtensionEncoder encoder = extensions.lookup(ExtensionId.parse(ext.extensionId())); + if (encoder == null) { + throw new VortexException("no extension encoder registered for " + ext.extensionId() + + " to serialize its metadata of type " + other.getClass().getName()); + } + yield encoder.serializeMetadata(other); + } + }; + } + + /// `byte[0] = TimeUnit tag, bytes[1..3] = tz_len (u16 LE), bytes[3..3+tz_len] = tz UTF-8`. + private static byte[] timestampBytes(TimeUnit unit, ZoneId zone) { + byte[] tz = zone == null ? new byte[0] : zone.getId().getBytes(StandardCharsets.UTF_8); + if (tz.length > 0xFFFF) { + throw new VortexException("timestamp timezone longer than 65535 bytes: " + zone); + } + byte[] out = new byte[3 + tz.length]; + out[0] = (byte) unit.ordinal(); + out[1] = (byte) tz.length; + out[2] = (byte) (tz.length >>> 8); + System.arraycopy(tz, 0, out, 3, tz.length); + return out; + } + /// Serializes `dtype` into `fbb`, returning the offset of the resulting `FbsDType` table. /// Recurses into nested types (struct fields, list/map elements, extension storage) bottom-up, /// as FlatBuffers construction requires. @@ -47,7 +99,7 @@ static ByteBuffer buildDType(DType dtype) { /// @param fbb the builder to serialize into /// @param dtype the type to serialize /// @return the offset of the written `FbsDType` table - static int serializeDType(FbsBuilder fbb, DType dtype) { + int serializeDType(FbsBuilder fbb, DType dtype) { return switch (dtype) { case DType.Null _ -> { FbsNull.startFbsNull(fbb); @@ -110,8 +162,7 @@ static int serializeDType(FbsBuilder fbb, DType dtype) { int storageDtypeOff = serializeDType(fbb, e.storageDType()); // Always present, empty when there is none: Rust refuses an extension dtype whose // metadata field is absent ("failed to parse extension metadata"), failing the file. - byte[] metaBytes = e.metadata() == null ? new byte[0] : e.metadata().toArray(ValueLayout.JAVA_BYTE); - int metaOff = FbsExtension.createMetadataVector(fbb, metaBytes); + int metaOff = FbsExtension.createMetadataVector(fbb, metadataBytes(e)); int inner = FbsExtension.createFbsExtension(fbb, idOff, storageDtypeOff, metaOff); yield FbsDType.createFbsDType(fbb, FbsType.FbsExtension, inner); } diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/ExtensionEncoder.java b/writer/src/main/java/io/github/dfa1/vortex/writer/ExtensionEncoder.java index 343d5cd2d..a402a11fd 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/ExtensionEncoder.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/ExtensionEncoder.java @@ -1,7 +1,9 @@ package io.github.dfa1.vortex.writer; +import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; import java.util.Collection; @@ -36,6 +38,23 @@ public interface ExtensionEncoder { /// @return packed storage array Object encodeAll(DType.Extension dtype, Collection values); + /// Serializes this extension's metadata to the bytes a file stores, Rust's `serialize_metadata`: + /// the inverse of the reader's `ExtensionDecoder.parseMetadata`. Called for metadata the writer + /// does not serialize itself, which is anything but + /// [io.github.dfa1.vortex.core.model.TimeUnitMetadata], + /// [io.github.dfa1.vortex.core.model.TimestampMetadata] and + /// [io.github.dfa1.vortex.core.model.OpaqueMetadata]. + /// + /// The default refuses it. + /// + /// @param metadata the metadata of an extension dtype with this id + /// @return the wire bytes + /// @throws VortexException if this encoder cannot serialize `metadata` + default byte[] serializeMetadata(ExtensionMetadata metadata) { + throw new VortexException(extensionId() + " cannot serialize metadata of type " + + metadata.getClass().getName() + "; override ExtensionEncoder#serializeMetadata"); + } + /// Rejects a dtype this extension cannot represent — Rust's `validate_dtype`. Called for every /// extension dtype with this id in the schema when the writer is created. /// diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java b/writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java index 0b48cdcf8..921fd69c4 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java @@ -307,6 +307,8 @@ private static Set editionExcluded( /// Runs [ExtensionEncoder#validate(DType.Extension)] on every extension dtype in `dtype`, nested /// ones included, that has a registered encoder — Rust validates each extension dtype it builds. + /// Also serializes each one's metadata once, so metadata no encoder can serialize fails here + /// rather than when the file is closed, after its data is written. private void validateExtensions(DType dtype) { switch (dtype) { case DType.Extension ext -> { @@ -319,6 +321,7 @@ private void validateExtensions(DType dtype) { // written from its storage array; still held to Rust's rules for the id wellKnown.validate(ext); } + new DTypeFbsSerializer(extensionRegistry).metadataBytes(ext); validateExtensions(ext.storageDType()); } case DType.Struct struct -> struct.fieldTypes().forEach(this::validateExtensions); @@ -760,7 +763,7 @@ public void close() throws IOException { long footerOff = bytesWritten; write(footerBuf); - ByteBuffer dtypeBuf = DTypeFbsSerializer.buildDType(schema); + ByteBuffer dtypeBuf = new DTypeFbsSerializer(extensionRegistry).buildDType(schema); long dtypeOff = bytesWritten; write(dtypeBuf); diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/DateExtensionEncoder.java b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/DateExtensionEncoder.java index aed40bb7a..974f87d7d 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/DateExtensionEncoder.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/DateExtensionEncoder.java @@ -4,10 +4,10 @@ import io.github.dfa1.vortex.core.model.PType; import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimeUnitMetadata; import io.github.dfa1.vortex.core.model.ExtensionId; import io.github.dfa1.vortex.writer.ExtensionEncoder; -import java.lang.foreign.MemorySegment; import java.time.LocalDate; import java.util.Collection; @@ -34,12 +34,10 @@ public void validate(DType.Extension dtype) { @Override public DType.Extension dtype(boolean nullable) { - // Rust vortex.date metadata: 1 byte = TimeUnit tag (Days = 4), required by Rust reader. - MemorySegment meta = MemorySegment.ofArray(new byte[]{(byte) TimeUnit.Days.ordinal()}); return new DType.Extension( ExtensionId.VORTEX_DATE.id(), new DType.Primitive(PType.I32, nullable), - meta, + new TimeUnitMetadata(TimeUnit.Days), nullable); } diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/DateTimePartsEncodingEncoder.java b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/DateTimePartsEncodingEncoder.java index 0b7a1990e..facb69bfc 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/DateTimePartsEncodingEncoder.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/DateTimePartsEncodingEncoder.java @@ -7,6 +7,7 @@ import io.github.dfa1.vortex.core.model.EncodingId; import io.github.dfa1.vortex.core.model.ExtensionId; import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimestampMetadata; import io.github.dfa1.vortex.core.proto.ProtoDateTimePartsMetadata; import java.lang.foreign.MemorySegment; @@ -47,11 +48,6 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) { throw new VortexException(EncodingId.VORTEX_DATETIMEPARTS, "expected timestamp storage, got " + (data == null ? "null" : data.getClass().getSimpleName())); } - MemorySegment extMeta = ext.metadata(); - if (extMeta == null || extMeta.byteSize() < 3) { - throw new VortexException(EncodingId.VORTEX_DATETIMEPARTS, - "extension metadata missing or too short"); - } Parts parts = split(d.timestamps(), unitOf(ext)); long[] days = parts.days(); long[] seconds = parts.seconds(); @@ -145,16 +141,19 @@ private static DateTimePartsData asParts(DType dtype, Object data) { } if (!(data instanceof long[] storage) || !(dtype instanceof DType.Extension ext) || !ExtensionId.VORTEX_TIMESTAMP.id().equals(ext.extensionId()) - || ext.metadata() == null || ext.metadata().byteSize() < 3 - || TimeUnit.fromTag(ext.metadata().toArray(java.lang.foreign.ValueLayout.JAVA_BYTE)[0]) - == TimeUnit.Days) { + || !(ext.metadata() instanceof TimestampMetadata(TimeUnit unit, var _)) + || unit == TimeUnit.Days) { return null; } return new DateTimePartsData(storage, false); } private static TimeUnit unitOf(DType.Extension ext) { - return TimeUnit.fromTag(ext.metadata().toArray(java.lang.foreign.ValueLayout.JAVA_BYTE)[0]); + if (!(ext.metadata() instanceof TimestampMetadata(TimeUnit unit, var _))) { + throw new VortexException(EncodingId.VORTEX_DATETIMEPARTS, + "extension metadata is not timestamp metadata: " + ext.metadata()); + } + return unit; } /// Splits epoch ticks into whole days, seconds within the day, and sub-second ticks, the three diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/UuidExtensionEncoder.java b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/UuidExtensionEncoder.java index 5085de26f..1eb42b18a 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/UuidExtensionEncoder.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/UuidExtensionEncoder.java @@ -4,9 +4,9 @@ import io.github.dfa1.vortex.core.model.DTypes; import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.writer.ExtensionEncoder; -import java.lang.foreign.MemorySegment; import java.util.Collection; import java.util.UUID; @@ -34,12 +34,11 @@ public void validate(DType.Extension dtype) { @Override public DType.Extension dtype(boolean nullable) { DType.Primitive u8 = DTypes.U8; - // Rust vortex.uuid metadata: 0 bytes means "no specific version" — but the field - // must be present in the flatbuffer or the Rust reader's ok_or_else() check fails. + // Rust vortex.uuid metadata: 0 bytes means "no specific version" return new DType.Extension( ExtensionId.VORTEX_UUID.id(), new DType.FixedSizeList(u8, 16, nullable), - MemorySegment.ofArray(new byte[0]), + OpaqueMetadata.EMPTY, nullable); } diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/WriterZoneMapTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/WriterZoneMapTest.java index b2b33afc6..de2a94bae 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/WriterZoneMapTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/WriterZoneMapTest.java @@ -300,7 +300,7 @@ void extensionColumn_emitsStorageMinMaxZoneMap(@TempDir Path tmp) throws IOExcep // two zones of two rows: zone 0 = [10, 11], zone 1 = [20, 21]. ExtEncoding propagates the // storage primitive's min/max, so the zone-map carries MAX+MIN+NULL_COUNT — same as I64. DType ext = new DType.Extension( - "test.ext", DTypes.I64, null, false); + "test.ext", DTypes.I64, io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, false); DType.Struct schema = new DType.Struct(List.of(ColumnName.of("t")), List.of(ext), false); WriteOptions opts = WriteOptions.cascading(0).withZoneMaps(true).withGlobalDict(false).withEdition(Editions.CORE_2025_10_0); Path file = tmp.resolve("ext.vtx"); diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/ZoneMapStatCodecTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/ZoneMapStatCodecTest.java index 956004ba1..c958019db 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/ZoneMapStatCodecTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/ZoneMapStatCodecTest.java @@ -13,7 +13,6 @@ import org.junit.jupiter.params.provider.MethodSource; import java.io.IOException; -import java.lang.foreign.MemorySegment; import static org.assertj.core.api.Assertions.assertThat; @@ -120,7 +119,9 @@ void empty_returnsNull() { class ExtensionType { private static final DType TIMESTAMP_MS = - new DType.Extension("vortex.timestamp", DTypes.I64, MemorySegment.ofArray(new byte[]{0, 0, 0}), false); + new DType.Extension("vortex.timestamp", DTypes.I64, + new io.github.dfa1.vortex.core.model.TimestampMetadata( + io.github.dfa1.vortex.core.model.TimeUnit.Nanoseconds, null), false); @Test void plainPrimitiveStorage_unwrapsToStoragePrimitive() throws IOException { diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/DateTimePartsEncodingEncoderTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/DateTimePartsEncodingEncoderTest.java index ed9d2da94..bc63087d3 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/DateTimePartsEncodingEncoderTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/DateTimePartsEncodingEncoderTest.java @@ -34,9 +34,8 @@ class DateTimePartsEncodingEncoderTest { private static final DType EXT_TIMESTAMP_NS = timestampDType(TimeUnit.Nanoseconds); private static DType timestampDType(TimeUnit unit) { - MemorySegment meta = MemorySegment.ofArray(new byte[]{(byte) unit.ordinal(), 0, 0}); return new DType.Extension("vortex.timestamp", - DTypes.I64, meta, false); + DTypes.I64, new io.github.dfa1.vortex.core.model.TimestampMetadata(unit, null), false); } private static ArrayNode toArrayNode(EncodeNode node) { @@ -89,12 +88,12 @@ void encode_producesThreeChildren_noBuffersAtRoot() { void encode_missingMetadata_throws() { // Given DType noMeta = new DType.Extension("vortex.timestamp", - DTypes.I64, null, false); + DTypes.I64, io.github.dfa1.vortex.core.model.OpaqueMetadata.EMPTY, false); DateTimePartsData data = new DateTimePartsData(new long[]{0L}, false); // When / Then assertThatThrownBy(() -> ENCODER.encode(noMeta, data, EncodeTestHelper.testCtx())) - .hasMessageContaining("extension metadata missing"); + .hasMessageContaining("extension metadata is not timestamp metadata"); } } diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ExtEncodingEncoderTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ExtEncodingEncoderTest.java index 91857a127..f0bb5ad9d 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ExtEncodingEncoderTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ExtEncodingEncoderTest.java @@ -7,6 +7,9 @@ import io.github.dfa1.vortex.reader.decode.DecodeContext; import io.github.dfa1.vortex.core.model.EncodingId; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; +import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimestampMetadata; import io.github.dfa1.vortex.writer.WriteRegistry; import io.github.dfa1.vortex.reader.ReadRegistry; import io.github.dfa1.vortex.reader.decode.TestRegistry; @@ -32,6 +35,7 @@ class ExtEncodingEncoderTest { + private static final TimestampMetadata TIMESTAMP_MS = new TimestampMetadata(TimeUnit.Milliseconds, null); private static final ExtEncodingEncoder ENCODER = new ExtEncodingEncoder(); private static final ExtEncodingDecoder DECODER = new ExtEncodingDecoder(); private static final ReadRegistry REGISTRY = TestRegistry.ofDecoders(new PrimitiveEncodingDecoder(), DECODER); @@ -45,7 +49,7 @@ class ZoneMapStats { // pruning and MIN/MAX push-down purely for being a timestamp rather than a bare i64. private static final DType.Extension TIMESTAMP = - new DType.Extension("vortex.timestamp", DTypes.I64, null, false); + new DType.Extension("vortex.timestamp", DTypes.I64, TIMESTAMP_MS, false); @Test void encode_extensionColumn_reportsStorageSpaceBounds() throws Exception { @@ -95,7 +99,7 @@ class Encode { void accepts_extensionDtype_returnsTrue() { // Given DType extDType = new DType.Extension("vortex.timestamp", - DTypes.I64, null, false); + DTypes.I64, TIMESTAMP_MS, false); // When / Then assertThat(ENCODER.accepts(extDType)).isTrue(); @@ -112,7 +116,7 @@ void encode_extensionWrappingI64_roundTrips() { // Given long[] data = {100L, 200L, 300L, 400L}; DType storageDType = DTypes.I64; - DType extDType = new DType.Extension("vortex.timestamp", storageDType, null, false); + DType extDType = new DType.Extension("vortex.timestamp", storageDType, TIMESTAMP_MS, false); // When EncodeResult resultEncoded = ENCODER.encode(extDType, data, EncodeTestHelper.testCtx()); @@ -153,7 +157,7 @@ void encodeCascade_exposesStorageAsOpenChild() { // Given long[] data = {100L, 200L, 300L, 400L}; DType storageDType = DTypes.I64; - DType extDType = new DType.Extension("vortex.timestamp", storageDType, null, false); + DType extDType = new DType.Extension("vortex.timestamp", storageDType, TIMESTAMP_MS, false); // When CascadeStep result = ENCODER.encodeCascade(extDType, data, EncodeTestHelper.testCtx()); @@ -191,7 +195,7 @@ void decode_extensionWrappingI64_returnsStorageArray() { MemorySegment buf = TestSegments.leLongs(values); DType storageDType = DTypes.I64; - DType extDType = new DType.Extension("vortex.timestamp", storageDType, null, false); + DType extDType = new DType.Extension("vortex.timestamp", storageDType, TIMESTAMP_MS, false); ArrayNode primitiveNode = new ArrayNode(EncodingId.VORTEX_PRIMITIVE, null, new ArrayNode[0], new int[]{0}); ArrayNode extNode = new ArrayNode(EncodingId.VORTEX_EXT, null, new ArrayNode[]{primitiveNode}, new int[0]); @@ -257,7 +261,7 @@ static Stream everyWellKnownExtension() { nulls)), Arguments.of(UuidExtensionEncoder.INSTANCE.dtype(true), UuidExtensionEncoder.INSTANCE.encodeAll(UuidExtensionEncoder.INSTANCE.dtype(true), nulls)), - Arguments.of(new DType.Extension("acme.custom", DTypes.U16, null, true), + Arguments.of(new DType.Extension("acme.custom", DTypes.U16, OpaqueMetadata.EMPTY, true), new NullableData(new short[ROWS], new boolean[ROWS]))); } diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/TimeExtensionEncoderTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/TimeExtensionEncoderTest.java index a76aa21b2..4261770e9 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/TimeExtensionEncoderTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/TimeExtensionEncoderTest.java @@ -9,7 +9,6 @@ import io.github.dfa1.vortex.core.model.TimeDtype; import org.junit.jupiter.api.Test; -import java.lang.foreign.MemorySegment; import java.time.LocalTime; import java.util.Arrays; import java.util.List; @@ -156,9 +155,9 @@ void encodeAll_nullInNonNullableColumn_throws() { @Test void encodeAll_daysUnit_throws() { // Given a hand-built Days-tagged dtype (TimeDtype.of rejects Days, so build directly) - MemorySegment meta = MemorySegment.ofArray(new byte[]{(byte) TimeUnit.Days.ordinal()}); DType.Extension dtype = new DType.Extension( - ExtensionId.VORTEX_TIME.id(), DTypes.I32, meta, false); + ExtensionId.VORTEX_TIME.id(), DTypes.I32, + new io.github.dfa1.vortex.core.model.TimeUnitMetadata(TimeUnit.Days), false); List values = List.of(T); // When / Then diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ZoneMapStatsFitnessTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ZoneMapStatsFitnessTest.java index 4283e4aa6..1c476d6ed 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ZoneMapStatsFitnessTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ZoneMapStatsFitnessTest.java @@ -174,7 +174,8 @@ private static Stream boundsOwingSamples() { Arguments.of(EncodingId.VORTEX_PCO, DTypes.I64, ascending), Arguments.of(EncodingId.VORTEX_ZIGZAG, DTypes.I64, ascending), Arguments.of(EncodingId.VORTEX_EXT, - new DType.Extension("vortex.timestamp", DTypes.I64, null, false), ascending), + new DType.Extension("vortex.timestamp", DTypes.I64, new io.github.dfa1.vortex.core.model.TimestampMetadata( + io.github.dfa1.vortex.core.model.TimeUnit.Milliseconds, null), false), ascending), Arguments.of(EncodingId.VORTEX_SPARSE, DTypes.I64, sparse), Arguments.of(EncodingId.VORTEX_PATCHED, DTypes.I64, ascending), Arguments.of(EncodingId.VORTEX_DECIMAL, new DType.Decimal((byte) 9, (byte) 2, false), From cab193f23315991d24a7eb78954347f2998d6940 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sun, 1 Nov 2026 09:38:42 +0100 Subject: [PATCH 2/6] refactor(core): move MemorySize to core.io MemorySize is a byte count used by the I/O bounds (extension metadata cap, layout metadata cap, global-dict retention), not part of the dtype model, so it moves next to IoBounds in core.io. The AGENTS.md module map also lists the DTypes and extension metadata types it was missing. Co-Authored-By: Claude Sonnet 5 --- AGENTS.md | 7 ++++--- CHANGELOG.md | 1 + .../github/dfa1/vortex/core/{model => io}/MemorySize.java | 2 +- .../io/github/dfa1/vortex/core/model/OpaqueMetadata.java | 1 + .../dfa1/vortex/core/{model => io}/MemorySizeTest.java | 2 +- docs/reference.md | 2 +- .../performance/JniWritesJavaReadsBigFileBenchmark.java | 2 +- .../io/github/dfa1/vortex/reader/PostscriptParser.java | 2 +- .../io/github/dfa1/vortex/reader/VortexHttpReader.java | 2 +- .../java/io/github/dfa1/vortex/writer/WriteOptions.java | 2 +- .../io/github/dfa1/vortex/writer/ColumnEncodingTest.java | 2 +- .../io/github/dfa1/vortex/writer/GlobalDictUtf8Test.java | 2 +- .../io/github/dfa1/vortex/writer/WriteOptionsTest.java | 2 +- 13 files changed, 16 insertions(+), 13 deletions(-) rename core/src/main/java/io/github/dfa1/vortex/core/{model => io}/MemorySize.java (98%) rename core/src/test/java/io/github/dfa1/vortex/core/{model => io}/MemorySizeTest.java (98%) diff --git a/AGENTS.md b/AGENTS.md index c78ea9382..4f8adca17 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,9 +34,10 @@ fsst — io.github.dfa1.vortex.fsst: standalone FSST (Fast Static Symbol Tabl Zero dependency on core/reader/writer/FFM-wire-concerns beyond the JDK's own java.lang.foreign; writer/reader depend on it, not the other way around. core — everything lives under `io.github.dfa1.vortex.core.*`: - core.model DType, PType, TimeUnit, EncodingId, LayoutId, ColumnName, ExtensionId, TimeDtype, TimestampDtype, - EditionId, Edition, EditionFamily, Editions, MemorySize - core.io IoBounds, PTypeIO, VortexFormat + core.model DType, DTypes, PType, TimeUnit, EncodingId, LayoutId, ColumnName, ExtensionId, TimeDtype, TimestampDtype, + ExtensionMetadata, OpaqueMetadata, TimeUnitMetadata, TimestampMetadata, + EditionId, Edition, EditionFamily, Editions + core.io IoBounds, PTypeIO, VortexFormat, MemorySize core.error VortexException core.compute FastLanes, PrimitiveArrays, Utf8Order, Utf8Validation core.simd SimdOperations (kernels shared by reader+writer, all eleven ptypes), diff --git a/CHANGELOG.md b/CHANGELOG.md index c69906665..c8b156509 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking:** CSV import stores an empty cell in a number or boolean column as null instead of `0` or `false`, so inferred number and boolean columns are now nullable (read through `MaskedArray`), and an empty cell or a missing column fails the import where the schema declares the column non-nullable. ([#592](https://github.com/dfa1/vortex-java/issues/592)) - **Breaking:** the shared non-nullable dtypes move from `DType` to the new `DTypes` class: `DTypes.I64`, `DTypes.UTF8`, … ([#624](https://github.com/dfa1/vortex-java/pull/624)) - **Breaking:** an extension dtype's metadata is a typed `ExtensionMetadata` record instead of a `MemorySegment`, parsed when a file opens and serialized when it is written, as Rust does: `TimeUnitMetadata` for date and time, `TimestampMetadata` with a `ZoneId` for timestamp, `OpaqueMetadata` for bytes nothing parses, and a third-party record through `ExtensionDecoder#parseMetadata` and `ExtensionEncoder#serializeMetadata`. ([#625](https://github.com/dfa1/vortex-java/pull/625)) +- **Breaking:** `MemorySize` moves from `io.github.dfa1.vortex.core.model` to `io.github.dfa1.vortex.core.io`. ([#625](https://github.com/dfa1/vortex-java/pull/625)) ### Fixed - Two threads first using `DType` at the same time no longer deadlock the JVM, as they could when, for example, two Parquet imports started together. ([#624](https://github.com/dfa1/vortex-java/pull/624)) diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/MemorySize.java b/core/src/main/java/io/github/dfa1/vortex/core/io/MemorySize.java similarity index 98% rename from core/src/main/java/io/github/dfa1/vortex/core/model/MemorySize.java rename to core/src/main/java/io/github/dfa1/vortex/core/io/MemorySize.java index d802d40a7..e84479c16 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/MemorySize.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/io/MemorySize.java @@ -1,4 +1,4 @@ -package io.github.dfa1.vortex.core.model; +package io.github.dfa1.vortex.core.io; /// A validated, non-negative byte count. /// diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java index 9620b43a8..a98fa3e9c 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java @@ -1,6 +1,7 @@ package io.github.dfa1.vortex.core.model; import io.github.dfa1.vortex.core.error.VortexException; +import io.github.dfa1.vortex.core.io.MemorySize; import java.lang.foreign.MemorySegment; import java.lang.foreign.ValueLayout; diff --git a/core/src/test/java/io/github/dfa1/vortex/core/model/MemorySizeTest.java b/core/src/test/java/io/github/dfa1/vortex/core/io/MemorySizeTest.java similarity index 98% rename from core/src/test/java/io/github/dfa1/vortex/core/model/MemorySizeTest.java rename to core/src/test/java/io/github/dfa1/vortex/core/io/MemorySizeTest.java index 9d503c155..bc266629f 100644 --- a/core/src/test/java/io/github/dfa1/vortex/core/model/MemorySizeTest.java +++ b/core/src/test/java/io/github/dfa1/vortex/core/io/MemorySizeTest.java @@ -1,4 +1,4 @@ -package io.github.dfa1.vortex.core.model; +package io.github.dfa1.vortex.core.io; import org.junit.jupiter.api.Test; diff --git a/docs/reference.md b/docs/reference.md index d345e2e2e..54e1ce9ff 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -70,7 +70,7 @@ shortcut returning a nullable copy), `withNullable(boolean)`, `DType.Struct.fiel | `ExtensionId` | `sealed interface` — `WellKnown` enum + `Custom` record | Extension-type identity (`vortex.date`/`time`/`timestamp`/`uuid` well-known); same `parse(String)` and `Custom` policy as `EncodingId`. `WellKnown#validate(DType.Extension)` applies Rust's metadata and storage checks for the four spec types. `DType.Extension` still carries the raw string; the file parser rejects ids `parse` would refuse | | `LayoutId` | `sealed interface` — `WellKnown` enum + `Custom` record | Layout identity (separate namespace from encodings; `vortex.flat` is layout-only); both zoned aliases `vortex.zoned`/`vortex.stats` | | `ColumnName` | `record ColumnName(String value)` | Validated column name: non-blank, no control characters. `ColumnName.violation(String)` is the policy chokepoint shared by builder, writer, and file parser | -| `MemorySize` | `record MemorySize(long bytes)` | Validated non-negative byte count; `ofKiB`/`ofMiB`/`ofGiB` factories, `toGiB()` for display. Rejects negative values (`IllegalArgumentException`) — a programmer-error guard, not a `VortexException`-worthy untrusted-input check | +| `MemorySize` | `record MemorySize(long bytes)` | In `io.github.dfa1.vortex.core.io`. Validated non-negative byte count; `ofKiB`/`ofMiB`/`ofGiB` factories, `toGiB()` for display. Rejects negative values (`IllegalArgumentException`) — a programmer-error guard, not a `VortexException`-worthy untrusted-input check | | `EditionFamily` | `enum` — `CORE`, `PREVIEW`, `ZSTD` | Closed (unlike `EncodingId`/`LayoutId`): an edition family is a cross-implementation compatibility promise, so a custom family carries no real guarantee — see [Editions](#editions) | | `EditionId` | `record EditionId(EditionFamily family, YearMonth cutMonth, int version)` | e.g. `core2025.05.0`; `isAtOrBefore` orders editions within a family only | | `Edition` | `record Edition(EditionId id, Set added)` | Only `Editions`'s catalog constants should be constructed (API contract, not compiler-enforced — see ADR 0023) | diff --git a/performance/src/main/java/io/github/dfa1/vortex/performance/JniWritesJavaReadsBigFileBenchmark.java b/performance/src/main/java/io/github/dfa1/vortex/performance/JniWritesJavaReadsBigFileBenchmark.java index 16ff1a4e9..f5eacc580 100644 --- a/performance/src/main/java/io/github/dfa1/vortex/performance/JniWritesJavaReadsBigFileBenchmark.java +++ b/performance/src/main/java/io/github/dfa1/vortex/performance/JniWritesJavaReadsBigFileBenchmark.java @@ -9,7 +9,7 @@ import dev.vortex.arrow.ArrowAllocation; import dev.vortex.jni.NativeLoader; import io.github.dfa1.vortex.core.model.ColumnName; -import io.github.dfa1.vortex.core.model.MemorySize; +import io.github.dfa1.vortex.core.io.MemorySize; import io.github.dfa1.vortex.reader.array.Array; import io.github.dfa1.vortex.reader.ReadRegistry; import io.github.dfa1.vortex.reader.VortexReader; diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java b/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java index d52128048..ca06f7468 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java @@ -5,7 +5,7 @@ import io.github.dfa1.vortex.core.model.EncodingId; import io.github.dfa1.vortex.core.model.ExtensionId; import io.github.dfa1.vortex.core.model.LayoutId; -import io.github.dfa1.vortex.core.model.MemorySize; +import io.github.dfa1.vortex.core.io.MemorySize; import io.github.dfa1.vortex.core.model.PType; import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.fbs.FbsBinary; diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/VortexHttpReader.java b/reader/src/main/java/io/github/dfa1/vortex/reader/VortexHttpReader.java index 50ab2f082..da24b8b1f 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/VortexHttpReader.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/VortexHttpReader.java @@ -1,7 +1,7 @@ package io.github.dfa1.vortex.reader; import io.github.dfa1.vortex.core.model.DType; -import io.github.dfa1.vortex.core.model.MemorySize; +import io.github.dfa1.vortex.core.io.MemorySize; import io.github.dfa1.vortex.core.io.IoBounds; import io.github.dfa1.vortex.core.error.VortexException; import io.github.dfa1.vortex.core.io.VortexFormat; diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/WriteOptions.java b/writer/src/main/java/io/github/dfa1/vortex/writer/WriteOptions.java index 55d8462cc..da970e74d 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/WriteOptions.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/WriteOptions.java @@ -4,7 +4,7 @@ import io.github.dfa1.vortex.core.model.Edition; import io.github.dfa1.vortex.core.model.EditionFamily; import io.github.dfa1.vortex.core.model.Editions; -import io.github.dfa1.vortex.core.model.MemorySize; +import io.github.dfa1.vortex.core.io.MemorySize; import java.util.HashMap; import java.util.Map; diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/ColumnEncodingTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/ColumnEncodingTest.java index a86614a81..6dbda80c8 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/ColumnEncodingTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/ColumnEncodingTest.java @@ -150,7 +150,7 @@ void copyMethodsKeepTheOverride() { // When WriteOptions result = base.withZoneMaps(false).withGlobalDict(false).withoutEditions() - .withGlobalDictMaxRetainedBytes(new io.github.dfa1.vortex.core.model.MemorySize(1)); + .withGlobalDictMaxRetainedBytes(new io.github.dfa1.vortex.core.io.MemorySize(1)); // Then assertThat(result.columnEncodings()).containsEntry(PRICE, ALP_CASCADE); diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/GlobalDictUtf8Test.java b/writer/src/test/java/io/github/dfa1/vortex/writer/GlobalDictUtf8Test.java index ead58f49d..0124f6f81 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/GlobalDictUtf8Test.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/GlobalDictUtf8Test.java @@ -3,7 +3,7 @@ import io.github.dfa1.vortex.core.model.ColumnName; import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.DTypes; -import io.github.dfa1.vortex.core.model.MemorySize; +import io.github.dfa1.vortex.core.io.MemorySize; import io.github.dfa1.vortex.reader.ReadRegistry; import io.github.dfa1.vortex.reader.ScanOptions; import io.github.dfa1.vortex.reader.VortexReader; diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/WriteOptionsTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/WriteOptionsTest.java index f40eb2eec..a0848f845 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/WriteOptionsTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/WriteOptionsTest.java @@ -1,6 +1,6 @@ package io.github.dfa1.vortex.writer; -import io.github.dfa1.vortex.core.model.MemorySize; +import io.github.dfa1.vortex.core.io.MemorySize; import org.junit.jupiter.api.Test; From ec3824b6850fd0b88543a855b1f4a4d8dc4ca845 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sun, 1 Nov 2026 09:43:45 +0100 Subject: [PATCH 3/6] docs: link the custom extension recipe and its integration test The how-to's "Add a custom extension type" recipe and CustomExtensionIntegrationTest show the same extension SPI; each now points at the other so they change together, the test being the complete, tested example. Co-Authored-By: Claude Sonnet 5 --- docs/how-to.md | 7 +++++++ .../vortex/integration/CustomExtensionIntegrationTest.java | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/how-to.md b/docs/how-to.md index 26bf42aa6..58120b453 100644 --- a/docs/how-to.md +++ b/docs/how-to.md @@ -1149,6 +1149,13 @@ the file, as Rust does for an extension id its session has no vtable for; with for an extension column needs a registered encoder; the storage array itself (`long[]` here) can always be written. Auto-routing a `Collection` applies to top-level columns only. +The complete, tested example is +[`CustomExtensionIntegrationTest`](../integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java): +an `example.vector` embedding type with the same pieces as this recipe (a typed metadata record, +`serializeMetadata`, `parseMetadata`, `validate` on both sides), written and read back through full, +filtered and limited scans, plus the unknown-id and validation failures. The recipe and the test +change together. + --- ## Read files with unknown encodings diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java index f3f169b8d..27f66e8a2 100644 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/CustomExtensionIntegrationTest.java @@ -57,10 +57,14 @@ /// "adding a new logical type" (an `ExtVTable` registered on the session). /// /// The type is a fixed-dimension `float[]` embedding over `FixedSizeList` storage, with -/// the dimension also recorded in the metadata (u32 LE), so the test covers non-primitive storage, +/// the dimension also recorded in a typed `VectorMetadata` (a u32 LE on the wire), so the test covers non-primitive storage, /// metadata round-trip, nulls and the `validate` hook on both sides. Reads go through a full scan, a /// filter on a sibling column and a limit: a full scan never prunes or slices, and a sliced /// fixed-size list must keep each vector's floats with its row. +/// +/// The documented counterpart is the "Add a custom extension type" recipe in `docs/how-to.md`, +/// which links here as the complete example: both show the same extension SPI (a typed metadata +/// record, `serializeMetadata`, `parseMetadata`, `validate`), so a change to one changes the other. class CustomExtensionIntegrationTest { static { From 0e800d6e45fbd34d47ccc9f4e14e826274936022 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sun, 1 Nov 2026 14:04:05 +0100 Subject: [PATCH 4/6] fix: review findings on typed extension metadata - The writer checks serialized metadata against the reader's 64 KiB cap, and refuses an encoder returning null, both naming the id: it could write a file its own reader refuses. - The reader applies the cap to every id again, the temporal ones included. - A timestamp's zone is kept as the string the file stores, as Rust keeps it: a file whose zone this JDK's database does not know opens again, and a rewrite keeps its bytes (ZoneId normalized "+05" to "+05:00"). TimestampMetadata#zone() parses it. - OpaqueMetadata copies every segment: a caller's heap array, changed later, changed the value and its hash. - The writer serializes each extension's metadata once (one serializer per writer), so the bytes written at close are the bytes checked at create; a built-in record under a custom id goes through that id's encoder, so it reads back through its own decoder. - resolveExtensions returns the same node when nothing changed: the Variant and Constant decoders run it per chunk. - One untrusted-id guard, ExtensionMetadataParser.parseId, instead of three. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 2 +- .../vortex/core/model/ExtensionMetadata.java | 4 +- .../vortex/core/model/OpaqueMetadata.java | 12 +- .../vortex/core/model/TimestampDtype.java | 5 +- .../vortex/core/model/TimestampMetadata.java | 37 ++++- .../vortex/core/model/DTypeExtensionTest.java | 15 ++ .../vortex/core/model/ExtensionIdTest.java | 2 +- docs/compatibility.md | 5 +- .../dfa1/vortex/reader/PostscriptParser.java | 19 +-- .../dfa1/vortex/reader/ReadRegistry.java | 81 ++++++---- .../reader/decode/VariantEncodingDecoder.java | 7 +- .../extension/ExtensionMetadataParser.java | 43 ++++-- .../PostscriptParserDTypeGuardsTest.java | 2 +- .../dfa1/vortex/reader/ReadRegistryTest.java | 15 ++ .../ExtensionMetadataParserTest.java | 9 +- .../vortex/writer/DTypeFbsSerializer.java | 64 +++++--- .../dfa1/vortex/writer/ExtensionEncoder.java | 7 +- .../dfa1/vortex/writer/VortexWriter.java | 7 +- .../vortex/writer/DTypeFbsSerializerTest.java | 141 ++++++++++++++++++ 19 files changed, 366 insertions(+), 111 deletions(-) create mode 100644 writer/src/test/java/io/github/dfa1/vortex/writer/DTypeFbsSerializerTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index c8b156509..89b3e563e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking:** `ScanOptions.filter()` and `limit()` return `Optional` and `Optional`, as Rust's `ScanBuilder` holds them; `withLimit` takes a `ScanLimit`, and `NO_LIMIT`, `ScanOptions.limit(long)`, `rowFilter()`, `hasFilter()` and `hasLimit()` are gone. ([#598](https://github.com/dfa1/vortex-java/pull/598)) - **Breaking:** CSV import stores an empty cell in a number or boolean column as null instead of `0` or `false`, so inferred number and boolean columns are now nullable (read through `MaskedArray`), and an empty cell or a missing column fails the import where the schema declares the column non-nullable. ([#592](https://github.com/dfa1/vortex-java/issues/592)) - **Breaking:** the shared non-nullable dtypes move from `DType` to the new `DTypes` class: `DTypes.I64`, `DTypes.UTF8`, … ([#624](https://github.com/dfa1/vortex-java/pull/624)) -- **Breaking:** an extension dtype's metadata is a typed `ExtensionMetadata` record instead of a `MemorySegment`, parsed when a file opens and serialized when it is written, as Rust does: `TimeUnitMetadata` for date and time, `TimestampMetadata` with a `ZoneId` for timestamp, `OpaqueMetadata` for bytes nothing parses, and a third-party record through `ExtensionDecoder#parseMetadata` and `ExtensionEncoder#serializeMetadata`. ([#625](https://github.com/dfa1/vortex-java/pull/625)) +- **Breaking:** an extension dtype's metadata is a typed `ExtensionMetadata` record instead of a `MemorySegment`, parsed when a file opens and serialized when it is written, as Rust does: `TimeUnitMetadata` for date and time, `TimestampMetadata` (its zone kept as stored) for timestamp, `OpaqueMetadata` for bytes nothing parses, and a third-party record through `ExtensionDecoder#parseMetadata` and `ExtensionEncoder#serializeMetadata`. ([#625](https://github.com/dfa1/vortex-java/pull/625)) - **Breaking:** `MemorySize` moves from `io.github.dfa1.vortex.core.model` to `io.github.dfa1.vortex.core.io`. ([#625](https://github.com/dfa1/vortex-java/pull/625)) ### Fixed diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java index 7d05d2403..8a6a8d556 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java @@ -5,7 +5,9 @@ /// back; nothing in between sees the bytes. /// /// An extension point: a third-party extension defines its own. It must be a **record**, so a -/// dtype compares and hashes by value ([DType.Extension] refuses anything else). The well-known +/// dtype compares and hashes by value ([DType.Extension] refuses anything else), with components +/// that do too: a `byte[]` or `MemorySegment` component compares by identity unless the record +/// overrides `equals` and `hashCode`, as [OpaqueMetadata] does. The well-known /// temporal extensions use [TimeUnitMetadata] and [TimestampMetadata]; every other id keeps its /// bytes as [OpaqueMetadata] (Rust's `ForeignExtMetadata`). public interface ExtensionMetadata { diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java index a98fa3e9c..5934f04c7 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java @@ -39,13 +39,11 @@ public record OpaqueMetadata(MemorySegment bytes) implements ExtensionMetadata { if (bytes.byteSize() > MAX_SIZE.bytes()) { throw new VortexException("extension metadata too large: " + bytes.byteSize() + " > " + MAX_SIZE.bytes()); } - // A dtype outlives the reader that parsed it (callers keep reader.dtype()), but a slice of - // the file mapping fails once the reader closes: copy that one onto the heap. Small, as the - // cap above bounds it; a heap segment is kept as it is. - if (bytes.isNative()) { - bytes = MemorySegment.ofArray(bytes.toArray(ValueLayout.JAVA_BYTE)); - } - bytes = bytes.asReadOnly(); + // Always a copy: a slice of the file mapping fails once the reader closes, though a dtype + // outlives it (callers keep reader.dtype()); and a caller's heap array, changed later, + // would change this value and its hash under a map holding the dtype. Small, as the cap + // above bounds it. + bytes = MemorySegment.ofArray(bytes.toArray(ValueLayout.JAVA_BYTE)).asReadOnly(); } @Override diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampDtype.java b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampDtype.java index 398283ee1..7b5a89409 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampDtype.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampDtype.java @@ -30,7 +30,7 @@ public static DType.Extension of(TimeUnit unit, ZoneId zone, boolean nullable) { return new DType.Extension( ExtensionId.VORTEX_TIMESTAMP.id(), new DType.Primitive(PType.I64, nullable), - new TimestampMetadata(unit, zone), + TimestampMetadata.of(unit, zone), nullable); } @@ -48,8 +48,9 @@ public static TimeUnit readUnit(DType.Extension ext) { /// @param ext declared extension dtype /// @return parsed zone id, or empty when no timezone is recorded /// @throws VortexException if the metadata is not a [TimestampMetadata] + /// @throws java.time.DateTimeException if the stored timezone is not a zone id this JDK knows public static Optional timezone(DType.Extension ext) { - return Optional.ofNullable(metadata(ext).zone()); + return metadata(ext).zone(); } private static TimestampMetadata metadata(DType.Extension ext) { diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java index e0ebc00cd..815c89f57 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java @@ -2,19 +2,44 @@ import java.time.ZoneId; import java.util.Objects; +import java.util.Optional; /// The metadata of `vortex.timestamp`, Rust's `TimestampOptions`: the [TimeUnit] its `I64` storage /// counts in and an optional timezone. /// -/// @param unit the storage's time unit -/// @param zone the timezone, or `null` for none -public record TimestampMetadata(TimeUnit unit, ZoneId zone) implements ExtensionMetadata { +/// The timezone is the string the file stores, as Rust keeps it, not a [ZoneId]: a file opens +/// whatever zone it names, even one this JDK's zone database does not know, and writing it back +/// keeps the exact bytes (`ZoneId` normalizes, e.g. `+05` to `+05:00`). [#zone()] parses it. +/// +/// @param unit the storage's time unit +/// @param timezone the timezone, or `null` for none +public record TimestampMetadata(TimeUnit unit, String timezone) implements ExtensionMetadata { - /// Requires a unit. + /// Requires a unit; an empty timezone is none, as on the wire. /// - /// @param unit the storage's time unit - /// @param zone the timezone, or `null` for none + /// @param unit the storage's time unit + /// @param timezone the timezone, or `null` for none public TimestampMetadata { Objects.requireNonNull(unit, "unit"); + if (timezone != null && timezone.isEmpty()) { + timezone = null; + } + } + + /// Metadata for `zone`, stored by its id. + /// + /// @param unit the storage's time unit + /// @param zone the timezone, or `null` for none + /// @return the metadata + public static TimestampMetadata of(TimeUnit unit, ZoneId zone) { + return new TimestampMetadata(unit, zone == null ? null : zone.getId()); + } + + /// The timezone as a [ZoneId]. + /// + /// @return the zone, or empty when none is recorded + /// @throws java.time.DateTimeException if the stored timezone is not a zone id this JDK knows + public Optional zone() { + return Optional.ofNullable(timezone).map(ZoneId::of); } } diff --git a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java index 03209b0aa..de083ff1a 100644 --- a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java +++ b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java @@ -100,6 +100,21 @@ void nativeBytes_outliveTheirArena() { assertThat(result.bytes().get(ValueLayout.JAVA_BYTE, 0)).isEqualTo((byte) 3); } + @Test + void callerArray_changedLater_doesNotChangeTheValue() { + // Given metadata built from a caller's array + byte[] caller = {1}; + OpaqueMetadata sut = new OpaqueMetadata(MemorySegment.ofArray(caller)); + int hash = sut.hashCode(); + + // When the caller reuses its array + caller[0] = 2; + + // Then — a dtype holding it keeps its equality and hash, so a map holding it still finds it + assertThat(sut).isEqualTo(new OpaqueMetadata(MemorySegment.ofArray(new byte[]{1}))); + assertThat(sut.hashCode()).isEqualTo(hash); + } + @Test void bytes_areReadOnly() { // Given diff --git a/core/src/test/java/io/github/dfa1/vortex/core/model/ExtensionIdTest.java b/core/src/test/java/io/github/dfa1/vortex/core/model/ExtensionIdTest.java index f83700930..0070875d6 100644 --- a/core/src/test/java/io/github/dfa1/vortex/core/model/ExtensionIdTest.java +++ b/core/src/test/java/io/github/dfa1/vortex/core/model/ExtensionIdTest.java @@ -223,7 +223,7 @@ private static DType.Extension unit(ExtensionId id, DType storage, TimeUnit unit } private static DType.Extension timestamp(DType storage, TimeUnit unit, ZoneId zone) { - return new DType.Extension(ExtensionId.VORTEX_TIMESTAMP.id(), storage, new TimestampMetadata(unit, zone), + return new DType.Extension(ExtensionId.VORTEX_TIMESTAMP.id(), storage, TimestampMetadata.of(unit, zone), storage.nullable()); } } diff --git a/docs/compatibility.md b/docs/compatibility.md index 95e5f7d0f..78d2db66b 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -322,8 +322,9 @@ built-ins apply Rust's `deserialize_metadata` + `validate_dtype` rules UTF-8 zone and `I64` storage; `vortex.uuid` 0–1 metadata bytes (a known version) over `FixedSizeList(non-null U8, 16)`. An extension dtype must carry its metadata field, empty when there is none, as Rust requires; the writer always emits it. As in Rust, the metadata is parsed when -the file opens (a malformed one, or a timestamp zone `java.time.ZoneId` cannot parse, refuses the -file) and kept typed; an id nothing parses keeps its bytes (`OpaqueMetadata`, Rust's +the file opens (a malformed one refuses the file) and kept typed; a timestamp's zone is kept as the +string the file stores, so a zone this JDK's database does not know still opens and round-trips, +and only `TimestampMetadata#zone()` parses it; an id nothing parses keeps its bytes (`OpaqueMetadata`, Rust's `ForeignExtMetadata`). `ExtensionId` is a sealed `WellKnown`/`Custom` type like `EncodingId`: the spec ids diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java b/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java index ca06f7468..119bccde2 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/PostscriptParser.java @@ -164,23 +164,6 @@ private static EncodingId parseEncodingSpec(String raw, int index) { return EncodingId.parse(raw); } - /// Parses an extension dtype's wire id at the schema boundary, so every [DType.Extension] read - /// from a file carries an id [ExtensionId#parse] accepts. A missing, blank or control-character - /// id is untrusted-input corruption that would otherwise surface as an [IllegalArgumentException]. - /// - /// @param raw the wire string from the extension dtype, or `null` when the field is absent - /// @return the typed [ExtensionId] - private static ExtensionId parseExtensionId(String raw) { - if (raw == null) { - throw new VortexException("extension dtype missing id"); - } - try { - return ExtensionId.parse(raw); - } catch (IllegalArgumentException ex) { - throw new VortexException("invalid extension id in file schema: " + ex.getMessage()); - } - } - /// Parses one layout-spec wire string to its [LayoutId] at the footer boundary. A blank id /// is untrusted-input corruption; [LayoutId#parse] would reject it with an /// [IllegalArgumentException], so surface it as a [VortexException] with the spec index. @@ -340,7 +323,7 @@ private static DType convertDType(io.github.dfa1.vortex.core.fbs.FbsDType fbs) { if (metadata == null) { throw new VortexException("extension dtype missing metadata"); } - ExtensionId id = parseExtensionId(e.id()); + ExtensionId id = ExtensionMetadataParser.parseId(e.id()); yield new DType.Extension( id.id(), storage, diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/ReadRegistry.java b/reader/src/main/java/io/github/dfa1/vortex/reader/ReadRegistry.java index 4afe1220c..b722e0d6d 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/ReadRegistry.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/ReadRegistry.java @@ -8,6 +8,7 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.EncodingId; import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; import io.github.dfa1.vortex.core.model.OpaqueMetadata; import io.github.dfa1.vortex.reader.decode.AlpEncodingDecoder; import io.github.dfa1.vortex.reader.decode.AlpRdEncodingDecoder; @@ -52,6 +53,7 @@ import io.github.dfa1.vortex.reader.decode.ZstdEncodingDecoder; import io.github.dfa1.vortex.reader.extension.DateExtensionDecoder; import io.github.dfa1.vortex.reader.extension.ExtensionDecoder; +import io.github.dfa1.vortex.reader.extension.ExtensionMetadataParser; import io.github.dfa1.vortex.reader.extension.TimeExtensionDecoder; import io.github.dfa1.vortex.reader.extension.TimestampExtensionDecoder; import io.github.dfa1.vortex.reader.extension.UuidExtensionDecoder; @@ -157,13 +159,18 @@ public ExtensionDecoder lookup(ExtensionId extensionId) { /// @throws VortexException if a registered decoder rejects its metadata or dtype, or an /// extension id has no decoder and unknown ids are not allowed public DType resolveExtensions(DType dtype) { + // A node is rebuilt only when something under it changed: the Variant and Constant + // decoders run this on every chunk, and nearly always nothing does. return switch (dtype) { case null -> null; case DType.Extension ext -> { - ExtensionId id = parseId(ext.extensionId()); + ExtensionId id = ExtensionMetadataParser.parseId(ext.extensionId()); ExtensionDecoder decoder = extensions.get(id); - DType.Extension resolved = new DType.Extension(ext.extensionId(), - resolveExtensions(ext.storageDType()), ext.metadata(), ext.nullable()); + DType storage = resolveExtensions(ext.storageDType()); + DType.Extension resolved = ext; + if (storage != ext.storageDType()) { + resolved = new DType.Extension(ext.extensionId(), storage, ext.metadata(), ext.nullable()); + } if (decoder != null) { resolved = parseMetadata(decoder, resolved); validate(decoder, resolved); @@ -176,20 +183,51 @@ public DType resolveExtensions(DType dtype) { } yield resolved; } - case DType.Struct(var names, var types, var nullable) -> - new DType.Struct(names, types.stream().map(this::resolveExtensions).toList(), nullable); - case DType.List(var element, var nullable) -> new DType.List(resolveExtensions(element), nullable); - case DType.FixedSizeList(var element, var size, var nullable) -> - new DType.FixedSizeList(resolveExtensions(element), size, nullable); - case DType.Map(var key, var value, var keysSorted, var nullable) -> - new DType.Map(resolveExtensions(key), resolveExtensions(value), keysSorted, nullable); - case DType.Union(var names, var types, var typeIds, var nullable) -> - new DType.Union(names, types.stream().map(this::resolveExtensions).toList(), typeIds, nullable); + case DType.Struct struct -> { + java.util.List types = resolveAll(struct.fieldTypes()); + yield types == struct.fieldTypes() ? struct + : new DType.Struct(struct.fieldNames(), types, struct.nullable()); + } + case DType.List list -> { + DType element = resolveExtensions(list.elementType()); + yield element == list.elementType() ? list : new DType.List(element, list.nullable()); + } + case DType.FixedSizeList list -> { + DType element = resolveExtensions(list.elementType()); + yield element == list.elementType() ? list + : new DType.FixedSizeList(element, list.fixedSize(), list.nullable()); + } + case DType.Map map -> { + DType key = resolveExtensions(map.keyType()); + DType value = resolveExtensions(map.valueType()); + yield key == map.keyType() && value == map.valueType() ? map + : new DType.Map(key, value, map.keysSorted(), map.nullable()); + } + case DType.Union union -> { + java.util.List types = resolveAll(union.variantTypes()); + yield types == union.variantTypes() ? union + : new DType.Union(union.names(), types, union.typeIds(), union.nullable()); + } case DType.Null _, DType.Bool _, DType.Primitive _, DType.Decimal _, DType.Utf8 _, DType.Binary _, DType.Variant _ -> dtype; }; } + /// Resolves each of `types`, returning `types` itself when none changed. + private java.util.List resolveAll(java.util.List types) { + java.util.List out = null; + for (int i = 0; i < types.size(); i++) { + DType resolved = resolveExtensions(types.get(i)); + if (out == null && resolved != types.get(i)) { + out = new java.util.ArrayList<>(types.subList(0, i)); + } + if (out != null) { + out.add(resolved); + } + } + return out == null ? types : java.util.List.copyOf(out); + } + /// Parses metadata the reader kept as bytes with the extension's decoder. A third-party parser /// may trip over malformed bytes with any runtime exception, or return metadata that is not a /// record; reading a file must only ever fail with [VortexException]. @@ -198,8 +236,11 @@ private static DType.Extension parseMetadata(ExtensionDecoder decoder, DType. return ext; } try { - return new DType.Extension(ext.extensionId(), ext.storageDType(), decoder.parseMetadata(bytes), - ext.nullable()); + ExtensionMetadata parsed = decoder.parseMetadata(bytes); + if (ext.metadata().equals(parsed)) { + return ext; // a decoder keeping the bytes, as the default does + } + return new DType.Extension(ext.extensionId(), ext.storageDType(), parsed, ext.nullable()); } catch (VortexException e) { throw e; } catch (RuntimeException e) { @@ -207,18 +248,6 @@ private static DType.Extension parseMetadata(ExtensionDecoder decoder, DType. } } - /// Parses an extension id read from a file; an id [ExtensionId#parse] refuses is corrupt input. - private static ExtensionId parseId(String raw) { - if (raw == null) { - throw new VortexException("extension dtype missing id"); - } - try { - return ExtensionId.parse(raw); - } catch (IllegalArgumentException e) { - throw new VortexException("invalid extension id: " + e.getMessage()); - } - } - /// Runs a decoder's [ExtensionDecoder#validate(DType.Extension)] on a dtype from an untrusted /// file. A third-party check may trip over malformed metadata with any runtime exception; /// reading a file must only ever fail with [VortexException]. diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoder.java index adae98c8f..82c64d4fa 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/VariantEncodingDecoder.java @@ -126,12 +126,7 @@ static DType dtypeFromProto(io.github.dfa1.vortex.core.proto.ProtoDType proto) { if (proto.extension() != null) { // an extension's nullability is its storage's, as the file-schema parser has it DType storage = dtypeFromProto(proto.extension().storage_dtype()); - ExtensionId id; - try { - id = ExtensionId.parse(proto.extension().id()); - } catch (IllegalArgumentException | NullPointerException e) { - throw new VortexException("invalid extension id in variant metadata: " + e.getMessage()); - } + ExtensionId id = ExtensionMetadataParser.parseId(proto.extension().id()); byte[] metadata = proto.extension().metadata() != null ? proto.extension().metadata() : new byte[0]; return new DType.Extension( id.id(), diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java index 167eef2ef..5462c4b09 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java @@ -12,13 +12,11 @@ import java.lang.foreign.ValueLayout; import java.nio.charset.CharacterCodingException; import java.nio.charset.StandardCharsets; -import java.time.DateTimeException; -import java.time.ZoneId; import static io.github.dfa1.vortex.core.io.VortexFormat.LE_SHORT; -/// Parses the metadata bytes a file stores for an extension dtype, as Rust's -/// `deserialize_metadata`: [TimeUnitMetadata] for `vortex.date` and `vortex.time`, +/// Parses the wire fields of an extension dtype from untrusted input: its id, and its metadata as +/// Rust's `deserialize_metadata` does — [TimeUnitMetadata] for `vortex.date` and `vortex.time`, /// [TimestampMetadata] for `vortex.timestamp`, and the bytes as [OpaqueMetadata] for every other id. /// The writer's `DTypeFbsSerializer` is the inverse. public final class ExtensionMetadataParser { @@ -26,14 +24,41 @@ public final class ExtensionMetadataParser { private ExtensionMetadataParser() { } + /// Parses an extension id read from a file; an id [ExtensionId#parse(String)] refuses is + /// corrupt input. + /// + /// @param raw the id string, possibly `null` + /// @return the parsed id + /// @throws VortexException if `raw` is `null` or not a valid extension id + public static ExtensionId parseId(String raw) { + if (raw == null) { + throw new VortexException("extension dtype missing id"); + } + try { + return ExtensionId.parse(raw); + } catch (IllegalArgumentException e) { + throw new VortexException("invalid extension id: " + e.getMessage(), e); + } + } + /// Parses the metadata of extension `id`. /// + /// ponytail: the well-known temporal ids are parsed here rather than through their decoders' + /// `ExtensionDecoder#parseMetadata`, so the file parser needs no registry and a reader without + /// those decoders still types them; route them through the decoders if one ever needs to + /// override the parsing. + /// /// @param id the extension the bytes belong to /// @param metadata the wire bytes, from an untrusted file /// @return the parsed metadata /// @throws VortexException if the bytes are malformed for `id`, or longer than /// [OpaqueMetadata#MAX_SIZE] public static ExtensionMetadata parse(ExtensionId id, MemorySegment metadata) { + // every id, typed or not: an untrusted file must not make the reader take a huge blob + if (metadata.byteSize() > OpaqueMetadata.MAX_SIZE.bytes()) { + throw new VortexException("extension metadata too large: " + + metadata.byteSize() + " > " + OpaqueMetadata.MAX_SIZE.bytes()); + } if (id == ExtensionId.VORTEX_DATE || id == ExtensionId.VORTEX_TIME) { return new TimeUnitMetadata(unit(id, metadata)); } @@ -43,16 +68,14 @@ public static ExtensionMetadata parse(ExtensionId id, MemorySegment metadata) { return new OpaqueMetadata(metadata); } - /// `byte[0] = TimeUnit tag, bytes[1..3] = tz_len (u16 LE), bytes[3..3+tz_len] = tz UTF-8`. + /// `byte[0] = TimeUnit tag, bytes[1..3] = tz_len (u16 LE), bytes[3..3+tz_len] = tz UTF-8`. The + /// timezone is kept as stored, as Rust keeps it: no zone database decides whether a file opens. private static TimestampMetadata timestamp(ExtensionId id, MemorySegment metadata) { if (metadata.byteSize() < 3) { throw new VortexException(id + " metadata must have at least 3 bytes, got " + metadata.byteSize()); } TimeUnit unit = unit(id, metadata); int tzLen = Short.toUnsignedInt(metadata.get(LE_SHORT, 1)); - if (tzLen == 0) { - return new TimestampMetadata(unit, null); - } if (metadata.byteSize() < 3 + tzLen) { throw new VortexException(id + " metadata is truncated: declared timezone length " + tzLen + " but only " + (metadata.byteSize() - 3) + " bytes available"); @@ -60,11 +83,9 @@ private static TimestampMetadata timestamp(ExtensionId id, MemorySegment metadat try { String timezone = StandardCharsets.UTF_8.newDecoder() .decode(metadata.asSlice(3, tzLen).asByteBuffer()).toString(); - return new TimestampMetadata(unit, ZoneId.of(timezone)); + return new TimestampMetadata(unit, timezone); } catch (CharacterCodingException e) { throw new VortexException(id + " timezone is not valid UTF-8", e); - } catch (DateTimeException e) { - throw new VortexException(id + " timezone is not a valid zone id: " + e.getMessage(), e); } } diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/PostscriptParserDTypeGuardsTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/PostscriptParserDTypeGuardsTest.java index 86e86d77f..de5d6af6b 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/PostscriptParserDTypeGuardsTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/PostscriptParserDTypeGuardsTest.java @@ -211,7 +211,7 @@ void convertDType_invalidExtensionId_throwsVortexException(String id) { MemorySegment layout = flatLayout(); assertThatThrownBy(() -> PostscriptParser.parseBlobs(footer, layout, dtype)) .isInstanceOf(VortexException.class) - .hasMessageContaining("invalid extension id in file schema"); + .hasMessageContaining("invalid extension id"); } @Test diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/ReadRegistryTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/ReadRegistryTest.java index 1b7a3e305..9cb2bf02a 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/ReadRegistryTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/ReadRegistryTest.java @@ -268,6 +268,21 @@ void resolveExtensions_registeredDecoder_parsesItsMetadata() { List.of(new DType.List(geopoint(new Srid(42)), false)), false)); } + @Test + void resolveExtensions_nothingToParse_returnsTheSameInstance() { + // Given — a schema whose extension's decoder keeps the bytes, as the default does. The + // Variant and Constant decoders resolve per chunk, so an unchanged tree must not be rebuilt. + ReadRegistry sut = ReadRegistry.builder().register(extension("acme.geopoint")).build(); + DType schema = new DType.Struct(List.of(ColumnName.of("c"), ColumnName.of("d")), + List.of(DTypes.I64, new DType.List(geopoint(OpaqueMetadata.EMPTY), false)), false); + + // When + DType result = sut.resolveExtensions(schema); + + // Then + assertThat(result).isSameAs(schema); + } + @Test void resolveExtensions_parserReturnsNonRecord_wrapsInVortexException() { // Given — a third-party parser returning metadata without value equality diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java index 0fc374530..eaff12ba3 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java @@ -12,7 +12,6 @@ import org.junit.jupiter.params.provider.MethodSource; import java.lang.foreign.MemorySegment; -import java.time.ZoneId; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; @@ -28,10 +27,13 @@ static Stream valid() { Arguments.of(ExtensionId.VORTEX_TIME, bytes(0), new TimeUnitMetadata(TimeUnit.Nanoseconds)), Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 0, 0), new TimestampMetadata(TimeUnit.Milliseconds, null)), Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(1, 3, 0, 'U', 'T', 'C'), - new TimestampMetadata(TimeUnit.Microseconds, ZoneId.of("UTC"))), + new TimestampMetadata(TimeUnit.Microseconds, "UTC")), // trailing bytes past the declared timezone are ignored, as Rust slices to tz_len Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(1, 3, 0, 'U', 'T', 'C', 0), - new TimestampMetadata(TimeUnit.Microseconds, ZoneId.of("UTC"))), + new TimestampMetadata(TimeUnit.Microseconds, "UTC")), + // a zone this JDK does not know still opens, kept as stored, as Rust keeps it + Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 3, 0, 'X', '/', 'Y'), + new TimestampMetadata(TimeUnit.Milliseconds, "X/Y")), Arguments.of(ExtensionId.VORTEX_UUID, bytes(4), new OpaqueMetadata(bytes(4))), Arguments.of(ExtensionId.parse("acme.geopoint"), bytes(1, 2, 3), new OpaqueMetadata(bytes(1, 2, 3)))); } @@ -43,7 +45,6 @@ static Stream invalid() { Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2), "at least 3 bytes"), Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 5, 0, 'U'), "truncated"), Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 1, 0, 0xc3), "not valid UTF-8"), - Arguments.of(ExtensionId.VORTEX_TIMESTAMP, bytes(2, 3, 0, 'X', '/', 'Y'), "not a valid zone id"), Arguments.of(ExtensionId.parse("acme.big"), MemorySegment.ofArray(new byte[(int) OpaqueMetadata.MAX_SIZE.bytes() + 1]), "metadata too large")); } diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/DTypeFbsSerializer.java b/writer/src/main/java/io/github/dfa1/vortex/writer/DTypeFbsSerializer.java index 60830f96b..4eef4f951 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/DTypeFbsSerializer.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/DTypeFbsSerializer.java @@ -30,13 +30,17 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; -import java.time.ZoneId; +import java.util.IdentityHashMap; +import java.util.Map; /// Serializes a [DType] tree to its FlatBuffer wire encoding (the file's DType blob, pointed to /// by the postscript). final class DTypeFbsSerializer { private final WriteRegistry extensions; + /// Each extension node's bytes, so the check at `VortexWriter.create` and the write at close + /// serialize a third-party record once, and the bytes written are the bytes checked. + private final Map serialized = new IdentityHashMap<>(); /// @param extensions the extension encoders, which serialize metadata of their own types DTypeFbsSerializer(WriteRegistry extensions) { @@ -55,34 +59,54 @@ ByteBuffer buildDType(DType dtype) { } /// Serializes an extension's metadata, as Rust's `serialize_metadata`: the inverse of the - /// reader's `ExtensionMetadataParser`. The built-in records and [OpaqueMetadata] are written - /// here; any other record by the extension's registered encoder. + /// reader's `ExtensionMetadataParser`. [OpaqueMetadata], and the built-in records under their + /// own ids, are written here; any other metadata by the id's registered encoder, so a custom id + /// using a built-in record reads back through its own decoder. /// /// @param ext an extension dtype /// @return the metadata's wire bytes - /// @throws VortexException if the metadata is a record no registered encoder serializes, or a - /// timezone longer than the wire's u16 length + /// @throws VortexException if no registered encoder serializes the metadata, the encoder + /// returns `null`, the bytes exceed [OpaqueMetadata#MAX_SIZE] (the reader would refuse + /// them), or a timezone is longer than the wire's u16 length byte[] metadataBytes(DType.Extension ext) { - return switch (ext.metadata()) { - case TimeUnitMetadata(TimeUnit unit) -> new byte[]{(byte) unit.ordinal()}; - case TimestampMetadata(TimeUnit unit, ZoneId zone) -> timestampBytes(unit, zone); - case OpaqueMetadata(MemorySegment bytes) -> bytes.toArray(ValueLayout.JAVA_BYTE); - case ExtensionMetadata other -> { - ExtensionEncoder encoder = extensions.lookup(ExtensionId.parse(ext.extensionId())); - if (encoder == null) { - throw new VortexException("no extension encoder registered for " + ext.extensionId() - + " to serialize its metadata of type " + other.getClass().getName()); - } - yield encoder.serializeMetadata(other); - } + return serialized.computeIfAbsent(ext, this::serialize); + } + + private byte[] serialize(DType.Extension ext) { + ExtensionId id = ExtensionId.parse(ext.extensionId()); + byte[] bytes = switch (ext.metadata()) { + case OpaqueMetadata(MemorySegment opaque) -> opaque.toArray(ValueLayout.JAVA_BYTE); + case TimeUnitMetadata(TimeUnit unit) when id == ExtensionId.VORTEX_DATE || id == ExtensionId.VORTEX_TIME -> + new byte[]{(byte) unit.ordinal()}; + case TimestampMetadata(TimeUnit unit, String timezone) when id == ExtensionId.VORTEX_TIMESTAMP -> + timestampBytes(unit, timezone); + case ExtensionMetadata other -> encoderBytes(ext, id, other); }; + if (bytes.length > OpaqueMetadata.MAX_SIZE.bytes()) { + throw new VortexException(ext.extensionId() + " metadata serializes to " + bytes.length + + " bytes, over the " + OpaqueMetadata.MAX_SIZE.bytes() + " a reader accepts"); + } + return bytes; + } + + private byte[] encoderBytes(DType.Extension ext, ExtensionId id, ExtensionMetadata metadata) { + ExtensionEncoder encoder = extensions.lookup(id); + if (encoder == null) { + throw new VortexException("no extension encoder registered for " + ext.extensionId() + + " to serialize its metadata of type " + metadata.getClass().getName()); + } + byte[] bytes = encoder.serializeMetadata(metadata); + if (bytes == null) { + throw new VortexException(ext.extensionId() + " encoder serialized its metadata to null"); + } + return bytes; } /// `byte[0] = TimeUnit tag, bytes[1..3] = tz_len (u16 LE), bytes[3..3+tz_len] = tz UTF-8`. - private static byte[] timestampBytes(TimeUnit unit, ZoneId zone) { - byte[] tz = zone == null ? new byte[0] : zone.getId().getBytes(StandardCharsets.UTF_8); + private static byte[] timestampBytes(TimeUnit unit, String timezone) { + byte[] tz = timezone == null ? new byte[0] : timezone.getBytes(StandardCharsets.UTF_8); if (tz.length > 0xFFFF) { - throw new VortexException("timestamp timezone longer than 65535 bytes: " + zone); + throw new VortexException("timestamp timezone longer than 65535 bytes: " + timezone); } byte[] out = new byte[3 + tz.length]; out[0] = (byte) unit.ordinal(); diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/ExtensionEncoder.java b/writer/src/main/java/io/github/dfa1/vortex/writer/ExtensionEncoder.java index a402a11fd..0cb55e63b 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/ExtensionEncoder.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/ExtensionEncoder.java @@ -41,9 +41,10 @@ public interface ExtensionEncoder { /// Serializes this extension's metadata to the bytes a file stores, Rust's `serialize_metadata`: /// the inverse of the reader's `ExtensionDecoder.parseMetadata`. Called for metadata the writer /// does not serialize itself, which is anything but - /// [io.github.dfa1.vortex.core.model.TimeUnitMetadata], - /// [io.github.dfa1.vortex.core.model.TimestampMetadata] and - /// [io.github.dfa1.vortex.core.model.OpaqueMetadata]. + /// [io.github.dfa1.vortex.core.model.OpaqueMetadata] and the built-in temporal records under + /// their own well-known ids: a custom id's metadata, even a built-in record, comes here, so it + /// reads back through the id's own decoder. The bytes may be at most 64 KiB, what a reader + /// accepts. /// /// The default refuses it. /// diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java b/writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java index 921fd69c4..a2db007d7 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java @@ -132,6 +132,8 @@ public final class VortexWriter implements Closeable { private final WriteRegistry defaultRegistry; // Extension encoders for the Collection auto-route: the built-ins, or the caller's registry. private final WriteRegistry extensionRegistry; + /// Serializes the schema at close, reusing the extension metadata bytes checked at create. + private final DTypeFbsSerializer dtypeSerializer; private final List cascadeCodecs; private final WriteRegistry cascadeRegistry; // The dict-layout probe's codecs: the cascade with the integer dictionary back in, as Rust's @@ -226,6 +228,7 @@ private VortexWriter( this.encodings = encodings; this.defaultRegistry = buildRegistry(encodings); this.extensionRegistry = extensionRegistry; + this.dtypeSerializer = new DTypeFbsSerializer(extensionRegistry); validateExtensions(schema); // A custom encoder list is the whole candidate set, cascade included: emitting a built-in // the caller left out would produce a file their own read registry may not decode. @@ -321,7 +324,7 @@ private void validateExtensions(DType dtype) { // written from its storage array; still held to Rust's rules for the id wellKnown.validate(ext); } - new DTypeFbsSerializer(extensionRegistry).metadataBytes(ext); + dtypeSerializer.metadataBytes(ext); validateExtensions(ext.storageDType()); } case DType.Struct struct -> struct.fieldTypes().forEach(this::validateExtensions); @@ -763,7 +766,7 @@ public void close() throws IOException { long footerOff = bytesWritten; write(footerBuf); - ByteBuffer dtypeBuf = new DTypeFbsSerializer(extensionRegistry).buildDType(schema); + ByteBuffer dtypeBuf = dtypeSerializer.buildDType(schema); long dtypeOff = bytesWritten; write(dtypeBuf); diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/DTypeFbsSerializerTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/DTypeFbsSerializerTest.java new file mode 100644 index 000000000..39c0383e0 --- /dev/null +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/DTypeFbsSerializerTest.java @@ -0,0 +1,141 @@ +package io.github.dfa1.vortex.writer; + +import io.github.dfa1.vortex.core.error.VortexException; +import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.core.model.DTypes; +import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.ExtensionMetadata; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; +import io.github.dfa1.vortex.core.model.TimeUnit; +import io.github.dfa1.vortex.core.model.TimeUnitMetadata; +import io.github.dfa1.vortex.core.model.TimestampMetadata; +import org.junit.jupiter.api.Test; + +import java.lang.foreign.MemorySegment; +import java.nio.charset.StandardCharsets; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.then; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; + +/// The writer half of Rust's `serialize_metadata`: what `DTypeFbsSerializer#metadataBytes` writes +/// for each kind of extension metadata, and what it refuses before a file is written. +class DTypeFbsSerializerTest { + + private static final ExtensionId GEOPOINT = ExtensionId.parse("acme.geopoint"); + + /// A third-party extension's typed metadata. + private record Srid(int srid) implements ExtensionMetadata { + } + + @Test + void builtInRecords_underTheirOwnIds_areWrittenAsTheReaderParsesThem() { + // Given — no encoders: the built-ins need none + DTypeFbsSerializer sut = new DTypeFbsSerializer(WriteRegistry.builder().build()); + + // When + byte[] date = sut.metadataBytes(extension("vortex.date", new TimeUnitMetadata(TimeUnit.Days))); + byte[] timestamp = sut.metadataBytes(extension("vortex.timestamp", + new TimestampMetadata(TimeUnit.Milliseconds, "UTC"))); + + // Then — the unit tag; then for a timestamp the u16 LE zone length and the zone as stored + assertThat(date).containsExactly(4); + assertThat(timestamp).containsExactly(2, 3, 0, 'U', 'T', 'C'); + } + + @Test + void builtInRecord_underACustomId_goesThroughThatIdsEncoder() { + // Given — a custom id reusing TimeUnitMetadata: written by the built-in path it would read + // back as opaque bytes, which its own decoder could not tell from any other blob + ExtensionEncoder encoder = encoder(GEOPOINT); + given(encoder.serializeMetadata(any())).willReturn(new byte[]{7}); + DTypeFbsSerializer sut = new DTypeFbsSerializer(WriteRegistry.builder().register(encoder).build()); + + // When + byte[] result = sut.metadataBytes(extension(GEOPOINT.id(), new TimeUnitMetadata(TimeUnit.Seconds))); + + // Then + assertThat(result).containsExactly(7); + } + + @Test + void encoderBytes_areSerializedOncePerExtension() { + // Given — the writer checks the metadata at create and writes it at close + ExtensionEncoder encoder = encoder(GEOPOINT); + given(encoder.serializeMetadata(any())).willReturn(new byte[]{1}); + DTypeFbsSerializer sut = new DTypeFbsSerializer(WriteRegistry.builder().register(encoder).build()); + DType.Extension ext = extension(GEOPOINT.id(), new Srid(4326)); + + // When + sut.metadataBytes(ext); + sut.metadataBytes(ext); + + // Then — a third-party serializer runs once, so the bytes written are the bytes checked + then(encoder).should(times(1)).serializeMetadata(any()); + } + + @Test + void oversizedMetadata_isRefused() { + // Given — bytes the reader's 64 KiB cap would refuse: the writer must not produce a file + // its own reader cannot open + ExtensionEncoder encoder = encoder(GEOPOINT); + given(encoder.serializeMetadata(any())).willReturn(new byte[(int) OpaqueMetadata.MAX_SIZE.bytes() + 1]); + DTypeFbsSerializer sut = new DTypeFbsSerializer(WriteRegistry.builder().register(encoder).build()); + + // When / Then + assertThatThrownBy(() -> sut.metadataBytes(extension(GEOPOINT.id(), new Srid(1)))) + .isInstanceOf(VortexException.class) + .hasMessageContaining("acme.geopoint metadata serializes to 65537 bytes"); + } + + @Test + void encoderReturningNull_isRefusedNamingTheId() { + // Given + ExtensionEncoder encoder = encoder(GEOPOINT); + given(encoder.serializeMetadata(any())).willReturn(null); + DTypeFbsSerializer sut = new DTypeFbsSerializer(WriteRegistry.builder().register(encoder).build()); + + // When / Then — a VortexException naming the extension, not an NPE inside the FlatBuffer builder + assertThatThrownBy(() -> sut.metadataBytes(extension(GEOPOINT.id(), new Srid(1)))) + .isInstanceOf(VortexException.class) + .hasMessageContaining("acme.geopoint encoder serialized its metadata to null"); + } + + @Test + void customRecordWithoutEncoder_isRefused() { + // Given + DTypeFbsSerializer sut = new DTypeFbsSerializer(WriteRegistry.builder().build()); + + // When / Then + assertThatThrownBy(() -> sut.metadataBytes(extension(GEOPOINT.id(), new Srid(1)))) + .isInstanceOf(VortexException.class) + .hasMessageContaining("no extension encoder registered for acme.geopoint"); + } + + @Test + void opaqueMetadata_isWrittenAsItsBytes() { + // Given — a foreign id's bytes, kept from the file it was read from + DTypeFbsSerializer sut = new DTypeFbsSerializer(WriteRegistry.builder().build()); + byte[] bytes = "crs".getBytes(StandardCharsets.US_ASCII); + + // When + byte[] result = sut.metadataBytes(extension("vortex.st.wkb", new OpaqueMetadata(MemorySegment.ofArray(bytes)))); + + // Then + assertThat(result).isEqualTo(bytes); + } + + private static DType.Extension extension(String id, ExtensionMetadata metadata) { + return new DType.Extension(id, DTypes.I64, metadata, false); + } + + private static ExtensionEncoder encoder(ExtensionId id) { + ExtensionEncoder encoder = mock(ExtensionEncoder.class); + given(encoder.extensionId()).willReturn(id); + return encoder; + } +} From ba95e850b90549db484228555c916546333aeece Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sun, 1 Nov 2026 14:04:32 +0100 Subject: [PATCH 5/6] test: a java.util.Currency extension example next to the vector one acme.currency stores one Currency per row as its ISO 4217 code over Utf8, with no metadata: string storage where the vector example has non-primitive storage and typed metadata, and a decoder that refuses a code that is not ISO 4217. The how-to links it beside the vector test. Co-Authored-By: Claude Sonnet 5 --- docs/how-to.md | 5 +- .../CurrencyExtensionIntegrationTest.java | 301 ++++++++++++++++++ 2 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 integration/src/test/java/io/github/dfa1/vortex/integration/CurrencyExtensionIntegrationTest.java diff --git a/docs/how-to.md b/docs/how-to.md index 58120b453..fe183cae4 100644 --- a/docs/how-to.md +++ b/docs/how-to.md @@ -1154,7 +1154,10 @@ The complete, tested example is an `example.vector` embedding type with the same pieces as this recipe (a typed metadata record, `serializeMetadata`, `parseMetadata`, `validate` on both sides), written and read back through full, filtered and limited scans, plus the unknown-id and validation failures. The recipe and the test -change together. +change together. A second one, +[`CurrencyExtensionIntegrationTest`](../integration/src/test/java/io/github/dfa1/vortex/integration/CurrencyExtensionIntegrationTest.java), +stores one `java.util.Currency` per row as its ISO 4217 code over `Utf8`, with no metadata, and +refuses a code that is not ISO 4217 when it decodes. --- diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/CurrencyExtensionIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/CurrencyExtensionIntegrationTest.java new file mode 100644 index 000000000..3f52b8f34 --- /dev/null +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/CurrencyExtensionIntegrationTest.java @@ -0,0 +1,301 @@ +package io.github.dfa1.vortex.integration; + +import dev.vortex.api.DataSource; +import dev.vortex.api.Session; +import dev.vortex.jni.NativeLoader; +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.DTypes; +import io.github.dfa1.vortex.core.model.ExtensionId; +import io.github.dfa1.vortex.core.model.OpaqueMetadata; +import io.github.dfa1.vortex.reader.Chunk; +import io.github.dfa1.vortex.reader.ReadRegistry; +import io.github.dfa1.vortex.reader.ScanFilter; +import io.github.dfa1.vortex.reader.ScanOptions; +import io.github.dfa1.vortex.reader.VortexReader; +import io.github.dfa1.vortex.reader.array.Array; +import io.github.dfa1.vortex.reader.array.MaskedArray; +import io.github.dfa1.vortex.reader.array.VarBinArray; +import io.github.dfa1.vortex.reader.extension.ExtensionDecoder; +import io.github.dfa1.vortex.writer.ExtensionEncoder; +import io.github.dfa1.vortex.writer.VortexWriter; +import io.github.dfa1.vortex.writer.WriteOptions; +import io.github.dfa1.vortex.writer.WriteRegistry; +import io.github.dfa1.vortex.writer.encode.NullableData; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.channels.FileChannel; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Currency; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/// A second third-party extension type, next to [CustomExtensionIntegrationTest]'s vector: an +/// `acme.currency` column with one [Currency] per row, stored as its ISO 4217 code over `Utf8`. +/// Where the vector shows non-primitive storage and typed metadata, this one shows string storage +/// and no metadata ([OpaqueMetadata#EMPTY]), and a decoder that is strict about the values it maps: +/// a code that is not ISO 4217 is refused, not read back as a string. +/// +/// `java.util.Currency` suits a row value: it is immutable, `Currency.getInstance` returns one +/// cached instance per code, and an unknown code fails fast. +class CurrencyExtensionIntegrationTest { + + static { + NativeLoader.loadJni(); + } + + private static final Session SESSION = Session.create(); + private static final ExtensionId CURRENCY = ExtensionId.parse("acme.currency"); + private static final ColumnName ID = ColumnName.of("id"); + private static final ColumnName PRICE_CURRENCY = ColumnName.of("price_currency"); + private static final List CURRENCIES = List.of( + Currency.getInstance("EUR"), Currency.getInstance("USD"), Currency.getInstance("JPY"), + Currency.getInstance("CHF")); + private static final int ROWS = 1_000; + + private static final WriteRegistry WRITE_REGISTRY = WriteRegistry.builder() + .registerDefaults() + .register(new CurrencyEncoder()) + .build(); + private static final ReadRegistry READ_REGISTRY = ReadRegistry.builder() + .registerDefaults() + .register(new CurrencyDecoder()) + .build(); + + @Test + void fullScan_decodesEveryCurrencyAndNull(@TempDir Path tmp) throws IOException { + // Given — Currency values written from a Collection, every eleventh row null + Path file = tmp.resolve("currencies.vortex"); + writeCurrencies(file); + + // When + List result = readCurrencies(file, ScanOptions.all()); + + // Then — the same instances, as Currency.getInstance caches one per code + assertThat(result).isEqualTo(expected(0)); + } + + @Test + void filteredScan_keepsEachCurrencyWithItsRow(@TempDir Path tmp) throws IOException { + // Given — a filter on the sibling id column: the scan returns only the matching rows, and + // each currency must stay next to its own row + Path file = tmp.resolve("currencies.vortex"); + writeCurrencies(file); + long threshold = ROWS / 2; + + // When + List result = readCurrencies(file, ScanOptions.all().withFilter(ScanFilter.gte(ID, threshold))); + + // Then + assertThat(result).isEqualTo(expected(threshold)); + } + + @Test + void codeThatIsNotIso4217_isRefusedOnDecode(@TempDir Path tmp) throws IOException { + // Given — a file from a writer without the encoder, its storage written as plain strings + Path file = tmp.resolve("bad.vortex"); + try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE); + var writer = VortexWriter.create(ch, schema(false), WriteOptions.defaults())) { + writer.writeChunk(Map.of(ID, new long[]{0L, 1L}, PRICE_CURRENCY, new String[]{"EUR", "ABC"})); + } + + try (VortexReader reader = VortexReader.open(file, READ_REGISTRY); + var iter = reader.scan(ScanOptions.all()); + Chunk chunk = iter.next()) { + // When / Then + assertThatThrownBy(() -> chunk.as(PRICE_CURRENCY, Currency.class)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("row 1: ABC is not an ISO 4217 currency code"); + } + } + + @Test + void allowUnknown_readsColumnAsItsCodes(@TempDir Path tmp) throws IOException { + // Given — without the decoder the column reads as its Utf8 storage, Rust's ForeignExtDType + Path file = tmp.resolve("currencies.vortex"); + writeCurrencies(file); + ReadRegistry allowUnknown = ReadRegistry.builder().registerDefaults().allowUnknown().build(); + + // When + String firstCode; + try (VortexReader reader = VortexReader.open(file, allowUnknown); + var iter = reader.scan(ScanOptions.all()); + Chunk chunk = iter.next()) { + VarBinArray codes = (VarBinArray) MaskedArray.innerOrSelf(chunk.column(PRICE_CURRENCY)); + firstCode = codes.getString(1); + } + + // Then + assertThat(firstCode).isEqualTo(currency(1).getCurrencyCode()); + } + + @Test + void vortexJni_withoutVtable_refusesTheFile(@TempDir Path tmp) throws IOException { + // Given — the Rust reference has no acme.currency vtable and its JNI session exposes no + // allow_unknown: the behavior the strict Java default mirrors + Path file = tmp.resolve("currencies.vortex"); + writeCurrencies(file); + String uri = file.toAbsolutePath().toUri().toString(); + + // When / Then + assertThatThrownBy(() -> DataSource.open(SESSION, uri).scan(dev.vortex.api.ScanOptions.of())) + .hasMessageContaining("No such DType extension ID: acme.currency"); + } + + // ── the custom extension type ──────────────────────────────────────────── + + /// The `acme.currency` dtype: `Utf8` storage of ISO 4217 codes, no metadata. + static DType.Extension currencyDtype(boolean nullable) { + return new DType.Extension(CURRENCY.id(), nullable ? DTypes.UTF8.asNullable() : DTypes.UTF8, + OpaqueMetadata.EMPTY, nullable); + } + + /// Shared by encoder and decoder, as one Rust `ExtVTable::validate_dtype` serves both directions. + static void validateCurrency(DType.Extension dtype) { + if (!(dtype.storageDType() instanceof DType.Utf8)) { + throw new VortexException("acme.currency storage must be Utf8, got " + dtype.storageDType()); + } + } + + static final class CurrencyEncoder implements ExtensionEncoder { + + @Override + public ExtensionId extensionId() { + return CURRENCY; + } + + @Override + public DType.Extension dtype(boolean nullable) { + return currencyDtype(nullable); + } + + @Override + public void validate(DType.Extension dtype) { + validateCurrency(dtype); + } + + @Override + public Object encodeAll(DType.Extension dtype, Collection values) { + String[] codes = new String[values.size()]; + boolean[] validity = new boolean[codes.length]; + boolean anyNull = false; + int row = 0; + for (Object value : values) { + if (value == null) { + anyNull = true; + codes[row] = ""; + } else { + codes[row] = ((Currency) value).getCurrencyCode(); + validity[row] = true; + } + row++; + } + if (!anyNull) { + return codes; + } + if (!dtype.nullable()) { + throw new VortexException("null element in non-nullable acme.currency column"); + } + return new NullableData(codes, validity); + } + } + + static final class CurrencyDecoder implements ExtensionDecoder { + + @Override + public ExtensionId extensionId() { + return CURRENCY; + } + + @Override + public DType.Extension dtype(boolean nullable) { + return currencyDtype(nullable); + } + + @Override + public Class domainType() { + return Currency.class; + } + + @Override + public void validate(DType.Extension dtype) { + validateCurrency(dtype); + } + + @Override + public List decodeAll(DType.Extension dtype, Array storage) { + MaskedArray.Unwrapped unwrapped = MaskedArray.unwrap(storage); + VarBinArray codes = (VarBinArray) unwrapped.inner(); + int n = Math.toIntExact(codes.length()); + List out = new ArrayList<>(n); + for (int row = 0; row < n; row++) { + if (unwrapped.validity() != null && !unwrapped.validity().getBoolean(row)) { + out.add(null); + continue; + } + String code = codes.getString(row); + try { + out.add(Currency.getInstance(code)); + } catch (IllegalArgumentException e) { + throw new VortexException("row " + row + ": " + code + " is not an ISO 4217 currency code", e); + } + } + return out; + } + } + + // ── helpers ────────────────────────────────────────────────────────────── + + private static DType.Struct schema(boolean nullable) { + return new DType.Struct(List.of(ID, PRICE_CURRENCY), List.of(DTypes.I64, currencyDtype(nullable)), false); + } + + /// Every eleventh row is null; the rest cycle through four currencies. + private static Currency currency(long row) { + return row % 11 == 5 ? null : CURRENCIES.get((int) (row % CURRENCIES.size())); + } + + private static List expected(long fromRow) { + List out = new ArrayList<>(); + for (long row = fromRow; row < ROWS; row++) { + out.add(currency(row)); + } + return out; + } + + private static void writeCurrencies(Path file) throws IOException { + long[] ids = new long[ROWS]; + List values = new ArrayList<>(ROWS); + for (int row = 0; row < ROWS; row++) { + ids[row] = row; + values.add(currency(row)); + } + try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE); + var writer = VortexWriter.create(ch, schema(true), WriteOptions.defaults(), WRITE_REGISTRY)) { + writer.writeChunk(Map.of(ID, ids, PRICE_CURRENCY, values)); + } + } + + /// The decoded currency column, in scan order. + private static List readCurrencies(Path file, ScanOptions options) throws IOException { + List out = new ArrayList<>(); + try (VortexReader reader = VortexReader.open(file, READ_REGISTRY); + var iter = reader.scan(options)) { + while (iter.hasNext()) { + try (Chunk chunk = iter.next()) { + out.addAll(chunk.as(PRICE_CURRENCY, Currency.class)); + } + } + } + return out; + } +} From fcf4f0e3c0e04c5404a3fbc23ec5cef1f2f314e7 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sun, 1 Nov 2026 14:12:35 +0100 Subject: [PATCH 6/6] fix(core): OpaqueMetadata prints a bounded prefix and hashes in place toString hex-dumped the whole value, up to the 64 KiB cap: a 128 K-character string that ends up in exception messages ("metadata must be TimeUnitMetadata, got ...") built from an untrusted file. It now shows the size and at most the first 16 bytes. hashCode copied the bytes out on every call; it reads them in place, same hash as Arrays.hashCode. Co-Authored-By: Claude Sonnet 5 --- .../vortex/core/model/OpaqueMetadata.java | 18 ++++++++-- .../vortex/core/model/DTypeExtensionTest.java | 36 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java index 5934f04c7..f13e5238a 100644 --- a/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java @@ -5,7 +5,6 @@ import java.lang.foreign.MemorySegment; import java.lang.foreign.ValueLayout; -import java.util.Arrays; import java.util.HexFormat; /// Extension metadata kept as the bytes a file stores, for an id nothing parses: Rust's @@ -31,6 +30,9 @@ public record OpaqueMetadata(MemorySegment bytes) implements ExtensionMetadata { /// No metadata. public static final OpaqueMetadata EMPTY = new OpaqueMetadata(MemorySegment.ofArray(new byte[0])); + /// How many bytes [#toString()] shows. + private static final int TO_STRING_BYTES = 16; + /// Bounds the bytes, which may arrive from an untrusted file, and keeps them read-only. /// /// @param bytes the wire bytes @@ -52,13 +54,23 @@ public boolean equals(Object o) { && bytes.byteSize() == other.byteSize() && bytes.mismatch(other) == -1; } + /// The `Arrays.hashCode` of the bytes, read in place rather than copied out per call. @Override public int hashCode() { - return Arrays.hashCode(bytes.toArray(ValueLayout.JAVA_BYTE)); + int hash = 1; + for (long i = 0; i < bytes.byteSize(); i++) { + hash = 31 * hash + bytes.get(ValueLayout.JAVA_BYTE, i); + } + return hash; } + /// The size and at most the first [#TO_STRING_BYTES] bytes in hex: the bytes can be 64 KiB and + /// this string ends up in exception messages. @Override public String toString() { - return "OpaqueMetadata[" + HexFormat.of().formatHex(bytes.toArray(ValueLayout.JAVA_BYTE)) + "]"; + long shown = Math.min(bytes.byteSize(), TO_STRING_BYTES); + String hex = HexFormat.of().formatHex(bytes.asSlice(0, shown).toArray(ValueLayout.JAVA_BYTE)); + return "OpaqueMetadata[" + bytes.byteSize() + " bytes: " + hex + + (shown < bytes.byteSize() ? "…" : "") + "]"; } } diff --git a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java index de083ff1a..49efd3162 100644 --- a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java +++ b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeExtensionTest.java @@ -115,6 +115,42 @@ void callerArray_changedLater_doesNotChangeTheValue() { assertThat(sut.hashCode()).isEqualTo(hash); } + @Test + void toString_ofLargeMetadata_showsSizeAndAPrefixOnly() { + // Given metadata at the 64 KiB cap, which ends up in exception messages via toString + OpaqueMetadata sut = new OpaqueMetadata(MemorySegment.ofArray(new byte[(int) OpaqueMetadata.MAX_SIZE.bytes()])); + + // When + String result = sut.toString(); + + // Then — not a 128 K-character hex dump + assertThat(result).isEqualTo("OpaqueMetadata[65536 bytes: 00000000000000000000000000000000…]"); + } + + @Test + void toString_ofSmallMetadata_showsEveryByte() { + // Given + OpaqueMetadata sut = new OpaqueMetadata(MemorySegment.ofArray(new byte[]{'C', 'R', 'S'})); + + // When + String result = sut.toString(); + + // Then + assertThat(result).isEqualTo("OpaqueMetadata[3 bytes: 435253]"); + } + + @Test + void hashCode_isTheArraysHashOfTheBytes() { + // Given — read in place rather than copied out, it must still match the array hash + byte[] bytes = {1, -2, 3, 127, -128}; + + // When + int result = new OpaqueMetadata(MemorySegment.ofArray(bytes)).hashCode(); + + // Then + assertThat(result).isEqualTo(java.util.Arrays.hashCode(bytes)); + } + @Test void bytes_areReadOnly() { // Given