From 08055d30bcedb9b4f77010b155246f2b81297e03 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Mon, 19 Oct 2026 07:08:09 +0200 Subject: [PATCH 1/3] fix(core): refuse a decimal dtype Rust refuses, on write as on read Rust's DecimalDType::try_new wants a precision in 1..=76 and a scale no greater than it. Only the FlatBuffer dtype parser checked that, so DType.decimal(80, 0) or (10, 12) wrote a file that vortex-jni and our own reader both refuse, and a vortex.variant decimal (parsed from proto) was never checked, with its u32/i32 fields truncated by a (byte) cast where Rust's try_into errors (300 became the valid precision 44). The rule moves into DType.Decimal itself, as DType.Map already does for its key: the compact constructor checks the byte components and Decimal.of checks wide values before narrowing. It throws VortexException, because the file parsers construct through it. The parser's own copy is gone. Breaking: such a dtype used to be accepted. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WtuVGQ5h5Lk6Zn4JnkxF1K --- CHANGELOG.md | 1 + .../github/dfa1/vortex/core/model/DType.java | 39 +++++++++++- .../vortex/core/model/DTypeDecimalTest.java | 60 +++++++++++++++++++ .../dfa1/vortex/reader/PostscriptParser.java | 17 +----- .../reader/decode/VariantEncodingDecoder.java | 6 +- 5 files changed, 104 insertions(+), 19 deletions(-) create mode 100644 core/src/test/java/io/github/dfa1/vortex/core/model/DTypeDecimalTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 820a1989d..0289f57e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking:** `ExtensionId` is a sealed `WellKnown`/`Custom` type like `EncodingId`, so third-party extension ids can be named, and `ExtensionId.parse` returns the id instead of an `Optional`. ([#530](https://github.com/dfa1/vortex-java/issues/530)) ### Fixed +- **Breaking:** `DType.Decimal` refuses a precision outside 1 to 76 or a scale above the precision, as Rust does, so the writer no longer writes decimal columns that Rust and vortex-java cannot read, and a variant's decimal is checked as it is read. ([#571](https://github.com/dfa1/vortex-java/pull/571)) - SQL `ORDER BY ... LIMIT` and `GROUP BY` with a few thousand result cells no longer fail with "Code grows beyond 64 KB"; past the limit Calcite scans instead. ([#569](https://github.com/dfa1/vortex-java/pull/569)) - A file whose array FlatBuffer holds a corrupt offset fails the read with `VortexException` instead of `IndexOutOfBoundsException`. ([#493](https://github.com/dfa1/vortex-java/issues/493)) - A column whose dtype does not fit its bit-packed, delta or RLE encoding fails with `VortexException` instead of `ClassCastException`, and an empty RLE boolean chunk reads. ([#493](https://github.com/dfa1/vortex-java/issues/493)) 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 fe0e1d588..1a36a44a1 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 @@ -139,7 +139,7 @@ default DType withNullable(boolean nullable) { /// @param scale number of digits to the right of the decimal point /// @return non-nullable [Decimal] static Decimal decimal(int precision, int scale) { - return new Decimal((byte) precision, (byte) scale, false); + return Decimal.of(precision, scale, false); } /// Returns a fresh [StructBuilder] for assembling a [Struct] dtype with @@ -227,6 +227,43 @@ record Primitive(PType ptype, boolean nullable) implements DType { /// @param scale number of digits to the right of the decimal point /// @param nullable whether null values are permitted record Decimal(byte precision, byte scale, boolean nullable) implements DType { + + /// Rust's `DecimalDType::try_new` bounds. Runs both on programmatic construction, so the + /// writer never emits a dtype Rust refuses, and while parsing an untrusted file's DType + /// blob, so it raises [VortexException] rather than [IllegalArgumentException] — same + /// reasoning as [Map]. + /// + /// @throws VortexException if `precision` is outside `1..76` or `scale` exceeds it + public Decimal { + requireValid(precision, scale); + } + + /// Creates a decimal from wide values, rejecting rather than truncating one that does not + /// fit the record's `byte` components (Rust's `try_into` on the proto's `u32`/`i32`). + /// + /// @param precision total number of significant decimal digits, `1..76` + /// @param scale number of digits to the right of the decimal point, at most `precision` + /// @param nullable whether null values are permitted + /// @return the decimal dtype + /// @throws VortexException if `precision` is outside `1..76`, `scale` exceeds it or does + /// not fit in a byte + public static Decimal of(int precision, int scale, boolean nullable) { + requireValid(precision, scale); + if (scale < Byte.MIN_VALUE) { + throw new VortexException("decimal scale " + scale + " does not fit in i8"); + } + return new Decimal((byte) precision, (byte) scale, nullable); + } + + // A negative scale (e.g. -2 for hundreds) is legal. + private static void requireValid(int precision, int scale) { + if (precision < 1 || precision > 76) { + throw new VortexException("decimal precision " + precision + " out of range (expected 1..76)"); + } + if (scale > precision) { + throw new VortexException("decimal scale " + scale + " exceeds precision " + precision); + } + } } /// Variable-length UTF-8 string logical type. diff --git a/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeDecimalTest.java b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeDecimalTest.java new file mode 100644 index 000000000..81a64ac79 --- /dev/null +++ b/core/src/test/java/io/github/dfa1/vortex/core/model/DTypeDecimalTest.java @@ -0,0 +1,60 @@ +package io.github.dfa1.vortex.core.model; + +import io.github.dfa1.vortex.core.error.VortexException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/// Rust's `DecimalDType::try_new` bounds. Before they lived on the record, the writer accepted +/// `DType.decimal(80, 0)` or `(10, 12)` and wrote a file that vortex-jni and our own reader refuse. +class DTypeDecimalTest { + + @ParameterizedTest + @CsvSource({"1, 0", "76, 76", "10, -5", "5, -100", "38, 0"}) + void inRange_isAccepted(int precision, int scale) { + // Given / When + DType.Decimal result = DType.Decimal.of(precision, scale, true); + + // Then + assertThat(result.precision()).isEqualTo((byte) precision); + assertThat(result.scale()).isEqualTo((byte) scale); + assertThat(result.nullable()).isTrue(); + } + + @ParameterizedTest + @CsvSource({ + "0, 0", // Rust's NonZero precision + "77, 0", // above i256's 76 digits + "300, 2", // must not truncate to (byte) 44, a valid precision + "-1, 0", // a proto u32 above 2^31 reads as a negative int + "10, 12", // scale above precision + "10, -129", // must not truncate to (byte) 127 + }) + void outOfRange_throwsVortexException(int precision, int scale) { + // Given / When / Then: VortexException, since the file parsers construct through here + assertThatThrownBy(() -> DType.Decimal.of(precision, scale, false)) + .isInstanceOf(VortexException.class); + } + + @Test + void constructor_enforcesTheSameBounds() { + // Given a byte precision of 200 — negative as a byte, and never valid + byte precision = (byte) 200; + + // When / Then + assertThatThrownBy(() -> new DType.Decimal(precision, (byte) 0, false)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("out of range"); + } + + @Test + void factory_enforcesTheSameBounds() { + // Given / When / Then: the public decimal(int, int) shortcut the writer's users call + assertThatThrownBy(() -> DType.decimal(10, 12)) + .isInstanceOf(VortexException.class) + .hasMessageContaining("exceeds precision 10"); + } +} 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 bfb91ce3a..84f10bba1 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 @@ -265,21 +265,8 @@ private static DType convertDType(io.github.dfa1.vortex.core.fbs.FbsDType fbs) { } case FbsType.FbsDecimal -> { var d = fbs.type(new FbsDecimal()); - int precision = d.precision(); - int scale = d.scale(); - // Rust's DecimalDType::try_new bounds: precision 1..76 (i256), and a positive scale no - // greater than the precision; a negative scale (e.g. -2 for hundreds) is legal. Reject - // crafted values up front rather than letting a downstream BigDecimal/byte-width - // calculation fail with an unrelated exception. - if (precision < 1 || precision > 76) { - throw new VortexException( - "decimal precision " + precision + " out of range (expected 1..76)"); - } - if (scale > precision) { - throw new VortexException( - "decimal scale " + scale + " exceeds precision " + precision); - } - yield new DType.Decimal((byte) precision, (byte) scale, d.nullable()); + // DType.Decimal.of enforces Rust's DecimalDType::try_new bounds as VortexException + yield DType.Decimal.of(d.precision(), d.scale(), d.nullable()); } case FbsType.FbsUtf8 -> new DType.Utf8(fbs.type(new FbsUtf8()).nullable()); case FbsType.FbsBinary -> new DType.Binary(fbs.type(new FbsBinary()).nullable()); 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 4747e81dd..0deb31d35 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 @@ -85,9 +85,9 @@ static DType dtypeFromProto(io.github.dfa1.vortex.core.proto.ProtoDType proto) { proto.primitive().nullable()); } if (proto.decimal() != null) { - return new DType.Decimal( - (byte) proto.decimal().precision(), - (byte) proto.decimal().scale(), + return DType.Decimal.of( + proto.decimal().precision(), + proto.decimal().scale(), proto.decimal().nullable()); } if (proto.utf8() != null) { From edc88a807a5d3f7206ea5ca428d59e8452ff62a2 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Mon, 19 Oct 2026 07:08:20 +0200 Subject: [PATCH 2/3] fix(writer): take the frame-of-reference reference as an unsigned min Rust's FoRData::encode subtracts Stat::Min, an unsigned minimum for a u64 column. computeRef took a signed one for every ptype, so for u64 values on both sides of 2^63 it picked 2^63 (Long.MIN_VALUE) over a smaller value below it, leaving that value a full-width residual where Rust's are a few bits. The cascade's estimate already used the unsigned min (ArrayStats), so estimate and encode disagreed too. The files were readable (decode is a wrapping add); this is compression parity. Widened U8..U32 values are zero-extended, so the U64 order serves every unsigned width. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WtuVGQ5h5Lk6Zn4JnkxF1K --- CHANGELOG.md | 1 + .../encode/FrameOfReferenceEncodingEncoder.java | 11 +++++++---- .../FrameOfReferenceEncodingEncoderTest.java | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0289f57e0..77a29fadb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking:** `ExtensionId` is a sealed `WellKnown`/`Custom` type like `EncodingId`, so third-party extension ids can be named, and `ExtensionId.parse` returns the id instead of an `Optional`. ([#530](https://github.com/dfa1/vortex-java/issues/530)) ### Fixed +- Frame-of-reference encoding takes the unsigned minimum of a `u64` column whose values span 2^63 as its reference, as Rust does, instead of a reference that left those values at full width. ([#571](https://github.com/dfa1/vortex-java/pull/571)) - **Breaking:** `DType.Decimal` refuses a precision outside 1 to 76 or a scale above the precision, as Rust does, so the writer no longer writes decimal columns that Rust and vortex-java cannot read, and a variant's decimal is checked as it is read. ([#571](https://github.com/dfa1/vortex-java/pull/571)) - SQL `ORDER BY ... LIMIT` and `GROUP BY` with a few thousand result cells no longer fail with "Code grows beyond 64 KB"; past the limit Calcite scans instead. ([#569](https://github.com/dfa1/vortex-java/pull/569)) - A file whose array FlatBuffer holds a corrupt offset fails the read with `VortexException` instead of `IndexOutOfBoundsException`. ([#493](https://github.com/dfa1/vortex-java/issues/493)) diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/FrameOfReferenceEncodingEncoder.java b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/FrameOfReferenceEncodingEncoder.java index 00e391bb2..692c778df 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/FrameOfReferenceEncodingEncoder.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/FrameOfReferenceEncodingEncoder.java @@ -58,7 +58,7 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) { long[] longs = PrimitiveArrays.toLongs(data, ptype, EncodingId.FASTLANES_FOR); int n = longs.length; - long ref = computeRef(longs, n); + long ref = computeRef(longs, n, ptype); MemorySegment residuals = toResidualBuffer(longs, ref, ptype, ctx); MemorySegment meta = buildForMeta(ref, ptype); @@ -78,7 +78,7 @@ public CascadeStep encodeCascade(DType dtype, Object data, EncodeContext encodeC long[] longs = PrimitiveArrays.toLongs(data, ptype, EncodingId.FASTLANES_FOR); int n = longs.length; - long ref = computeRef(longs, n); + long ref = computeRef(longs, n, ptype); // Skip when ref == 0 and ptype is unsigned: residuals == input, so FOR adds metadata // overhead (ref scalar + extra node) for zero compression benefit over plain bitpack. // Matches Rust IntFoRScheme's skip estimate for this case. @@ -95,8 +95,11 @@ public CascadeStep encodeCascade(DType dtype, Object data, EncodeContext encodeC PrimitiveEncodingEncoder.minOf(stats), PrimitiveEncodingEncoder.maxOf(stats), true); } - private static long computeRef(long[] longs, int n) { - return n > 0 ? SimdOperationsSupport.preferred().minMax(longs, PType.I64)[0] : 0L; + // Rust takes the reference from Stat::Min, an unsigned min for unsigned ptypes: a signed one + // picks a u64 above 2^63 over a smaller value below it, so a column straddling 2^63 got full-width + // residuals. Widened U8..U32 values are zero-extended, so the U64 order serves every unsigned width. + private static long computeRef(long[] longs, int n, PType ptype) { + return n > 0 ? SimdOperationsSupport.preferred().minMax(longs, ptype.isUnsigned() ? PType.U64 : PType.I64)[0] : 0L; } private static MemorySegment buildForMeta(long ref, PType ptype) { diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/FrameOfReferenceEncodingEncoderTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/FrameOfReferenceEncodingEncoderTest.java index cc5667e6f..622d459fa 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/FrameOfReferenceEncodingEncoderTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/FrameOfReferenceEncodingEncoderTest.java @@ -265,6 +265,22 @@ void encode_unsignedU32_statsUseUnsignedField() throws IOException { assertThat(result.statsMax().uint64_value()).isEqualTo(300L); } + @Test + void encode_u64StraddlingTwoToThe63_referenceIsUnsignedMin() throws IOException { + // Given values just below and above 2^63: a signed min picks 2^63 (Long.MIN_VALUE), + // leaving 2^63 - 10 a full-width residual, where Rust's Stat::Min picks 2^63 - 10 + long below = Long.MAX_VALUE - 9; + long[] data = {Long.MIN_VALUE + 5, below, Long.MIN_VALUE}; + + // When + EncodeResult result = ENCODER.encode(new DType.Primitive(PType.U64, false), data, EncodeTestHelper.testCtx()); + + // Then + MemorySegment meta = result.rootNode().metadata(); + ProtoScalarValue reference = ProtoScalarValue.decode(meta, 0, meta.byteSize()); + assertThat(reference.uint64_value()).isEqualTo(below); + } + @Test void encodeCascade_signedI64_statsCarryMinAndMax() throws IOException { // Given — same #379 regression as encode_signedI64_statsCarryMinAndMax, but for the From 75eec2769666b26c6f1dbaf45921474d45cae8f2 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Mon, 19 Oct 2026 07:08:20 +0200 Subject: [PATCH 3/3] test(fuzz): decimal and binary columns in the write-read and vortex-jni targets The differential target found every writer interop bug of the fuzz round, but it wrote only ints, floats, bools and utf8. Decimals at each physical width Rust picks (i32, i64, i128) and arbitrary-byte binary columns now join them, with the same value shapes. 116,767 inputs over five minutes: Java and vortex-jni agree. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01WtuVGQ5h5Lk6Zn4JnkxF1K --- .../WriteReadFuzzIntegrationTest.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/WriteReadFuzzIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/WriteReadFuzzIntegrationTest.java index 8cb2a59ce..746f49aa8 100644 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/WriteReadFuzzIntegrationTest.java +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/WriteReadFuzzIntegrationTest.java @@ -3,10 +3,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import java.io.IOException; +import java.math.BigDecimal; +import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.ArrayList; +import java.util.HexFormat; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -30,6 +33,7 @@ import io.github.dfa1.vortex.reader.array.Array; import io.github.dfa1.vortex.reader.array.BoolArray; import io.github.dfa1.vortex.reader.array.ByteArray; +import io.github.dfa1.vortex.reader.array.DecimalArray; import io.github.dfa1.vortex.reader.array.DoubleArray; import io.github.dfa1.vortex.reader.array.FloatArray; import io.github.dfa1.vortex.reader.array.IntArray; @@ -67,13 +71,17 @@ @Tag("fuzz") class WriteReadFuzzIntegrationTest { + private static final HexFormat HEX = HexFormat.of(); + static { NativeLoader.loadJni(); } private static final List TYPES = List.of( DType.I8, DType.I16, DType.I32, DType.I64, DType.U8, DType.U16, DType.U32, DType.U64, - DType.F32, DType.F64, DType.BOOL, DType.UTF8); + DType.F32, DType.F64, DType.BOOL, DType.UTF8, DType.BINARY, + // One per decimal physical width Rust picks: i32, i64, i128 + DType.decimal(9, 2), DType.decimal(18, 4), DType.decimal(38, 0)); @TempDir Path tempDir; @@ -169,6 +177,10 @@ private static Object canonical(DType dtype, long raw) { return switch (dtype) { case DType.Bool _ -> (raw & 1) != 0; case DType.Utf8 _ -> raw % 97 == 0 ? "" : "row-" + Long.toUnsignedString(raw, 36); + // Arbitrary bytes, not UTF-8, of length 0..7: the raw's low bytes + case DType.Binary _ -> HEX.formatHex(ByteBuffer.allocate(8).putLong(raw).array(), 0, (int) (raw & 7)); + // The raw's low digits, so the shapes (runs, small ranges, constants) carry over + case DType.Decimal d -> BigDecimal.valueOf(raw % (long) Math.pow(10, Math.min(d.precision(), 18)), d.scale()); case DType.Primitive p -> switch (p.ptype()) { case I8 -> (long) (byte) raw; case I16 -> (long) (short) raw; @@ -200,6 +212,12 @@ private static Object writerValues(DType dtype, List canonical) { if (dtype instanceof DType.Utf8) { return canonical.toArray(new String[0]); } + if (dtype instanceof DType.Decimal) { + return canonical.toArray(new BigDecimal[0]); + } + if (dtype instanceof DType.Binary) { + return canonical.stream().map(v -> v == null ? null : HEX.parseHex((String) v)).toArray(byte[][]::new); + } boolean[] valid = new boolean[n]; long[] bits = new long[n]; boolean[] bools = new boolean[n]; @@ -316,6 +334,8 @@ private static void appendJava(Chunk chunk, Column column, List out) { private static Object javaValue(Array values, long i, DType dtype) { Object raw = switch (values) { + case DecimalArray a -> a.getDecimal(i); + case VarBinArray a when dtype instanceof DType.Binary -> HEX.formatHex(a.getBytes(i)); case LongArray a -> a.getLong(i); case IntArray a -> (long) a.getInt(i); case ShortArray a -> (long) a.getShort(i); @@ -357,6 +377,8 @@ private static Object jniValue(Object value, DType dtype) { case Float f -> Float.floatToIntBits(f); case Boolean b -> b; case Character c -> (long) c; + case BigDecimal d -> d; + case byte[] b -> HEX.formatHex(b); case Number n -> zeroExtend(dtype, n.longValue()); case org.apache.arrow.vector.util.Text text -> text.toString(); default -> value.toString();