Skip to content

Commit f8ad15d

Browse files
dfa1claude
andcommitted
refactor(reader): typed columns from ScanIterator down
Chunk's two parallel string-keyed maps (columns + columnDtypes) collapse into one SequencedMap<ColumnName, Chunk.Column> — the Array and its DType travel together in the Column carrier, so desync is unrepresentable, and schema order is now part of the contract (previously Map.of gave no order guarantee for 1-2 column chunks). ColumnName originates in ScanIterator.initialize(), parsed once from the file's DType.Struct (the parse edge has already policy- certified every name); ChunkSpec, layout lookups, zone-stat caches and the column-map builders are typed end to end. Public rims keep String sugar converted exactly once: chunk.column(String), RowFilter references, columnZoneStats. A policy-invalid query name fails fast with the policy message — it could never match a certified column; valid-but-absent names keep the exact previous behavior and messages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 8a81331 commit f8ad15d

14 files changed

Lines changed: 181 additions & 124 deletions

File tree

cli/src/main/java/io/github/dfa1/vortex/cli/tui/VortexInspectorTui.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import io.github.dfa1.vortex.reader.layout.Layout;
44
import io.github.dfa1.vortex.reader.SegmentSpec;
55
import io.github.dfa1.vortex.core.model.DType;
6+
import io.github.dfa1.vortex.core.model.ColumnName;
67
import io.github.dfa1.vortex.reader.array.Array;
78
import io.github.dfa1.vortex.cli.tui.term.Ansi;
89
import io.github.dfa1.vortex.cli.tui.term.Key;
@@ -766,11 +767,12 @@ private void runDataLoad(String columnName) {
766767
return;
767768
}
768769
try (Chunk chunk = it.next()) {
769-
Array array = chunk.columns().get(columnName);
770-
if (array == null) {
770+
Chunk.Column column = chunk.columns().get(ColumnName.of(columnName));
771+
if (column == null) {
771772
dataCache.put(columnName, new DataState.Loaded(List.of()));
772773
return;
773774
}
775+
Array array = column.array();
774776
int n = (int) Math.min(array.length(), DATA_PREVIEW_ROWS);
775777
List<String> out = new ArrayList<>(n);
776778
for (int i = 0; i < n; i++) {

integration/src/test/java/io/github/dfa1/vortex/integration/AllowUnknownIntegrationTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import dev.vortex.arrow.ArrowAllocation;
66
import dev.vortex.jni.NativeLoader;
77
import io.github.dfa1.vortex.core.error.VortexException;
8-
import io.github.dfa1.vortex.reader.array.Array;
8+
import io.github.dfa1.vortex.reader.Chunk;
99
import io.github.dfa1.vortex.reader.array.UnknownArray;
1010
import io.github.dfa1.vortex.reader.ReadRegistry;
1111
import io.github.dfa1.vortex.reader.VortexReader;
@@ -87,8 +87,8 @@ void allowUnknown_emptyRegistry_allColumnsReturnUnknownArray(@TempDir Path tmp)
8787
iter.forEachRemaining(c -> {
8888
totalRows.addAndGet(c.rowCount());
8989
chunkCount.incrementAndGet();
90-
for (Array col : c.columns().values()) {
91-
if (!(col instanceof UnknownArray)) {
90+
for (Chunk.Column col : c.columns().values()) {
91+
if (!(col.array() instanceof UnknownArray)) {
9292
allUnknown.set(false);
9393
}
9494
}
@@ -130,8 +130,8 @@ void allowUnknown_loadAllRegistry_noUnknownArrayForSupportedEncodings(@TempDir P
130130
var iter = vf.scan(io.github.dfa1.vortex.reader.ScanOptions.all())) {
131131
iter.forEachRemaining(c -> {
132132
chunkCount.incrementAndGet();
133-
for (Array col : c.columns().values()) {
134-
if (col instanceof UnknownArray) {
133+
for (Chunk.Column col : c.columns().values()) {
134+
if (col.array() instanceof UnknownArray) {
135135
anyUnknown.set(true);
136136
}
137137
}

integration/src/test/java/io/github/dfa1/vortex/integration/JavaRoundTripIntegrationTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private static int[] readIntColumn(Path file, String column) throws IOException
6565
var iter = vf.scan(ScanOptions.columns(column))) {
6666
var ints = new ArrayList<Integer>();
6767
iter.forEachRemaining(c -> {
68-
IntArray arr = (IntArray) c.columns().get(column);
68+
IntArray arr = c.column(column);
6969
for (long i = 0; i < arr.length(); i++) {
7070
ints.add(arr.getInt(i));
7171
}

integration/src/test/java/io/github/dfa1/vortex/integration/RustJavaReaderComparisonIntegrationTest.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import io.github.dfa1.vortex.reader.ReadRegistry;
2222
import io.github.dfa1.vortex.inspect.VortexInspector;
2323
import io.github.dfa1.vortex.reader.VortexReader;
24+
import io.github.dfa1.vortex.core.model.ColumnName;
2425
import io.github.dfa1.vortex.reader.Chunk;
2526
import org.apache.arrow.memory.BufferAllocator;
2627
import org.apache.arrow.vector.BigIntVector;
@@ -229,18 +230,19 @@ private static Stats javaStats(Path file) throws Exception {
229230
while (iter.hasNext()) {
230231
try (Chunk chunk = iter.next()) {
231232
rowCount += chunk.rowCount();
232-
for (Map.Entry<String, Array> e : chunk.columns().entrySet()) {
233-
if (extensionCols.contains(e.getKey())) {
233+
for (Map.Entry<ColumnName, Chunk.Column> e : chunk.columns().entrySet()) {
234+
String name = e.getKey().value();
235+
if (extensionCols.contains(name)) {
234236
continue;
235237
}
236-
Array arr = e.getValue();
238+
Array arr = e.getValue().array();
237239
Double numSum = numericSum(arr);
238240
if (numSum != null) {
239-
numSums.merge(e.getKey(), numSum, Double::sum);
241+
numSums.merge(name, numSum, Double::sum);
240242
}
241243
Long strLen = stringByteLength(arr);
242244
if (strLen != null && strLen > 0) {
243-
strLenSums.merge(e.getKey(), strLen, Long::sum);
245+
strLenSums.merge(name, strLen, Long::sum);
244246
}
245247
}
246248
}

integration/src/test/java/io/github/dfa1/vortex/integration/RustWritesJavaReadsIntegrationTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ private static List<JavaChunk> scanAll(VortexReader vf,
125125
iter.forEachRemaining(c -> {
126126
var mat = new LinkedHashMap<String, Object>(c.columns().size());
127127
for (var e : c.columns().entrySet()) {
128-
mat.put(e.getKey(), snapshotArray(e.getValue()));
128+
mat.put(e.getKey().value(), snapshotArray(e.getValue().array()));
129129
}
130130
results.add(new JavaChunk(c.rowCount(), mat));
131131
});
@@ -212,7 +212,7 @@ private static long[] readJavaLongColumn(Path file, String column) throws IOExce
212212
var iter = vf.scan(io.github.dfa1.vortex.reader.ScanOptions.columns(column))) {
213213
var longs = new ArrayList<Long>();
214214
iter.forEachRemaining(c -> {
215-
LongArray arr = (LongArray) c.columns().get(column);
215+
LongArray arr = c.column(column);
216216
for (long i = 0; i < arr.length(); i++) {
217217
longs.add(arr.getLong(i));
218218
}

performance/src/main/java/io/github/dfa1/vortex/performance/JniWritesJavaReadsBigFileBenchmark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ private long scanJava() throws IOException {
179179
var iter = vf.scan(io.github.dfa1.vortex.reader.ScanOptions.columns("c0"))) {
180180
while (iter.hasNext()) {
181181
try (Chunk c = iter.next()) {
182-
Array arr = c.columns().get("c0");
182+
Array arr = c.column("c0");
183183
MemorySegment buf = arr.materialize(Arena.ofAuto());
184184
long count = buf.byteSize() / Long.BYTES;
185185
for (long i = 0; i < count; i++) {

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

Lines changed: 55 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.github.dfa1.vortex.reader;
22

3+
import io.github.dfa1.vortex.core.model.ColumnName;
34
import io.github.dfa1.vortex.core.model.DType;
45
import io.github.dfa1.vortex.core.error.VortexException;
56
import io.github.dfa1.vortex.reader.array.Array;
@@ -14,8 +15,8 @@
1415
import java.time.LocalDate;
1516
import java.time.LocalTime;
1617
import java.util.List;
17-
import java.util.Map;
1818
import java.util.Objects;
19+
import java.util.SequencedMap;
1920
import java.util.UUID;
2021
import java.util.function.Consumer;
2122

@@ -26,6 +27,11 @@
2627
/// references into that arena (or into the underlying mmap region), valid only
2728
/// while this `Chunk` is open.
2829
///
30+
/// Columns are keyed by [ColumnName] — the validated name domain the file parser
31+
/// certifies at the read boundary, so every key here is policy-valid and unique.
32+
/// [#column(String)] keeps a string-sugar overload for callers that hold a raw
33+
/// name; it wraps the string in a [ColumnName] before looking it up.
34+
///
2935
/// **Lifecycle.** `Chunk` is [AutoCloseable]. Always wrap consumption in
3036
/// try-with-resources:
3137
///
@@ -43,50 +49,80 @@
4349
public final class Chunk implements AutoCloseable {
4450

4551
private final long rowCount;
46-
private final Map<String, Array> columns;
47-
private final Map<String, DType> columnDtypes;
52+
private final SequencedMap<ColumnName, Column> columns;
4853
private final Arena arena;
4954
private final Consumer<Chunk> onClose;
5055
private boolean closed;
5156

52-
Chunk(long rowCount, Map<String, Array> columns, Map<String, DType> columnDtypes,
57+
Chunk(long rowCount, SequencedMap<ColumnName, Column> columns,
5358
Arena arena, Consumer<Chunk> onClose) {
5459
this.rowCount = rowCount;
5560
this.columns = Objects.requireNonNull(columns);
56-
this.columnDtypes = Objects.requireNonNull(columnDtypes);
5761
this.arena = Objects.requireNonNull(arena);
5862
this.onClose = Objects.requireNonNull(onClose);
5963
}
6064

65+
/// One decoded column: its zero-copy [Array] view and the [DType] it was declared with in the
66+
/// file's schema. The dtype travels with the array so extension decoding (see [#as(String, Class)])
67+
/// and dtype-aware tooling need no second lookup.
68+
///
69+
/// @param array the decoded column values, valid only while the owning [Chunk] is open
70+
/// @param dtype the column's declared type from the file schema
71+
public record Column(Array array, DType dtype) {
72+
73+
/// Binds a decoded array to its declared type.
74+
///
75+
/// @param array the decoded column values
76+
/// @param dtype the column's declared type
77+
public Column {
78+
Objects.requireNonNull(array, "array");
79+
Objects.requireNonNull(dtype, "dtype");
80+
}
81+
}
82+
6183
/// Number of logical rows in this chunk (after any limit truncation).
6284
///
6385
/// @return row count of this chunk
6486
public long rowCount() {
6587
return rowCount;
6688
}
6789

68-
/// Returns the decoded columns by name. The map is unmodifiable; values are
69-
/// valid only while this `Chunk` is open.
90+
/// Returns the decoded columns in schema (projection) order. The map is unmodifiable and
91+
/// preserves encounter order; each [Column] value is valid only while this `Chunk` is open.
7092
///
71-
/// @return projected columns keyed by name
72-
public Map<String, Array> columns() {
93+
/// @return projected columns keyed by [ColumnName], in schema order
94+
public SequencedMap<ColumnName, Column> columns() {
7395
return columns;
7496
}
7597

76-
/// Looks up a column by name with a checked cast to the caller's expected
77-
/// [Array] subtype.
98+
/// Looks up a column by its raw string name with a checked cast to the caller's expected
99+
/// [Array] subtype. The name is validated through [ColumnName#of(String)] first, so a
100+
/// policy-invalid query name fails fast with the policy's [IllegalArgumentException] — it
101+
/// could never match a certified column anyway.
78102
///
79103
/// @param name column name as declared in the file's [io.github.dfa1.vortex.core.model.DType] schema
80104
/// @param <T> expected concrete [Array] subtype
81105
/// @return the column array
106+
/// @throws IllegalArgumentException if `name` violates the column-name policy
107+
/// @throws VortexException if no column with the given name is present in this chunk
108+
public <T extends Array> T column(String name) {
109+
return column(ColumnName.of(name));
110+
}
111+
112+
/// Looks up a column by its validated [ColumnName] with a checked cast to the caller's
113+
/// expected [Array] subtype.
114+
///
115+
/// @param name validated column name as declared in the file's schema
116+
/// @param <T> expected concrete [Array] subtype
117+
/// @return the column array
82118
/// @throws VortexException if no column with the given name is present in this chunk
83119
@SuppressWarnings("unchecked")
84-
public <T extends Array> T column(String name) {
85-
Array arr = columns.get(name);
86-
if (arr == null) {
120+
public <T extends Array> T column(ColumnName name) {
121+
Column col = columns.get(name);
122+
if (col == null) {
87123
throw new VortexException("unknown column: " + name);
88124
}
89-
return (T) arr;
125+
return (T) col.array();
90126
}
91127

92128
/// Decodes an extension column into a typed `List<T>` of domain values.
@@ -110,16 +146,16 @@ public <T extends Array> T column(String name) {
110146
/// or the requested `domainType` doesn't match the column's extension id
111147
@SuppressWarnings("unchecked")
112148
public <T> List<T> as(String name, Class<T> domainType) {
113-
DType colDtype = columnDtypes.get(name);
114-
if (colDtype == null) {
149+
Column col = columns.get(ColumnName.of(name));
150+
if (col == null) {
115151
throw new VortexException("unknown column: " + name);
116152
}
117-
if (!(colDtype instanceof DType.Extension ext)) {
153+
if (!(col.dtype() instanceof DType.Extension ext)) {
118154
throw new VortexException("not an extension column: " + name);
119155
}
120156
ExtensionId id = ExtensionId.parse(ext.extensionId())
121157
.orElseThrow(() -> new VortexException("not a spec extension id: " + ext.extensionId()));
122-
Array storage = column(name);
158+
Array storage = col.array();
123159
Object result = switch (id) {
124160
case VORTEX_DATE -> {
125161
requireDomainType(name, domainType, LocalDate.class);

0 commit comments

Comments
 (0)