diff --git a/AGENTS.md b/AGENTS.md index c60be724b..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), @@ -302,9 +303,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..89b3e563e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ 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` (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 - 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/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/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..8a6a8d556 --- /dev/null +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/ExtensionMetadata.java @@ -0,0 +1,14 @@ +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), 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 new file mode 100644 index 000000000..f13e5238a --- /dev/null +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/OpaqueMetadata.java @@ -0,0 +1,76 @@ +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; +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])); + + /// 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 + /// @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()); + } + // 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 + public boolean equals(Object o) { + return o instanceof OpaqueMetadata(MemorySegment other) + && 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() { + 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() { + 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/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..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 @@ -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,36 @@ 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, + TimestampMetadata.of(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] + /// @throws java.time.DateTimeException if the stored timezone is not a zone id this JDK knows 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 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..815c89f57 --- /dev/null +++ b/core/src/main/java/io/github/dfa1/vortex/core/model/TimestampMetadata.java @@ -0,0 +1,45 @@ +package io.github.dfa1.vortex.core.model; + +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. +/// +/// 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; an empty timezone is none, as on the wire. + /// + /// @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/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/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..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 @@ -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,151 @@ 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() { + }; + + // When / Then + assertThatThrownBy(() -> new DType.Extension("acme.custom", I32, identity, false)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must be a record"); + } - @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]); + @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]); + + // 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 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 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 + OpaqueMetadata sut = new OpaqueMetadata(MemorySegment.ofArray(new byte[]{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 + 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..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 @@ -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, TimestampMetadata.of(unit, zone), + storage.nullable()); } } } diff --git a/docs/compatibility.md b/docs/compatibility.md index 0826e9acd..78d2db66b 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -321,7 +321,11 @@ 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 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 (`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..fe183cae4 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++) { @@ -1141,6 +1149,16 @@ 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. 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. + --- ## Read files with unknown encodings diff --git a/docs/reference.md b/docs/reference.md index 8956c214f..54e1ce9ff 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 @@ -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) | @@ -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/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; + } +} 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..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 @@ -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; @@ -55,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 { @@ -107,13 +113,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 +161,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 +266,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 +298,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 +328,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); } - static MemorySegment dimensionMetadata(int dim) { + /// 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 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 +360,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 +386,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 +445,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/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 84f10bba1..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 @@ -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; @@ -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; @@ -163,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. @@ -339,10 +323,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 = ExtensionMetadataParser.parseId(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..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,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.decode.AlpEncodingDecoder; import io.github.dfa1.vortex.reader.decode.AlpRdEncodingDecoder; import io.github.dfa1.vortex.reader.decode.ArrayNode; @@ -51,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; @@ -142,59 +145,106 @@ 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) { + // 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 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) { - 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()); + yield resolved; + } + 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.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()); + 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.Union union -> union.variantTypes().forEach(this::validateExtensions); case DType.Null _, DType.Bool _, DType.Primitive _, DType.Decimal _, DType.Utf8 _, - DType.Binary _, DType.Variant _ -> { - // no nested dtype + 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 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"); + /// 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 ExtensionId.parse(raw); - } catch (IllegalArgumentException e) { - throw new VortexException("invalid extension id: " + e.getMessage()); + 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) { + 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..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; @@ -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..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 @@ -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,12 @@ 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 = ExtensionMetadataParser.parseId(proto.extension().id()); + 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..5462c4b09 --- /dev/null +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParser.java @@ -0,0 +1,103 @@ +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 static io.github.dfa1.vortex.core.io.VortexFormat.LE_SHORT; + +/// 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 { + + 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)); + } + 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`. 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 (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, timezone); + } catch (CharacterCodingException e) { + throw new VortexException(id + " timezone is not valid UTF-8", 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/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 c3ca2f72a..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 @@ -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,163 @@ 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_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 + 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..eaff12ba3 --- /dev/null +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/ExtensionMetadataParserTest.java @@ -0,0 +1,78 @@ +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.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, "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, "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)))); + } + + 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.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..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 @@ -16,30 +16,106 @@ 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.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 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) { + 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`. [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 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 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, 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: " + timezone); + } + 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 +123,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 +186,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..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 @@ -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,24 @@ 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.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. + /// + /// @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..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. @@ -307,6 +310,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 +324,7 @@ private void validateExtensions(DType dtype) { // written from its storage array; still held to Rust's rules for the id wellKnown.validate(ext); } + dtypeSerializer.metadataBytes(ext); validateExtensions(ext.storageDType()); } case DType.Struct struct -> struct.fieldTypes().forEach(this::validateExtensions); @@ -760,7 +766,7 @@ public void close() throws IOException { long footerOff = bytesWritten; write(footerBuf); - ByteBuffer dtypeBuf = DTypeFbsSerializer.buildDType(schema); + ByteBuffer dtypeBuf = dtypeSerializer.buildDType(schema); long dtypeOff = bytesWritten; write(dtypeBuf); 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/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/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/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; + } +} 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; 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),