Skip to content

Commit acbaa0b

Browse files
dfa1claude
andcommitted
fix(reader): zone-map pruning compares in the column's type domain (#159)
ScanIterator's comparator caught ClassCastException and returned 0, which canPruneChunk reads as "cannot prune". Stats decode integers as Long and floats as Float/Double, so a filter value boxed at a different width (Integer for I64, Float for F32) threw internally and silently disabled pruning — a valid, selective predicate degraded to a full scan with no signal. The comparison now keys off the *column* type, not the boxed operand: - floating column -> Double.compare; - unsigned int column -> Long.compareUnsigned (U64 stats/values store raw bits, so a value >= 2^63 is a negative Long; signed compare keeps/drops the wrong chunks). U8/U16/U32 zero-extend to a positive Long, unaffected; - signed int column -> Long.compare. Keying off the column also avoids routing an integer column through double-compare, which would lose precision past 2^53 and mis-prune. Eq/Neq previously had their own inline comparator with the same swallow; they now route through the shared one. A genuinely incomparable filter value (e.g. a String against a numeric column) now raises VortexException instead of a silent no-prune — a behaviour change, noted in the changelog. Adds DType.isUnsigned() (exhaustive over the sealed set) to classify the column. Coverage — ZoneMapPruningTest (27): BoxedWidth (Integer == Long, all six operators), Unsigned (U64 >= 2^63 keep/prune correctness), FloatWidths (F32 stat vs Double/Float filters), IntegerColumnFloatFilter (Double filter on I64 compares in the integer domain past 2^53), TypeMismatch (String throws). Plus DTypeIsUnsignedTest. Closes #159. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 354bb8a commit acbaa0b

5 files changed

Lines changed: 364 additions & 26 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ All notable changes to **vortex-java** are documented here.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [Unreleased]
9+
10+
### Added
11+
12+
- `DType.isUnsigned()``true` for the unsigned integer primitives (`U8``U64`), `false` otherwise. ([#159](https://github.com/dfa1/vortex-java/issues/159))
13+
14+
### Fixed
15+
16+
- Zone-map pruning now compares filter values in the *column's* type domain rather than by the boxed value's type. A predicate whose value is boxed at a different width (e.g. `Integer` on an `I64` column) — or any value on a `U64` column — previously pruned nothing and silently degraded to a full scan; it now prunes correctly (unsigned columns by unsigned order). As part of this, a filter value genuinely incomparable to its column (e.g. a `String` against a numeric column) now raises `VortexException` during the scan instead of silently disabling pruning — a behaviour change for callers that relied on the previous silent full scan. ([#159](https://github.com/dfa1/vortex-java/issues/159))
17+
818
## [0.9.0] — 2026-06-24
919

1020
Two import-only breaking changes — the `vortex-core` types moved under `io.github.dfa1.vortex.core.*`, and the no-arg `DType` factories became constants. In return, Vortex now ships with **no FlatBuffers or Protobuf runtime dependency**: the `.fbs`/`.proto` schemas compile in-house to `MemorySegment`-native Java, dropping `com.google.flatbuffers:flatbuffers-java` — the last automatic-module dependency — so a named JPMS `module-info` is viable, and the generated wire classes are prefixed so they no longer collide on your classpath (ADR 0017).

core/src/main/java/io/github/dfa1/vortex/core/model/DType.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,20 @@ public sealed interface DType
2626
/// @return `true` if null values are permitted
2727
boolean nullable();
2828

29+
/// Returns whether this is an unsigned integer type (`U8`–`U64`). `false` for every other
30+
/// type, including signed integers, floats, and the composite/extension types. Useful where
31+
/// unsigned values are stored in a signed `long` (e.g. zone-map comparisons), so the caller
32+
/// knows to use unsigned ordering.
33+
///
34+
/// @return `true` if this is an unsigned-integer [Primitive]
35+
default boolean isUnsigned() {
36+
return switch (this) {
37+
case Primitive(var pt, _) -> pt.isUnsigned();
38+
case Null _, Bool _, Decimal _, Utf8 _, Binary _, Struct _, List _,
39+
FixedSizeList _, Extension _, Variant _ -> false;
40+
};
41+
}
42+
2943
/// Returns a copy of this type marked nullable. Sugar over
3044
/// [#withNullable(boolean)] so call sites read as a fluent adjective:
3145
/// `DType.I64.asNullable()`.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package io.github.dfa1.vortex.core.model;
2+
3+
import org.junit.jupiter.api.Test;
4+
import org.junit.jupiter.params.ParameterizedTest;
5+
import org.junit.jupiter.params.provider.EnumSource;
6+
7+
import java.util.List;
8+
9+
import static org.assertj.core.api.Assertions.assertThat;
10+
11+
class DTypeIsUnsignedTest {
12+
13+
@ParameterizedTest
14+
@EnumSource(value = PType.class, names = {"U8", "U16", "U32", "U64"})
15+
void unsignedPrimitives_areUnsigned(PType pt) {
16+
// Given / When / Then
17+
assertThat(new DType.Primitive(pt, false).isUnsigned()).isTrue();
18+
}
19+
20+
@ParameterizedTest
21+
@EnumSource(value = PType.class, names = {"I8", "I16", "I32", "I64", "F16", "F32", "F64"})
22+
void signedAndFloatPrimitives_areNotUnsigned(PType pt) {
23+
// Given / When / Then
24+
assertThat(new DType.Primitive(pt, false).isUnsigned()).isFalse();
25+
}
26+
27+
@Test
28+
void nonPrimitiveTypes_areNotUnsigned() {
29+
// Given — composite/extension types are never "unsigned", even one that wraps a U64 column
30+
List<DType> types = List.of(
31+
DType.BOOL, DType.UTF8, DType.BINARY, DType.NULL, DType.VARIANT,
32+
new DType.Decimal((byte) 10, (byte) 2, false),
33+
new DType.Struct(List.of("u"), List.of(DType.U64), false));
34+
35+
// When / Then
36+
assertThat(types).allSatisfy(t -> assertThat(t.isUnsigned()).isFalse());
37+
}
38+
}

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

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import java.lang.foreign.SegmentAllocator;
4444
import java.lang.foreign.ValueLayout;
4545
import java.util.ArrayList;
46+
import java.util.HashMap;
4647
import java.util.Iterator;
4748
import java.util.LinkedHashMap;
4849
import java.util.List;
@@ -85,6 +86,7 @@ public final class ScanIterator implements Iterator<Chunk>, AutoCloseable {
8586
private List<ChunkSpec> chunks;
8687
private List<String> projectedNames;
8788
private List<DType> projectedDtypes;
89+
private Map<String, DType> columnDtypes;
8890
private int chunkIndex;
8991
private int peekedChunkIdx = -1;
9092
private long rowsReturned;
@@ -182,14 +184,57 @@ private static ChunkSpec buildChunkSpec(String[] colNames, Map<String, List<Layo
182184
// ── Layout tree traversal ─────────────────────────────────────────────────
183185

184186
@SuppressWarnings("unchecked")
185-
private static int compareValues(Object a, Object b) {
187+
private static int compareValues(Object a, Object b, DType column) {
188+
// Key the compare mode off the *column* type, not the boxed operand type. Stats decode
189+
// integers as Long and floats as Float/Double, and a caller may box a filter value at the
190+
// column's natural width (Integer for I32) or in a different width entirely. Letting the
191+
// column decide keeps pruning width-agnostic (issue #159) without ever routing an integer
192+
// column through double-compare (which would lose precision past 2^53 and mis-prune).
193+
if (a instanceof Number na && b instanceof Number nb) {
194+
if (column instanceof DType.Primitive prim) {
195+
if (prim.ptype().isFloating()) {
196+
return Double.compare(na.doubleValue(), nb.doubleValue());
197+
}
198+
// U64 stats/values store the raw 64 bits, so a value >= 2^63 is a negative Long; an
199+
// unsigned column must compare unsigned. U8/U16/U32 are zero-extended to a positive
200+
// Long where signed == unsigned, so this stays correct for them too.
201+
return column.isUnsigned()
202+
? Long.compareUnsigned(na.longValue(), nb.longValue())
203+
: Long.compare(na.longValue(), nb.longValue());
204+
}
205+
// Column type unresolved (not a struct field) — fall back to a width-agnostic compare
206+
// keyed off the operands so two valid numbers never drop into the throwing path.
207+
if (a instanceof Double || a instanceof Float || b instanceof Double || b instanceof Float) {
208+
return Double.compare(na.doubleValue(), nb.doubleValue());
209+
}
210+
return Long.compare(na.longValue(), nb.longValue());
211+
}
186212
try {
187213
return ((Comparable<Object>) a).compareTo(b);
188-
} catch (ClassCastException _) {
189-
return 0;
214+
} catch (ClassCastException e) {
215+
// A genuinely incomparable filter value (e.g. a String against a numeric column) is a
216+
// caller error — surface it instead of swallowing it into a silent no-prune.
217+
throw new VortexException("filter value of type " + b.getClass().getSimpleName()
218+
+ " is not comparable to the column's zone-map statistic of type "
219+
+ a.getClass().getSimpleName(), e);
190220
}
191221
}
192222

223+
/// Returns the declared [DType] of column `col`, or `null` if the file is not a struct or has
224+
/// no such column. Resolved once from the file's struct schema and cached; used to drive
225+
/// zone-map comparisons by the column's true type rather than the filter value's boxing.
226+
private DType columnDType(String col) {
227+
if (columnDtypes == null) {
228+
columnDtypes = new HashMap<>();
229+
if (file.dtype() instanceof DType.Struct struct) {
230+
for (int i = 0; i < struct.fieldNames().size(); i++) {
231+
columnDtypes.put(struct.fieldNames().get(i), struct.fieldTypes().get(i));
232+
}
233+
}
234+
}
235+
return columnDtypes.get(col);
236+
}
237+
193238
private static Map<String, Array> expandStruct(StructArray sa) {
194239
DType.Struct sd = (DType.Struct) sa.dtype();
195240
List<String> names = sd.fieldNames();
@@ -708,31 +753,31 @@ private boolean canPruneChunk(ChunkSpec chunk, RowFilter filter) {
708753
yield false;
709754
}
710755
Object max = readFlatStats(flat).max();
711-
yield max != null && compareValues(max, val) <= 0;
756+
yield max != null && compareValues(max, val, columnDType(col)) <= 0;
712757
}
713758
case RowFilter.Gte(var col, var val) -> {
714759
Layout flat = chunk.layoutFor(col);
715760
if (flat == null) {
716761
yield false;
717762
}
718763
Object max = readFlatStats(flat).max();
719-
yield max != null && compareValues(max, val) < 0;
764+
yield max != null && compareValues(max, val, columnDType(col)) < 0;
720765
}
721766
case RowFilter.Lt(var col, var val) -> {
722767
Layout flat = chunk.layoutFor(col);
723768
if (flat == null) {
724769
yield false;
725770
}
726771
Object min = readFlatStats(flat).min();
727-
yield min != null && compareValues(min, val) >= 0;
772+
yield min != null && compareValues(min, val, columnDType(col)) >= 0;
728773
}
729774
case RowFilter.Lte(var col, var val) -> {
730775
Layout flat = chunk.layoutFor(col);
731776
if (flat == null) {
732777
yield false;
733778
}
734779
Object min = readFlatStats(flat).min();
735-
yield min != null && compareValues(min, val) > 0;
780+
yield min != null && compareValues(min, val, columnDType(col)) > 0;
736781
}
737782
case RowFilter.Eq(var col, var val) -> {
738783
Layout flat = chunk.layoutFor(col);
@@ -745,13 +790,10 @@ private boolean canPruneChunk(ChunkSpec chunk, RowFilter filter) {
745790
if (min == null || max == null) {
746791
yield false;
747792
}
748-
try {
749-
@SuppressWarnings("unchecked")
750-
Comparable<Object> cv = (Comparable<Object>) val;
751-
yield cv.compareTo(min) < 0 || cv.compareTo(max) > 0;
752-
} catch (ClassCastException _) {
753-
yield false;
754-
}
793+
// val < min || val > max → no row in this chunk can equal val. Route through the
794+
// shared comparator so this path is width-agnostic and unsigned-aware too (#159).
795+
DType ct = columnDType(col);
796+
yield compareValues(val, min, ct) < 0 || compareValues(val, max, ct) > 0;
755797
}
756798
case RowFilter.Neq(var col, var val) -> {
757799
Layout flat = chunk.layoutFor(col);
@@ -764,13 +806,9 @@ private boolean canPruneChunk(ChunkSpec chunk, RowFilter filter) {
764806
if (min == null || max == null) {
765807
yield false;
766808
}
767-
try {
768-
@SuppressWarnings("unchecked")
769-
Comparable<Object> cv = (Comparable<Object>) val;
770-
yield cv.compareTo(min) == 0 && cv.compareTo(max) == 0;
771-
} catch (ClassCastException _) {
772-
yield false;
773-
}
809+
// Every row equals val (min == max == val) → no row is != val.
810+
DType ct = columnDType(col);
811+
yield compareValues(val, min, ct) == 0 && compareValues(val, max, ct) == 0;
774812
}
775813
case RowFilter.IsNull(var col) -> {
776814
Layout flat = chunk.layoutFor(col);

0 commit comments

Comments
 (0)