Skip to content

Commit b45fd98

Browse files
dfa1claude
andcommitted
refactor(scan)!: replace ScanResult with closeable Chunk; ScanIterator implements Iterator<Chunk>
ScanIterator.hasNext() used to silently close the previous chunk's Arena, invalidating any Array references the caller still held — a footgun the compiler could not catch. Java has no borrow checker, so Rust-style lending- iterator semantics cannot be enforced at compile time. Move to idiomatic Java lifecycle: - ScanResult → Chunk implements AutoCloseable. Each Chunk owns a confined Arena holding its decoded buffers; close() releases the arena. - ScanIterator now implements Iterator<Chunk>, AutoCloseable. hasNext() is side-effect-free; next() returns a fresh Chunk and throws IllegalStateException if a prior chunk is still open. Iterator close() closes any still-open chunk as a safety net. - forEachRemaining(Consumer<? super Chunk>) is overridden to wrap each next() in try-with-resources, so the standard JDK method works safely without inventing a custom forEachChunk. After Chunk.close(), touching previously-returned Array views raises FFM's scope check (IllegalStateException) instead of returning undefined data. Idiomatic call site: try (var reader = VortexReader.open(path); var iter = reader.scan(opts)) { while (iter.hasNext()) { try (Chunk chunk = iter.next()) { ... } } } CLAUDE.md gains a "prefer idiomatic modern Java" rule documenting the preference for overriding standard JDK methods (forEachRemaining) over parallel custom names. README, docs/explanation.md, docs/reference.md and CHANGELOG updated. All call sites (CLI, CSV exporter, benchmarks, all unit and integration tests) migrated to try-with-resources or forEachRemaining. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent ede7f84 commit b45fd98

27 files changed

Lines changed: 723 additions & 556 deletions

File tree

CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- `Chunk` (`io.github.dfa1.vortex.scan.Chunk`) — replaces `ScanResult`. Each chunk owns
13+
a confined `Arena` and implements `AutoCloseable`. The standard
14+
`Iterator.forEachRemaining(Consumer<Chunk>)` is overridden on `ScanIterator` to wrap
15+
each chunk in try-with-resources, so callers that don't need early-exit get
16+
automatic per-chunk cleanup with no new API to learn.
17+
1218
### Changed
1319

20+
- **Breaking — scan API lifecycle.** `ScanIterator` now implements
21+
`Iterator<Chunk>`. `next()` returns a `Chunk` that the caller must close
22+
(try-with-resources); `hasNext()` is side-effect-free. Calling `next()` while a
23+
prior `Chunk` is still open throws `IllegalStateException`. This removes the
24+
previous footgun where `iter.hasNext()` silently closed the previous chunk's
25+
arena, invalidating any `Array` references the caller still held. Use after
26+
`close()` raises FFM's scope check (`IllegalStateException`) instead of returning
27+
undefined data. See the updated examples in `README.md` and
28+
`docs/explanation.md#memory-model`.
29+
1430
### Fixed
1531

1632
### Removed
1733

34+
- `ScanResult` — renamed to `Chunk` and given lifecycle methods. Update imports:
35+
`io.github.dfa1.vortex.scan.ScanResult``io.github.dfa1.vortex.scan.Chunk`.
36+
1837
[0.5.0]: https://github.com/dfa1/vortex-java/compare/v0.4.0...main
1938

2039
## [0.4.0] — 2026-06-07

CLAUDE.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,12 @@ Omit a section if empty (e.g. integration module has no production deps; perform
180180
- Zero SonarQube bugs/smells policy.
181181
- No `sun.misc.Unsafe` or internal JDK APIs.
182182
- Prefer explicit over clever. Fail fast on unhandled cases.
183+
- Always prefer idiomatic modern Java. Reuse the standard library and language
184+
features the JDK already provides — e.g. override `Iterator.forEachRemaining`
185+
instead of inventing a parallel `forEachChunk`; use `Optional`, records,
186+
sealed types, pattern switches, virtual threads, FFM — over hand-rolled
187+
equivalents. New APIs should look and feel like JDK APIs Java developers
188+
already know.
183189
- Always use braces for `if`/`else`/`for`/`while` bodies, even single-liners:
184190
```java
185191
// WRONG

README.md

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,22 @@ for zero-copy memory-mapped reads.
3737
try (VortexReader vf = VortexReader.open(Path.of("data/example.vortex"));
3838
var iter = vf.scan(ScanOptions.all())) {
3939
while (iter.hasNext()) {
40-
var chunk = iter.next();
41-
LongArray ts = chunk.column("timestamp");
42-
for (long i = 0; i < ts.length(); i++) {
43-
System.out.println(ts.getLong(i));
40+
try (Chunk chunk = iter.next()) {
41+
LongArray ts = chunk.column("timestamp");
42+
for (long i = 0; i < ts.length(); i++) {
43+
System.out.println(ts.getLong(i));
44+
}
4445
}
4546
}
4647
}
4748
```
4849

49-
> **Note:** `iter.hasNext()` closes the previous chunk's arena. Access all column data
50-
> before calling `hasNext()` again. See [docs/explanation.md#memory-model](docs/explanation.md#memory-model).
50+
> **Lifecycle.** `ScanIterator` implements `Iterator<Chunk>` and `Chunk` implements
51+
> `AutoCloseable`. Each chunk owns a confined `Arena`; closing it releases the
52+
> decoded buffers. Calling `iter.next()` while a prior chunk is still open throws
53+
> `IllegalStateException`. Use try-with-resources, or
54+
> `iter.forEachRemaining(c -> ...)` which closes each chunk for you. See
55+
> [docs/explanation.md#memory-model](docs/explanation.md#memory-model).
5156
5257
For more examples — writing, projection, filtering, custom encodings, and the CLI —
5358
see the documentation below.

csv/src/main/java/io/github/dfa1/vortex/csv/CsvExporter.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import io.github.dfa1.vortex.io.VortexReader;
1616
import io.github.dfa1.vortex.scan.ScanIterator;
1717
import io.github.dfa1.vortex.scan.ScanOptions;
18-
import io.github.dfa1.vortex.scan.ScanResult;
18+
import io.github.dfa1.vortex.scan.Chunk;
1919

2020
import java.io.FilterWriter;
2121
import java.io.IOException;
@@ -83,20 +83,21 @@ private static void export(VortexReader reader, CsvWriter csvWriter, ExportOptio
8383
String[] row = new String[colCount];
8484
try (ScanIterator iter = reader.scan(scanOptions)) {
8585
while (iter.hasNext()) {
86-
ScanResult chunk = iter.next();
87-
Array[] arrays = new Array[colCount];
88-
for (int c = 0; c < colCount; c++) {
89-
arrays[c] = chunk.column(colNames.get(c));
90-
}
91-
long rowCount = chunk.rowCount();
92-
for (long r = 0; r < rowCount; r++) {
93-
if (!predicate.test(chunk, r)) {
94-
continue;
95-
}
86+
try (Chunk chunk = iter.next()) {
87+
Array[] arrays = new Array[colCount];
9688
for (int c = 0; c < colCount; c++) {
97-
row[c] = cellValue(arrays[c], r);
89+
arrays[c] = chunk.column(colNames.get(c));
90+
}
91+
long rowCount = chunk.rowCount();
92+
for (long r = 0; r < rowCount; r++) {
93+
if (!predicate.test(chunk, r)) {
94+
continue;
95+
}
96+
for (int c = 0; c < colCount; c++) {
97+
row[c] = cellValue(arrays[c], r);
98+
}
99+
csvWriter.writeRecord(row);
98100
}
99-
csvWriter.writeRecord(row);
100101
}
101102
}
102103
}
Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,23 @@
11
package io.github.dfa1.vortex.csv;
22

3-
import io.github.dfa1.vortex.scan.ScanResult;
3+
import io.github.dfa1.vortex.scan.Chunk;
44

55
/// Row-level predicate evaluated against decoded chunk data.
66
/// Used in conjunction with zone-map pruning: zone-maps skip whole chunks,
77
/// this predicate filters individual rows within surviving chunks.
88
@FunctionalInterface
99
public interface RowPredicate {
10+
/// Returns a predicate that accepts every row.
11+
///
12+
/// @return predicate that always returns {@code true}
1013
static RowPredicate all() {
1114
return (_, _) -> true;
1215
}
1316

14-
boolean test(ScanResult chunk, long rowIndex);
17+
/// Tests whether a row should be exported.
18+
///
19+
/// @param chunk decoded chunk containing the row
20+
/// @param rowIndex row index within {@code chunk}
21+
/// @return {@code true} if the row should be exported
22+
boolean test(Chunk chunk, long rowIndex);
1523
}

csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
import io.github.dfa1.vortex.core.array.LongArray;
66
import io.github.dfa1.vortex.core.array.VarBinArray;
77
import io.github.dfa1.vortex.io.VortexReader;
8+
import io.github.dfa1.vortex.scan.Chunk;
89
import io.github.dfa1.vortex.scan.ScanIterator;
910
import io.github.dfa1.vortex.scan.ScanOptions;
10-
import io.github.dfa1.vortex.scan.ScanResult;
1111
import io.github.dfa1.vortex.writer.WriteOptions;
1212
import org.junit.jupiter.api.Test;
1313
import org.junit.jupiter.api.io.TempDir;
@@ -41,14 +41,15 @@ void infersTypedColumnsAndRoundTrips(@TempDir Path tmp) throws Exception {
4141

4242
try (ScanIterator iter = reader.scan(ScanOptions.all())) {
4343
assertThat(iter.hasNext()).isTrue();
44-
ScanResult chunk = iter.next();
45-
assertThat(chunk.rowCount()).isEqualTo(2);
46-
LongArray ids = chunk.column("id");
47-
assertThat(ids.getLong(0)).isEqualTo(1L);
48-
assertThat(ids.getLong(1)).isEqualTo(2L);
49-
VarBinArray names = chunk.column("name");
50-
assertThat(names.getString(0)).isEqualTo("Alice");
51-
assertThat(names.getString(1)).isEqualTo("Bob");
44+
try (Chunk chunk = iter.next()) {
45+
assertThat(chunk.rowCount()).isEqualTo(2);
46+
LongArray ids = chunk.column("id");
47+
assertThat(ids.getLong(0)).isEqualTo(1L);
48+
assertThat(ids.getLong(1)).isEqualTo(2L);
49+
VarBinArray names = chunk.column("name");
50+
assertThat(names.getString(0)).isEqualTo("Alice");
51+
assertThat(names.getString(1)).isEqualTo("Bob");
52+
}
5253
}
5354
}
5455
}
@@ -107,9 +108,10 @@ void respectsSchemaOverride(@TempDir Path tmp) throws Exception {
107108
assertThat(schema.fieldTypes().getFirst()).isEqualTo(new DType.Utf8(false));
108109
try (ScanIterator iter = reader.scan(ScanOptions.all())) {
109110
assertThat(iter.hasNext()).isTrue();
110-
ScanResult chunk = iter.next();
111-
VarBinArray values = chunk.column("value");
112-
assertThat(values.getString(0)).isEqualTo("42");
111+
try (Chunk chunk = iter.next()) {
112+
VarBinArray values = chunk.column("value");
113+
assertThat(values.getString(0)).isEqualTo("42");
114+
}
113115
}
114116
}
115117
}

docs/explanation.md

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,40 @@ and the chunk-0 stats child — the tree collapses to `Struct → Chunked → [F
132132
## Memory model
133133

134134
`VortexReader` memory-maps the entire file into one `MemorySegment` (confined `Arena`).
135-
All `Array` buffers returned during a scan are zero-copy slices of that segment — their
136-
lifetime is tied to the `VortexReader`. Close the reader to release the mapped region.
135+
Decoded `Array` buffers returned during a scan are zero-copy slices of that segment —
136+
or of a per-chunk arena allocated for decode output. Close the reader to release
137+
the mapped region.
138+
139+
### Per-chunk lifetime: `Chunk implements AutoCloseable`
140+
141+
`ScanIterator` implements `Iterator<Chunk>`. Each `Chunk` owns a confined `Arena`
142+
that holds its decoded columnar buffers; calling `chunk.close()` releases the arena.
143+
The idiomatic pattern is nested try-with-resources:
144+
145+
```java
146+
try (var reader = VortexReader.open(path);
147+
var iter = reader.scan(opts)) { // releases iterator state
148+
while (iter.hasNext()) {
149+
try (Chunk chunk = iter.next()) { // releases this chunk's arena
150+
// use chunk.column(...) — refs are valid only inside this block
151+
}
152+
}
153+
}
154+
```
155+
156+
Calling `iter.next()` while a previous chunk is still open throws
157+
`IllegalStateException` — the API refuses to silently invalidate live references.
158+
After `chunk.close()`, touching any previously-returned `Array` raises FFM's scope
159+
check (`IllegalStateException` from `MemorySegment`), not undefined behavior.
137160

138-
The iterator-based scan API is load-bearing: `iter.hasNext()` closes the previous chunk's
139-
arena. Access all column data before calling `hasNext()` again.
161+
For bulk consumption with auto-close per element, override the standard
162+
`Iterator.forEachRemaining` is provided:
163+
164+
```java
165+
try (var iter = reader.scan(opts)) {
166+
iter.forEachRemaining(c -> sum += c.column("price").fold(0.0, Double::sum));
167+
}
168+
```
140169

141170
For the reader / scan method signatures, see [reference.md#reader-api](reference.md#reader-api).
142171

@@ -250,8 +279,8 @@ VortexReader.open(path)
250279
vortexReader.scan(opts) → ScanIterator
251280
└─ pre-index Flat nodes into ChunkSpec[] — one entry per row group per column
252281
253-
ScanIterator.next() → ScanResult (per row-group)
254-
└─ decodeLayout(layout, dtype, chunkArena)
282+
ScanIterator.next() → Chunk (per row-group, AutoCloseable; owns its own Arena)
283+
└─ decodeLayout(layout, dtype, chunk.arena)
255284
├─ Flat → slice MemorySegment from mmap region
256285
│ └─ EncodingRegistry.decodeSegment(seg, …)
257286
│ └─ Encoding.decode(DecodeContext) → Array (zero-copy)
@@ -260,8 +289,9 @@ ScanIterator.next() → ScanResult (per row-group)
260289
└─ Dict → decode values layout + codes layout separately, then expand
261290
```
262291

263-
All `Array` buffers are zero-copy slices of the mmap'd `MemorySegment`.
264-
Advancing the iterator (`hasNext()`) closes the chunk's `Arena` and releases them.
292+
Decoded `Array` buffers are either zero-copy slices of the mmap'd `MemorySegment`
293+
or allocations in the chunk's own `Arena`. `chunk.close()` releases that arena —
294+
after which any reference into it raises FFM's scope check.
265295

266296
### Write path
267297

docs/reference.md

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,23 +130,29 @@ Sealed predicate used for zone-map pruning (per-chunk min/max). Chunks that cann
130130

131131
### `ScanIterator` (`io.github.dfa1.vortex.scan.ScanIterator`)
132132

133-
Implements `AutoCloseable`. Drives one scan.
133+
Implements `Iterator<Chunk>` and `AutoCloseable`. Drives one scan.
134134

135-
| Method | Notes |
136-
|-------------|----------------------------------------------------------------------|
137-
| `hasNext()` | **Closes the previous chunk's arena.** Access all column data first. |
138-
| `next()` | Returns `ScanResult` |
139-
| `close()` | Releases iterator state |
135+
| Method | Notes |
136+
|------------------------|--------------------------------------------------------------------------------------|
137+
| `hasNext()` | Side-effect-free. Returns whether another chunk is available after zone-map pruning. |
138+
| `next()` | Returns a fresh `Chunk` whose arena the caller closes. Throws `IllegalStateException` if a prior `Chunk` is still open, or `NoSuchElementException` if exhausted. |
139+
| `forEachRemaining(Consumer)` | Overridden to wrap each `next()` in try-with-resources so chunks auto-close. |
140+
| `close()` | Releases iterator state and closes any chunk still open. |
140141

141-
### `ScanResult` (`io.github.dfa1.vortex.scan.ScanResult`)
142+
### `Chunk` (`io.github.dfa1.vortex.scan.Chunk`)
142143

143-
Record: `(long rowCount, Map<String, Array> columns)`.
144+
Implements `AutoCloseable`. Each chunk owns a confined `Arena` holding the decoded
145+
columnar buffers; closing the chunk releases the arena. After `close()`, touching
146+
any `Array` previously returned by `column(...)` or `columns()` raises FFM's scope
147+
check (`IllegalStateException`).
144148

145149
| Method | Notes |
146150
|-----------------------------------------|----------------------------------------------------------|
147151
| `rowCount()` | Rows in this chunk |
148152
| `columns()` | All columns in this chunk |
149153
| `<T extends Array> column(String name)` | Typed column lookup; throws `VortexException` if unknown |
154+
| `isClosed()` | Whether `close()` has run |
155+
| `close()` | Releases the chunk's arena. Idempotent. |
150156

151157
---
152158

0 commit comments

Comments
 (0)