Skip to content

Commit 4d18939

Browse files
dfa1claude
andcommitted
feat(writer): accept boxed nullable arrays on the map writeChunk path
The map entry point writeChunk(Map) only accepted raw primitive arrays, so a nullable column had to be supplied via the builder (writeChunk(c -> c.put(..., new Long[]{...}))). Passing a boxed Long[]/Integer[]/… through the map threw "unsupported data type". The two entry points now share ChunkImpl.validateAndAdapt: - writeChunk(Map) adapts every column up front (boxed nullable array → NullableData, raw arrays pass through) before the row-count check and encoding, so the map form routes nullable columns through MaskedEncoding exactly like the builder. - validateAndAdapt is now package-private and idempotent: an already-adapted NullableData passes through, since the builder pre-adapts before delegating to writeChunk(Map) (would otherwise double-adapt and reject NullableData). Tests: VortexWriterTest gains map-path accept (boxed Long[] with null) + reject (boxed on non-nullable) cases; the integration masked round-trip now drives the map form, verifying null preservation end-to-end through the JNI reader. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1370276 commit 4d18939

4 files changed

Lines changed: 75 additions & 9 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,8 +1031,8 @@ void javaWriter_rustReader_masked_nullableI64(@TempDir Path tmp) throws IOExcept
10311031
Long[] data = {10L, null, 30L, null, 50L, 60L};
10321032
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
10331033
var sut = VortexWriter.create(ch, schema, WriteOptions.defaults())) {
1034-
// When — boxed Long[] via the builder routes through the nullable → masked path
1035-
sut.writeChunk(c -> c.put("v", data));
1034+
// When — boxed Long[] via the map entry point routes through the nullable → masked path
1035+
sut.writeChunk(Map.of("v", data));
10361036
}
10371037

10381038
// Then — Rust reads a nullable BigInt vector; null positions survive, values round-trip

writer/src/main/java/io/github/dfa1/vortex/writer/ChunkImpl.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,25 @@ Map<String, Object> finish() {
4242
return data;
4343
}
4444

45-
private static Object validateAndAdapt(String column, DType dtype, Object value) {
45+
/// Validates a column's raw data against its schema dtype and adapts boxed nullable arrays
46+
/// (`Long[]`, `Integer[]`, `Boolean[]`, …) into the internal [NullableData] carrier. Shared by
47+
/// the builder ([#put]) and the map-based [VortexWriter#writeChunk(Map)] entry points so both
48+
/// accept the same shapes.
49+
///
50+
/// @param column the column name, for error messages
51+
/// @param dtype the column's declared schema type
52+
/// @param value the raw column data
53+
/// @return the adapted data: the original array for non-nullable primitives, a [NullableData]
54+
/// for boxed nullable arrays, or `value` unchanged for non-primitive carriers
55+
static Object validateAndAdapt(String column, DType dtype, Object value) {
4656
if (value == null) {
4757
throw new IllegalArgumentException("null array for column: " + column);
4858
}
59+
// Idempotent: an already-adapted NullableData (e.g. from the builder, which adapts before
60+
// delegating to VortexWriter.writeChunk(Map)) passes through so a second call is a no-op.
61+
if (value instanceof NullableData) {
62+
return value;
63+
}
4964
return switch (dtype) {
5065
case DType.Primitive p -> adaptPrimitive(column, p, value);
5166
case DType.Utf8 u -> adaptUtf8(column, u, value);

writer/src/main/java/io/github/dfa1/vortex/writer/VortexWriter.java

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -369,11 +369,29 @@ public void writeChunk(java.util.function.Consumer<Chunk> builder) throws IOExce
369369

370370
/// Write one chunk. Each column is encoded by the first registered encoder that accepts its dtype.
371371
///
372+
/// A nullable column may be supplied as a boxed array (`Long[]`, `Integer[]`, `Double[]`,
373+
/// `Boolean[]`, …) with `null` marking absent rows; it routes through `MaskedEncoding` just like
374+
/// the builder form. Non-nullable columns take the raw primitive array (`long[]`, `int[]`, …).
375+
///
372376
/// @param columns map from column name to typed array data
373377
/// @throws IOException if an I/O error occurs writing to the underlying channel
374378
/// @throws IllegalArgumentException if a schema column is missing from `columns`,
375379
/// or if column arrays disagree on row count
376380
public void writeChunk(Map<String, Object> columns) throws IOException {
381+
// Adapt each column up front so the map entry point accepts the same shapes as the
382+
// builder: boxed nullable arrays (Long[], Integer[], Boolean[], …) become NullableData,
383+
// raw primitive arrays pass through. Done before the row-count check so length validation
384+
// and encoding both see the normalized carrier.
385+
Map<String, Object> adapted = new LinkedHashMap<>();
386+
for (int i = 0; i < schema.fieldNames().size(); i++) {
387+
String colName = schema.fieldNames().get(i);
388+
Object data = columns.get(colName);
389+
if (data == null) {
390+
throw new IllegalArgumentException("missing column: " + colName);
391+
}
392+
adapted.put(colName, ChunkImpl.validateAndAdapt(colName, schema.fieldTypes().get(i), data));
393+
}
394+
377395
// Pre-validate row counts so a length mismatch is rejected with a clear error
378396
// before any data is serialised. Without this check, the writer would produce a
379397
// file whose column chunks claim different row counts — readable but logically
@@ -382,11 +400,7 @@ public void writeChunk(Map<String, Object> columns) throws IOException {
382400
String expectedFrom = null;
383401
for (int i = 0; i < schema.fieldNames().size(); i++) {
384402
String colName = schema.fieldNames().get(i);
385-
Object data = columns.get(colName);
386-
if (data == null) {
387-
throw new IllegalArgumentException("missing column: " + colName);
388-
}
389-
long len = rowCountForValidation(colName, columns.get(colName));
403+
long len = rowCountForValidation(colName, adapted.get(colName));
390404
if (expectedLen < 0) {
391405
expectedLen = len;
392406
expectedFrom = colName;
@@ -400,7 +414,7 @@ public void writeChunk(Map<String, Object> columns) throws IOException {
400414
for (int i = 0; i < schema.fieldNames().size(); i++) {
401415
String colName = schema.fieldNames().get(i);
402416
DType colDtype = schema.fieldTypes().get(i);
403-
Object data = columns.get(colName);
417+
Object data = adapted.get(colName);
404418

405419
// Auto-route extension columns: callers can pass List<LocalDate>, List<Instant>,
406420
// etc., and we route through the matching spec extension to produce the int[] /

writer/src/test/java/io/github/dfa1/vortex/writer/VortexWriterTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import java.io.IOException;
1414
import java.nio.channels.FileChannel;
15+
import java.nio.file.Files;
1516
import java.nio.file.Path;
1617
import java.nio.file.StandardOpenOption;
1718
import java.util.ArrayList;
@@ -85,6 +86,42 @@ void writeChunk_autoroutesExtensionCollectionViaSpecExtension(@TempDir Path tmp)
8586
}
8687
}
8788

89+
@Test
90+
void writeChunk_map_nullablePrimitive_acceptsBoxedArray(@TempDir Path tmp) throws IOException {
91+
// Given — nullable I64 column passed to the MAP entry point as a boxed Long[] with a null.
92+
// Regression: the map path used to reject boxed arrays ("unsupported data type: Long[]");
93+
// only the builder accepted them. Both now share ChunkImpl.validateAndAdapt, so the map
94+
// form routes the column through nullable → MaskedEncoding. The null round-trip itself is
95+
// asserted end-to-end (through the JNI reader) by the integration masked test.
96+
var schema = new DType.Struct(List.of("v"),
97+
List.of(new DType.Primitive(PType.I64, true)), false);
98+
Path file = tmp.resolve("nullable_map.vtx");
99+
100+
// When
101+
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
102+
var sut = VortexWriter.create(ch, schema, WriteOptions.defaults())) {
103+
sut.writeChunk(Map.of("v", new Long[]{10L, null, 30L}));
104+
}
105+
106+
// Then — the masked file is well-formed
107+
assertThat(Files.size(file)).isPositive();
108+
}
109+
110+
@Test
111+
void writeChunk_map_nonNullablePrimitive_rejectsBoxedArray(@TempDir Path tmp) throws IOException {
112+
// Given — a non-nullable I64 column rejects a boxed array on the map path, same as the
113+
// builder: boxed implies nullability, which the schema does not allow.
114+
Path file = tmp.resolve("err.vtx");
115+
try (var ch = FileChannel.open(file, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
116+
var sut = VortexWriter.create(ch, SCHEMA, WriteOptions.defaults())) {
117+
// When / Then
118+
assertThatThrownBy(() -> sut.writeChunk(Map.of("id", new Long[]{1L, 2L})))
119+
.isInstanceOf(IllegalArgumentException.class)
120+
.hasMessageContaining("non-nullable")
121+
.hasMessageContaining("id");
122+
}
123+
}
124+
88125
@Test
89126
void writeChunk_roundTripsTimeExtension(@TempDir Path tmp) throws IOException {
90127
// Given — milliseconds resolution exercises the I32 storage branch of

0 commit comments

Comments
 (0)