Skip to content

Commit 731dc81

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 731dc81

5 files changed

Lines changed: 360 additions & 26 deletions

File tree

CHANGELOG.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,20 @@ 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). ([#159](https://github.com/dfa1/vortex-java/issues/159))
17+
18+
### Changed
19+
20+
- **Behaviour change:** a filter value that is genuinely incomparable to its column (e.g. a `String` against a numeric column) now raises `VortexException` during the scan instead of silently disabling pruning. Callers that relied on the previous silent full scan will see an exception. ([#159](https://github.com/dfa1/vortex-java/issues/159))
21+
822
## [0.9.0] — 2026-06-24
923

1024
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: 51 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,49 @@ 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 && column instanceof DType.Primitive prim) {
194+
if (prim.ptype().isFloating()) {
195+
return Double.compare(na.doubleValue(), nb.doubleValue());
196+
}
197+
// U64 stats/values store the raw 64 bits, so a value >= 2^63 is a negative Long; an
198+
// unsigned column must compare unsigned. U8/U16/U32 are zero-extended to a positive
199+
// Long where signed == unsigned, so this stays correct for them too.
200+
return column.isUnsigned()
201+
? Long.compareUnsigned(na.longValue(), nb.longValue())
202+
: Long.compare(na.longValue(), nb.longValue());
203+
}
186204
try {
187205
return ((Comparable<Object>) a).compareTo(b);
188-
} catch (ClassCastException _) {
189-
return 0;
206+
} catch (ClassCastException e) {
207+
// A genuinely incomparable filter value (e.g. a String against a numeric column) is a
208+
// caller error — surface it instead of swallowing it into a silent no-prune.
209+
throw new VortexException("filter value of type " + b.getClass().getSimpleName()
210+
+ " is not comparable to the column's zone-map statistic of type "
211+
+ a.getClass().getSimpleName(), e);
190212
}
191213
}
192214

215+
/// Returns the declared [DType] of column `col`, or `null` if the file is not a struct or has
216+
/// no such column. Resolved once from the file's struct schema and cached; used to drive
217+
/// zone-map comparisons by the column's true type rather than the filter value's boxing.
218+
private DType columnDType(String col) {
219+
if (columnDtypes == null) {
220+
columnDtypes = new HashMap<>();
221+
if (file.dtype() instanceof DType.Struct struct) {
222+
for (int i = 0; i < struct.fieldNames().size(); i++) {
223+
columnDtypes.put(struct.fieldNames().get(i), struct.fieldTypes().get(i));
224+
}
225+
}
226+
}
227+
return columnDtypes.get(col);
228+
}
229+
193230
private static Map<String, Array> expandStruct(StructArray sa) {
194231
DType.Struct sd = (DType.Struct) sa.dtype();
195232
List<String> names = sd.fieldNames();
@@ -708,31 +745,31 @@ private boolean canPruneChunk(ChunkSpec chunk, RowFilter filter) {
708745
yield false;
709746
}
710747
Object max = readFlatStats(flat).max();
711-
yield max != null && compareValues(max, val) <= 0;
748+
yield max != null && compareValues(max, val, columnDType(col)) <= 0;
712749
}
713750
case RowFilter.Gte(var col, var val) -> {
714751
Layout flat = chunk.layoutFor(col);
715752
if (flat == null) {
716753
yield false;
717754
}
718755
Object max = readFlatStats(flat).max();
719-
yield max != null && compareValues(max, val) < 0;
756+
yield max != null && compareValues(max, val, columnDType(col)) < 0;
720757
}
721758
case RowFilter.Lt(var col, var val) -> {
722759
Layout flat = chunk.layoutFor(col);
723760
if (flat == null) {
724761
yield false;
725762
}
726763
Object min = readFlatStats(flat).min();
727-
yield min != null && compareValues(min, val) >= 0;
764+
yield min != null && compareValues(min, val, columnDType(col)) >= 0;
728765
}
729766
case RowFilter.Lte(var col, var val) -> {
730767
Layout flat = chunk.layoutFor(col);
731768
if (flat == null) {
732769
yield false;
733770
}
734771
Object min = readFlatStats(flat).min();
735-
yield min != null && compareValues(min, val) > 0;
772+
yield min != null && compareValues(min, val, columnDType(col)) > 0;
736773
}
737774
case RowFilter.Eq(var col, var val) -> {
738775
Layout flat = chunk.layoutFor(col);
@@ -745,13 +782,10 @@ private boolean canPruneChunk(ChunkSpec chunk, RowFilter filter) {
745782
if (min == null || max == null) {
746783
yield false;
747784
}
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-
}
785+
// val < min || val > max → no row in this chunk can equal val. Route through the
786+
// shared comparator so this path is width-agnostic and unsigned-aware too (#159).
787+
DType ct = columnDType(col);
788+
yield compareValues(val, min, ct) < 0 || compareValues(val, max, ct) > 0;
755789
}
756790
case RowFilter.Neq(var col, var val) -> {
757791
Layout flat = chunk.layoutFor(col);
@@ -764,13 +798,9 @@ private boolean canPruneChunk(ChunkSpec chunk, RowFilter filter) {
764798
if (min == null || max == null) {
765799
yield false;
766800
}
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-
}
801+
// Every row equals val (min == max == val) → no row is != val.
802+
DType ct = columnDType(col);
803+
yield compareValues(val, min, ct) == 0 && compareValues(val, max, ct) == 0;
774804
}
775805
case RowFilter.IsNull(var col) -> {
776806
Layout flat = chunk.layoutFor(col);

0 commit comments

Comments
 (0)