From 4b77b356a95d58a7e2e4f72e7f7140a16a198cf2 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Fri, 23 Oct 2026 18:07:25 +0200 Subject: [PATCH 1/3] fix(cli): filter nullable columns filter and select --where compared a row only against unmasked arrays, so any nullable column (a CSV import with an empty cell, a nullable Parquet column) failed with "filter not supported for column type: MaskedArray". A comparison now skips a null row, as SQL does (comparing with null is not true, for != as well), and compares the others through the masked column's values. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 3 ++ .../github/dfa1/vortex/cli/FilterCommand.java | 29 ++++++++++++++----- .../dfa1/vortex/cli/FilterCommandTest.java | 24 +++++++++++++++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 244776915..806a59c01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- `filter` and `select --where` on the command line work on a nullable column, where they failed; a null row matches no comparison, as in SQL. ([#593](https://github.com/dfa1/vortex-java/pull/593)) + ## [0.18.1] — 2026-10-22 ### Added diff --git a/cli/src/main/java/io/github/dfa1/vortex/cli/FilterCommand.java b/cli/src/main/java/io/github/dfa1/vortex/cli/FilterCommand.java index c8bd3bd07..ec0b11a1b 100644 --- a/cli/src/main/java/io/github/dfa1/vortex/cli/FilterCommand.java +++ b/cli/src/main/java/io/github/dfa1/vortex/cli/FilterCommand.java @@ -29,6 +29,7 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.List; +import java.util.function.IntPredicate; @SuppressWarnings("java:S106") // CLI command: stdout is the intended output channel final class FilterCommand { @@ -188,14 +189,12 @@ static RowPredicate toRowPredicate(RowFilter filter) { /// the CLI grammar never reaches — can be exercised directly. static RowPredicate columnPredicate(ColumnName col, Predicate predicate) { return switch (predicate) { - case Predicate.Gt(var val) -> (chunk, rowIdx) -> compareValue(chunk.column(col), rowIdx, val) > 0; - case Predicate.Gte(var val) -> (chunk, rowIdx) -> compareValue(chunk.column(col), rowIdx, val) >= 0; - case Predicate.Lt(var val) -> (chunk, rowIdx) -> compareValue(chunk.column(col), rowIdx, val) < 0; - case Predicate.Lte(var val) -> (chunk, rowIdx) -> compareValue(chunk.column(col), rowIdx, val) <= 0; - case Predicate.Eq(var val) -> - (chunk, rowIdx) -> compareValue(chunk.column(col), rowIdx, (Comparable) val) == 0; - case Predicate.Neq(var val) -> - (chunk, rowIdx) -> compareValue(chunk.column(col), rowIdx, (Comparable) val) != 0; + case Predicate.Gt(var val) -> comparison(col, val, order -> order > 0); + case Predicate.Gte(var val) -> comparison(col, val, order -> order >= 0); + case Predicate.Lt(var val) -> comparison(col, val, order -> order < 0); + case Predicate.Lte(var val) -> comparison(col, val, order -> order <= 0); + case Predicate.Eq(var val) -> comparison(col, (Comparable) val, order -> order == 0); + case Predicate.Neq(var val) -> comparison(col, (Comparable) val, order -> order != 0); case Predicate.IsNull _ -> (chunk, rowIdx) -> isRowNull(chunk.column(col), rowIdx); case Predicate.IsNotNull _ -> (chunk, rowIdx) -> !isRowNull(chunk.column(col), rowIdx); case Predicate.Between _ -> @@ -207,6 +206,20 @@ static RowPredicate columnPredicate(ColumnName col, Predicate predicate) { }; } + /// A comparison against `value`, true when `test` accepts the row's order against it. A null row + /// never matches, as in SQL, where comparing with null is not true; a nullable column's other + /// rows compare through its unmasked values (a nullable column reached here as a `MaskedArray` + /// and failed with "filter not supported for column type"). + private static RowPredicate comparison(ColumnName col, Comparable value, IntPredicate test) { + return (chunk, rowIdx) -> { + Array arr = chunk.column(col); + if (isRowNull(arr, rowIdx)) { + return false; + } + return test.test(compareValue(MaskedArray.innerOrSelf(arr), rowIdx, value)); + }; + } + // Only a masked column carries nulls; an unmasked array is null-free, so every row is non-null. private static boolean isRowNull(Array arr, long rowIdx) { return arr instanceof MaskedArray masked && !masked.isValid(rowIdx); diff --git a/cli/src/test/java/io/github/dfa1/vortex/cli/FilterCommandTest.java b/cli/src/test/java/io/github/dfa1/vortex/cli/FilterCommandTest.java index ea9b06f9d..645a66303 100644 --- a/cli/src/test/java/io/github/dfa1/vortex/cli/FilterCommandTest.java +++ b/cli/src/test/java/io/github/dfa1/vortex/cli/FilterCommandTest.java @@ -27,6 +27,7 @@ import java.nio.channels.FileChannel; import java.nio.file.StandardOpenOption; import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.stream.Stream; @@ -214,6 +215,29 @@ void filtersF16Column_keepsOnlyMatchingRows() throws IOException { assertThat(result.stdout().lines().toList()).containsExactly("h", "2.5", "3.5"); } + @ParameterizedTest + @CsvSource({"'>', 0, '1,3'", "'!=', 1, '3'", "'=', 3, '3'"}) + void filtersNullableColumn_skipsNullRows(String op, String value, String expected) throws IOException { + // Given a nullable I64 column [1, null, 3], as a CSV import with an empty cell writes it: + // filtering one threw "filter not supported for column type: MaskedArray" + Path nullable = tmp.resolve("nullable.vortex"); + DType.Struct schema = new DType.Struct(List.of(ColumnName.of("v")), List.of(DType.I64.asNullable()), false); + try (FileChannel ch = FileChannel.open(nullable, StandardOpenOption.CREATE, StandardOpenOption.WRITE); + VortexWriter writer = VortexWriter.create(ch, schema, WriteOptions.defaults())) { + writer.writeChunk(Map.of(ColumnName.of("v"), new Long[]{1L, null, 3L})); + } + + // When + CliTestSupport.Captured result = capture(() -> + FilterCommand.run(new String[]{"filter", nullable.toString(), "v", op, value})); + + // Then: as in SQL, a comparison with null is never true, so the null row never matches + assertThat(result.status()).isEqualTo(ExitStatus.OK); + List rows = new ArrayList<>(List.of("v")); + rows.addAll(List.of(expected.split(","))); + assertThat(result.stdout().lines().toList()).isEqualTo(rows); + } + @Test void operatorPrefixAttack_doesNotCauseBacktracking() { // Given — input designed to trigger polynomial backtracking under the previous regex From 1cc4b0fd9515bc6b7222756f446530412361d708 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Fri, 23 Oct 2026 18:07:33 +0200 Subject: [PATCH 2/3] fix(csv): store empty cells as null, not 0 or false (#592) An empty CSV cell in a number or boolean column was stored as 0, 0.0 or false, indistinguishable from a real value, and a column empty in every row became non-nullable I64 zeros. Inferred number and boolean columns are now nullable and store an empty cell as null; an inferred text column keeps it as "", as Arrow's CSV reader does by default. With an explicit schema a nullable column gets null (text too) and a non-nullable number or boolean column fails on an empty cell instead of storing a value. Every inferred column is nullable, not only those with an empty cell among the first chunk's rows, as Arrow and DuckDB infer them: an empty cell later in the file is then a null instead of a failed import. On a 2.7 GB CSV without empty cells that costs +0.008% file size and a few percent of write CPU. CsvEmptyCellsInteropIntegrationTest reads an imported file with vortex-jni, full and filtered, and finds nulls exactly where the cells were empty. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 3 + README.md | 3 +- .../github/dfa1/vortex/csv/CsvImporter.java | 100 +++++++++--- .../dfa1/vortex/csv/CsvImporterHttpTest.java | 4 +- .../dfa1/vortex/csv/CsvImporterTest.java | 154 +++++++++++++++++- docs/reference.md | 5 + .../CsvEmptyCellsInteropIntegrationTest.java | 123 ++++++++++++++ 7 files changed, 361 insertions(+), 31 deletions(-) create mode 100644 integration/src/test/java/io/github/dfa1/vortex/integration/CsvEmptyCellsInteropIntegrationTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 806a59c01..bf4b0891c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **Breaking:** CSV import stores an empty cell in a number or boolean column as null instead of `0` or `false`, so inferred number and boolean columns are now nullable (read through `MaskedArray`), and an empty cell or a missing column fails the import where the schema declares the column non-nullable. ([#592](https://github.com/dfa1/vortex-java/issues/592)) + ### Fixed - `filter` and `select --where` on the command line work on a nullable column, where they failed; a null row matches no comparison, as in SQL. ([#593](https://github.com/dfa1/vortex-java/pull/593)) diff --git a/README.md b/README.md index d93f8d372..8708e9012 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,8 @@ would otherwise guess. Working *on* this repository instead? Read [AGENTS.md](AG - **Column types are inferred from the first 65 536 rows** (long, then double, then boolean, else text). A later value that does not fit fails the import, so for a column that mixes types pass an explicit schema: `ImportOptions.defaults().withSchema(DType.structBuilder()...build())`. - Empty cells become `0`, `false` or `""`; the first row is a header unless + Inferred number and boolean columns are nullable: an empty cell is null there, and `""` in a text + column. The first row is a header unless `withHeader(false)`; the delimiter must be an ASCII character. - **Compact files from code**: start from the import's own write options, which keep the global dictionary off for streaming (`options.withWriteOptions(options.writeOptions().withCompact(true))`), diff --git a/csv/src/main/java/io/github/dfa1/vortex/csv/CsvImporter.java b/csv/src/main/java/io/github/dfa1/vortex/csv/CsvImporter.java index d68a03d80..741053ae0 100644 --- a/csv/src/main/java/io/github/dfa1/vortex/csv/CsvImporter.java +++ b/csv/src/main/java/io/github/dfa1/vortex/csv/CsvImporter.java @@ -5,6 +5,7 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.PType; import io.github.dfa1.vortex.writer.VortexWriter; +import io.github.dfa1.vortex.writer.encode.NullableData; import java.io.IOException; import java.io.InputStream; @@ -38,7 +39,9 @@ /// /// Column types are inferred in priority order: long → double → boolean → utf8. /// Provide a schema via [ImportOptions#withSchema] to skip inference. -/// Empty cell values are treated as 0 / false / "" for typed columns. +/// An empty cell is a missing value: inferred number and boolean columns are nullable and store it +/// as null; an inferred text column keeps it as `""`. With an explicit schema, an empty cell in a +/// nullable column is null, and in a non-nullable number or boolean column fails the import. /// /// Import is single-pass streaming. The first [ImportOptions#chunkSize()] data rows /// are buffered to infer the schema (or skipped when a schema is provided), then @@ -314,7 +317,16 @@ private static DType.Struct inferSchemaFromRows(String[] headers, ColumnChunk ro List names = Arrays.stream(headers).map(ColumnName::of).toList(); List types = new ArrayList<>(colCount); for (int c = 0; c < colCount; c++) { - types.add(resolveType(canBeLong[c], canBeDouble[c], canBeBool[c])); + DType type = resolveType(canBeLong[c], canBeDouble[c], canBeBool[c]); + // Every inferred number or boolean column is nullable, as Arrow's and DuckDB's CSV + // readers make them: an empty cell anywhere in the file, not only in the rows types + // are inferred from, is then a null rather than a failed import. A column without + // nulls pays next to nothing for it (+0.008% file size on a 2.7 GB import). Text keeps + // an empty cell as "", as Arrow does by default. + if (!(type instanceof DType.Utf8)) { + type = type.asNullable(); + } + types.add(type); } return new DType.Struct(names, types, false); } @@ -377,7 +389,8 @@ static Map buildChunk(DType.Struct schema, ColumnChunk rows, for (int c = 0; c < schema.fieldNames().size(); c++) { DType dtype = schema.fieldTypes().get(c); int column = c; - columns.add(CompletableFuture.supplyAsync(() -> buildColumn(dtype, rows, column), executor)); + String name = schema.fieldNames().get(c).value(); + columns.add(CompletableFuture.supplyAsync(() -> buildColumn(dtype, name, rows, column), executor)); } Map chunk = new LinkedHashMap<>(); for (int c = 0; c < columns.size(); c++) { @@ -390,30 +403,51 @@ static Map buildChunk(DType.Struct schema, ColumnChunk rows, /// ASCII numbers are parsed in place, anything else falls back to `Long.parseLong` and friends /// on the decoded text, so their results, and their errors, are unchanged. A column the file /// lacks (a schema with more fields than the CSV) is all empty fields. - private static Object buildColumn(DType dtype, ColumnChunk rows, int column) { + /// + /// An empty field is a missing value: null in a nullable column, `""` in a non-nullable text + /// column, and an error in a non-nullable number or boolean column, which has no value to + /// stand in for it (#592: it was stored as `0` or `false`). + private static Object buildColumn(DType dtype, String name, ColumnChunk rows, int column) { int n = rows.rows(); byte[] bytes = column < rows.columns() ? rows.bytes(column) : null; + // text carries its nulls in the byte[][] itself, so only numbers and booleans need a mask + boolean[] valid = dtype.nullable() && !(dtype instanceof DType.Utf8) ? new boolean[n] : null; return switch (dtype) { case DType.Primitive p when p.ptype() == PType.I64 -> { long[] arr = new long[n]; - for (int i = 0; bytes != null && i < n; i++) { + for (int i = 0; i < n; i++) { + if (isEmpty(bytes, rows, column, i)) { + requireNullable(valid, name, bytes); + continue; + } arr[i] = parseLong(bytes, rows.start(column, i), rows.end(column, i)); + markValid(valid, i); } - yield arr; + yield valid == null ? arr : new NullableData(arr, valid); } case DType.Primitive p when p.ptype() == PType.F64 -> { double[] arr = new double[n]; - for (int i = 0; bytes != null && i < n; i++) { + for (int i = 0; i < n; i++) { + if (isEmpty(bytes, rows, column, i)) { + requireNullable(valid, name, bytes); + continue; + } arr[i] = parseDouble(bytes, rows.start(column, i), rows.end(column, i)); + markValid(valid, i); } - yield arr; + yield valid == null ? arr : new NullableData(arr, valid); } case DType.Bool _ -> { boolean[] arr = new boolean[n]; - for (int i = 0; bytes != null && i < n; i++) { + for (int i = 0; i < n; i++) { + if (isEmpty(bytes, rows, column, i)) { + requireNullable(valid, name, bytes); + continue; + } arr[i] = parseBoolean(bytes, rows.start(column, i), rows.end(column, i)); + markValid(valid, i); } - yield arr; + yield valid == null ? arr : new NullableData(arr, valid); } case DType.Utf8 _ -> { byte[][] arr = new byte[n][]; @@ -421,8 +455,11 @@ private static Object buildColumn(DType dtype, ColumnChunk rows, int column) { // is usually all ASCII, which is always well-formed UTF-8 boolean ascii = bytes == null || n == 0 || isAscii(bytes, 0, rows.end(column, n - 1)); for (int i = 0; i < n; i++) { - if (bytes == null) { - arr[i] = EMPTY; + if (isEmpty(bytes, rows, column, i)) { + // a nullable column's null stays null in the byte[][], as the writer expects + if (!dtype.nullable()) { + arr[i] = EMPTY; + } } else if (ascii) { arr[i] = Arrays.copyOfRange(bytes, rows.start(column, i), rows.end(column, i)); } else { @@ -435,14 +472,35 @@ private static Object buildColumn(DType dtype, ColumnChunk rows, int column) { }; } + /// Whether row `i` of `column` is empty, or the file lacks the column (`bytes` is `null`). + private static boolean isEmpty(byte[] bytes, ColumnChunk rows, int column, int i) { + return bytes == null || rows.start(column, i) == rows.end(column, i); + } + + private static void markValid(boolean[] valid, int i) { + if (valid != null) { + valid[i] = true; + } + } + + /// Rejects an empty field in a non-nullable number or boolean column (`valid` is `null`), + /// naming a column the file lacks (`bytes` is `null`) as missing rather than as empty. + private static void requireNullable(boolean[] valid, String name, byte[] bytes) { + if (valid == null && bytes == null) { + throw new IllegalArgumentException("the CSV has no column for '" + name + "', but its type in the" + + " schema is not nullable"); + } + if (valid == null) { + throw new IllegalArgumentException("column '" + name + "' has an empty cell, but its type in the" + + " schema is not nullable"); + } + } + private static final byte[] EMPTY = new byte[0]; - /// `Long.parseLong` on the field's text, `0` for an empty field. Up to 18 ASCII digits cannot - /// overflow a long, so they are summed in place; anything else takes the `String` path. + /// `Long.parseLong` on the field's non-empty text. Up to 18 ASCII digits cannot overflow a + /// long, so they are summed in place; anything else takes the `String` path. static long parseLong(byte[] b, int from, int to) { - if (from == to) { - return 0L; - } int i = from; boolean negative = b[i] == '-'; if (negative || b[i] == '+') { @@ -480,9 +538,6 @@ static boolean parseBoolean(byte[] b, int from, int to) { /// The field's bytes as a Utf8 value: copied when well-formed UTF-8, else decoded and encoded /// back, which turns each malformed sequence into U+FFFD as FastCSV's decoding did. static byte[] utf8(byte[] b, int from, int to) { - if (from == to) { - return EMPTY; - } if (isAscii(b, from, to) || Utf8Validation.isWellFormed(b, from, to)) { return Arrays.copyOfRange(b, from, to); } @@ -509,16 +564,13 @@ static boolean isAscii(byte[] b, int from, int to) { 1e0, 1e1, 1e2, 1e3, 1e4, 1e5, 1e6, 1e7, 1e8, 1e9, 1e10, 1e11, 1e12, 1e13, 1e14, 1e15, 1e16, 1e17, 1e18, 1e19, 1e20, 1e21, 1e22}; - /// `Double.parseDouble` on the field's text, `0.0` for an empty field. + /// `Double.parseDouble` on the field's non-empty text. /// /// A plain decimal (`-12.345`) whose digits fit 2^53 is computed as `digits / 10^fractionDigits`: /// both operands are exact doubles and IEEE division rounds correctly, so the result is the /// correctly rounded value `parseDouble` returns (Clinger's fast path). Anything else (an /// exponent, more digits, `NaN`, spaces, a bare `.5`) takes `parseDouble` itself. static double parseDouble(byte[] b, int from, int to) { - if (from == to) { - return 0.0; - } int i = from; boolean negative = b[i] == '-'; if (negative || b[i] == '+') { diff --git a/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterHttpTest.java b/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterHttpTest.java index 3a8652aee..cb944d771 100644 --- a/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterHttpTest.java +++ b/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterHttpTest.java @@ -7,6 +7,7 @@ import io.github.dfa1.vortex.reader.ScanOptions; import io.github.dfa1.vortex.reader.VortexReader; import io.github.dfa1.vortex.reader.array.LongArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; import io.github.dfa1.vortex.reader.array.VarBinArray; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -68,7 +69,8 @@ void importsFromUrl_streamsBodyIntoVortex(@TempDir Path tmp) throws Exception { assertThat(iter.hasNext()).isTrue(); try (Chunk chunk = iter.next()) { assertThat(chunk.rowCount()).isEqualTo(2); - LongArray ids = chunk.column(ColumnName.of("id")); + // inferred numbers are nullable (#592), so the ids come back masked + LongArray ids = (LongArray) MaskedArray.unwrap(chunk.column(ColumnName.of("id"))).inner(); assertThat(ids.getLong(0)).isEqualTo(1L); VarBinArray names = chunk.column(ColumnName.of("name")); assertThat(names.getString(0)).isEqualTo("Alice"); diff --git a/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java b/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java index 03680b2df..767744e4b 100644 --- a/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java +++ b/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java @@ -3,6 +3,7 @@ import io.github.dfa1.vortex.core.model.ColumnName; import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.reader.array.LongArray; +import io.github.dfa1.vortex.reader.array.MaskedArray; import io.github.dfa1.vortex.reader.array.VarBinArray; import io.github.dfa1.vortex.reader.VortexReader; import io.github.dfa1.vortex.reader.Chunk; @@ -12,6 +13,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import java.io.ByteArrayInputStream; @@ -22,6 +24,7 @@ import java.util.concurrent.ForkJoinPool; import java.util.function.DoubleSupplier; import java.util.function.LongSupplier; +import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; @@ -43,16 +46,17 @@ void infersTypedColumnsAndRoundTrips(@TempDir Path tmp) throws Exception { assertThat(reader.dtype()).isInstanceOf(DType.Struct.class); DType.Struct schema = (DType.Struct) reader.dtype(); assertThat(schema.fieldNames().stream().map(ColumnName::value).toList()).containsExactly("id", "price", "active", "name"); - assertThat(schema.fieldTypes().get(0)).isEqualTo(DType.I64); - assertThat(schema.fieldTypes().get(1)).isEqualTo(DType.F64); - assertThat(schema.fieldTypes().get(2)).isEqualTo(DType.BOOL); + // inferred numbers and booleans are nullable, text is not (#592) + assertThat(schema.fieldTypes().get(0)).isEqualTo(DType.I64.asNullable()); + assertThat(schema.fieldTypes().get(1)).isEqualTo(DType.F64.asNullable()); + assertThat(schema.fieldTypes().get(2)).isEqualTo(DType.BOOL.asNullable()); assertThat(schema.fieldTypes().get(3)).isEqualTo(DType.UTF8); try (ScanIterator iter = reader.scan(ScanOptions.all())) { assertThat(iter.hasNext()).isTrue(); try (Chunk chunk = iter.next()) { assertThat(chunk.rowCount()).isEqualTo(2); - LongArray ids = chunk.column(ColumnName.of("id")); + LongArray ids = (LongArray) MaskedArray.unwrap(chunk.column(ColumnName.of("id"))).inner(); assertThat(ids.getLong(0)).isEqualTo(1L); assertThat(ids.getLong(1)).isEqualTo(2L); VarBinArray names = chunk.column(ColumnName.of("name")); @@ -125,6 +129,141 @@ void respectsSchemaOverride(@TempDir Path tmp) throws Exception { } } + @Test + void emptyCellsInNumberAndBooleanColumnsAreNull(@TempDir Path tmp) throws Exception { + // Given: qty and active have one empty cell, bc is empty everywhere, name has an empty + // text cell (#592: the numbers and booleans were stored as 0 and false) + String text = "id,qty,active,name,bc\n1,5,true,a,\n2,,,,\n3,0,false,c,\n"; + Path csv = tmp.resolve("empties.csv"); + Files.writeString(csv, text); + Path vortex = tmp.resolve("empties.vortex"); + Path exported = tmp.resolve("exported.csv"); + + // When + CsvImporter.importCsv(csv, vortex); + CsvExporter.exportCsv(vortex, exported, ExportOptions.defaults()); + + // Then: numbers and booleans nullable, text keeps "", and a null exports as an empty cell + // again, so the file round-trips where zeros would have read back as "0" + try (VortexReader reader = VortexReader.open(vortex)) { + DType.Struct schema = (DType.Struct) reader.dtype(); + assertThat(schema.fieldTypes()).containsExactly(DType.I64.asNullable(), DType.I64.asNullable(), + DType.BOOL.asNullable(), DType.UTF8, DType.I64.asNullable()); + } + assertThat(Files.readAllLines(exported)).isEqualTo(text.lines().toList()); + } + + @Test + void anEmptyCellAfterTheInferredRowsIsNull(@TempDir Path tmp) throws Exception { + // Given: qty has no empty cell in the rows types are inferred from, then one in the last + // row; it must still be a null, not a 0 and not a failed import + int rows = ImportOptions.defaults().chunkSize() + 10; + StringBuilder text = new StringBuilder("id,qty\n"); + for (int i = 0; i < rows; i++) { + text.append(i).append(',').append(i == rows - 1 ? "" : String.valueOf(i % 9)).append('\n'); + } + Path csv = tmp.resolve("late-empty.csv"); + Files.writeString(csv, text); + Path vortex = tmp.resolve("late-empty.vortex"); + Path exported = tmp.resolve("late-empty-exported.csv"); + + // When + CsvImporter.importCsv(csv, vortex); + CsvExporter.exportCsv(vortex, exported, ExportOptions.defaults()); + + // Then + assertThat(Files.readAllLines(exported)).isEqualTo(text.toString().lines().toList()); + } + + @ParameterizedTest + @MethodSource("numberAndBooleanTypes") + void anEmptyCellInANonNullableSchemaColumnFails(DType type, @TempDir Path tmp) throws Exception { + // Given: a quoted "" is an empty cell (a bare empty line in a one-column file is skipped); + // a non-nullable number or boolean has no value to store for it (#592: it stored 0 or false) + Path csv = tmp.resolve("schema.csv"); + Files.writeString(csv, "qty\n1\n\"\"\n"); + ImportOptions options = ImportOptions.defaults().withSchema(DType.structBuilder() + .field(ColumnName.of("qty"), type) + .build()); + + // When + Throwable result = catchThrowable(() -> CsvImporter.importCsv(csv, tmp.resolve("schema.vortex"), options)); + + // Then + assertThat(result).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("column 'qty' has an empty cell"); + } + + @ParameterizedTest + @MethodSource("numberAndBooleanTypes") + void aSchemaColumnTheFileLacksIsNullWhenNullable(DType type, @TempDir Path tmp) throws Exception { + // Given: the schema has one more column than the file, which is empty in every row + Path csv = tmp.resolve("short.csv"); + Files.writeString(csv, "id\n1\n2\n"); + Path vortex = tmp.resolve("short.vortex"); + ImportOptions options = ImportOptions.defaults().withSchema(DType.structBuilder() + .field(ColumnName.of("id"), DType.I64) + .field(ColumnName.of("extra"), type.asNullable()) + .build()); + + // When + CsvImporter.importCsv(csv, vortex, options); + + // Then: every row of the extra column is null + try (VortexReader reader = VortexReader.open(vortex); + ScanIterator iter = reader.scan(ScanOptions.all()); + Chunk chunk = iter.next()) { + MaskedArray.Unwrapped result = MaskedArray.unwrap(chunk.column(ColumnName.of("extra"))); + assertThat(result.validity()).isNotNull(); + assertThat(result.validity().getBoolean(0)).isFalse(); + assertThat(result.validity().getBoolean(1)).isFalse(); + } + } + + @ParameterizedTest + @MethodSource("numberAndBooleanTypes") + void aSchemaColumnTheFileLacksFailsWhenNotNullable(DType type, @TempDir Path tmp) throws Exception { + // Given: before #592 the missing column was written as zeros or false + Path csv = tmp.resolve("short.csv"); + Files.writeString(csv, "id\n1\n2\n"); + ImportOptions options = ImportOptions.defaults().withSchema(DType.structBuilder() + .field(ColumnName.of("id"), DType.I64) + .field(ColumnName.of("extra"), type) + .build()); + + // When + Throwable result = catchThrowable(() -> CsvImporter.importCsv(csv, tmp.resolve("short.vortex"), options)); + + // Then + assertThat(result).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("the CSV has no column for 'extra'"); + } + + @Test + void aNullableTextSchemaStoresEmptyCellsAsNull(@TempDir Path tmp) throws Exception { + // Given: inferred text keeps "", but a schema asking for nullable text gets null + Path csv = tmp.resolve("text.csv"); + Files.writeString(csv, "id,name\n1,a\n2,\n"); + Path vortex = tmp.resolve("text.vortex"); + ImportOptions options = ImportOptions.defaults().withSchema(DType.structBuilder() + .field(ColumnName.of("id"), DType.I64) + .field(ColumnName.of("name"), DType.UTF8.asNullable()) + .build()); + + // When + CsvImporter.importCsv(csv, vortex, options); + + // Then + try (VortexReader reader = VortexReader.open(vortex); + ScanIterator iter = reader.scan(ScanOptions.all()); + Chunk chunk = iter.next()) { + MaskedArray.Unwrapped name = MaskedArray.unwrap(chunk.column(ColumnName.of("name"))); + assertThat(name.validity()).isNotNull(); + assertThat(name.validity().getBoolean(0)).isTrue(); + assertThat(name.validity().getBoolean(1)).isFalse(); + } + } + @Test void chunksWrittenInTheBackgroundKeepFileOrder(@TempDir Path tmp) throws Exception { // Given: two full chunks and a partial one, so each is converted while the next is parsed @@ -146,7 +285,7 @@ void chunksWrittenInTheBackgroundKeepFileOrder(@TempDir Path tmp) throws Excepti ScanIterator iter = reader.scan(ScanOptions.all())) { while (iter.hasNext()) { try (Chunk chunk = iter.next()) { - LongArray ids = chunk.column(ColumnName.of("id")); + LongArray ids = (LongArray) MaskedArray.unwrap(chunk.column(ColumnName.of("id"))).inner(); for (int i = 0; i < chunk.rowCount(); i++) { assertThat(ids.getLong(i)).isEqualTo(expected++); } @@ -314,4 +453,9 @@ private static Object parsed(LongSupplier parse) { return e.getClass().getName() + ": " + e.getMessage(); } } + + /// The column types that have no stand-in for an empty cell: every type but text. + private static Stream numberAndBooleanTypes() { + return Stream.of(DType.I64, DType.F64, DType.BOOL); + } } diff --git a/docs/reference.md b/docs/reference.md index 082af8887..2371e551c 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -549,6 +549,11 @@ own, so it is only ever an `import` *source*: never a destination (there is no C `import`), and never a source for the other subcommands, which all expect a Vortex or Parquet file. +An empty cell is a missing value. Inferred number and boolean columns are nullable and store it +as null; an inferred text column stores `""`. With a schema, a nullable column stores null (text +too), and a non-nullable number or boolean column fails the import with +`IllegalArgumentException`. A schema column the file lacks is empty in every row. + | Method | Notes | |-----------------------------------------------|------------------------------------------------------------| | `importCsv(Path in, Path out)` | Defaults | diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/CsvEmptyCellsInteropIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/CsvEmptyCellsInteropIntegrationTest.java new file mode 100644 index 000000000..2a27d5321 --- /dev/null +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/CsvEmptyCellsInteropIntegrationTest.java @@ -0,0 +1,123 @@ +package io.github.dfa1.vortex.integration; + +import dev.vortex.api.DataSource; +import dev.vortex.api.Expression; +import dev.vortex.api.Partition; +import dev.vortex.api.Scan; +import dev.vortex.api.ScanOptions; +import dev.vortex.api.Session; +import dev.vortex.arrow.ArrowAllocation; +import dev.vortex.jni.NativeLoader; +import io.github.dfa1.vortex.csv.CsvImporter; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.vector.FieldVector; +import org.apache.arrow.vector.VectorSchemaRoot; +import org.apache.arrow.vector.ipc.ArrowReader; +import org.apache.arrow.vector.util.Text; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/// An empty CSV cell in a numeric or boolean column is a missing value, so vortex-jni must read it +/// back as null (#592): the import stored `0`, `0.0` or `false` instead, indistinguishable from a +/// real zero. An inferred text column keeps its empty cells as `""`, as Arrow's CSV reader does. +class CsvEmptyCellsInteropIntegrationTest { + + private static final Session SESSION = Session.create(); + private static final BufferAllocator ALLOCATOR = ArrowAllocation.rootAllocator(); + private static final int ROWS = 3_000; + + static { + NativeLoader.loadJni(); + } + + @TempDir + Path tmp; + + @Test + void emptyCells_readAsNullByVortexJni() throws IOException { + // Given: every 7th row empty in each typed column, `bc` empty in every row, and enough rows + // for several zones, so the filtered read below prunes and slices + StringBuilder csv = new StringBuilder("id,qty,price,active,name,bc\n"); + for (int i = 0; i < ROWS; i++) { + boolean empty = i % 7 == 3; + csv.append(i).append(',') + .append(empty ? "" : String.valueOf(i % 50)).append(',') + .append(empty ? "" : (i % 50) + ".5").append(',') + .append(empty ? "" : String.valueOf(i % 2 == 0)).append(',') + .append(empty ? "" : "n" + i).append(',') + .append('\n'); + } + Path csvFile = tmp.resolve("empties.csv"); + Files.writeString(csvFile, csv); + Path vortex = tmp.resolve("empties.vortex"); + + // When + CsvImporter.importCsv(csvFile, vortex); + List> result = readJni(vortex, null); + List> filtered = readJni(vortex, Expression.binary(Expression.BinaryOp.GTE, + Expression.column("id"), Expression.literal(2_000L))); + // a filter on the nullable columns themselves goes through our zone stats for zones with + // nulls: a wrong min/max or null count prunes rows Rust should return, or keeps a null + List> byQty = readJni(vortex, Expression.binary(Expression.BinaryOp.GTE, + Expression.column("qty"), Expression.literal(25L))); + List> byAllNull = readJni(vortex, Expression.binary(Expression.BinaryOp.GTE, + Expression.column("bc"), Expression.literal(0L))); + + // Then + List> expected = new ArrayList<>(); + for (int i = 0; i < ROWS; i++) { + boolean empty = i % 7 == 3; + expected.add(Arrays.asList((long) i, + empty ? null : (long) (i % 50), + empty ? null : (i % 50) + 0.5, + empty ? null : i % 2 == 0, + empty ? "" : "n" + i, + null)); + } + assertThat(result).isEqualTo(expected); + assertThat(filtered).isEqualTo(expected.subList(2_000, ROWS)); + // a null qty never satisfies the comparison + assertThat(byQty).isEqualTo(expected.stream() + .filter(row -> row.get(1) instanceof Long qty && qty >= 25) + .toList()); + assertThat(byAllNull).isEmpty(); + } + + private static List> readJni(Path file, Expression filter) throws IOException { + String[] columns = {"id", "qty", "price", "active", "name", "bc"}; + var options = ScanOptions.builder() + .projection(Expression.select(columns, Expression.root())); + if (filter != null) { + options.filter(filter); + } + List> rows = new ArrayList<>(); + Scan scan = DataSource.open(SESSION, file.toAbsolutePath().toUri().toString()).scan(options.build()); + while (scan.hasNext()) { + Partition partition = scan.next(); + try (ArrowReader reader = partition.scanArrow(ALLOCATOR)) { + while (reader.loadNextBatch()) { + VectorSchemaRoot root = reader.getVectorSchemaRoot(); + for (int i = 0; i < root.getRowCount(); i++) { + List row = new ArrayList<>(); + for (String column : columns) { + FieldVector vector = root.getVector(column); + Object value = vector.isNull(i) ? null : vector.getObject(i); + row.add(value instanceof Text text ? text.toString() : value); + } + rows.add(row); + } + } + } + } + return rows; + } +} From 44e0ebb4db5ccdddeacc03d25ea1786775731a32 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Fri, 23 Oct 2026 21:14:16 +0200 Subject: [PATCH 3/3] feat(csv): narrow integer types and F32 in an import schema A CSV import schema could only use I64, F64, Bool and Utf8: any other type, U16 included, failed with UnsupportedOperationException on the first chunk, after the output file was created. Now every integer type from I8 to U64 and F32 is accepted. Integers parse through the same byte fast path as I64, are range-checked against the declared type (a cast would wrap 65536 to 0 in a U16), and are narrowed into the array the writer takes. A U64 above Long.MAX_VALUE goes through Long.parseUnsignedLong. F32 uses Float.parseFloat: rounding a parsed double to float rounds twice and can differ. withSchema rejects F16 and non-primitive types up front. CsvEmptyCellsInteropIntegrationTest becomes CsvImportInteropIntegrationTest and gains a case where vortex-jni reads every narrow type back with Arrow's unsigned types and the same values, full and filtered on an I16 column. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 3 + README.md | 3 +- .../github/dfa1/vortex/csv/CsvImporter.java | 79 ++++++- .../github/dfa1/vortex/csv/ImportOptions.java | 22 +- .../dfa1/vortex/csv/CsvImporterTest.java | 93 ++++++++ docs/reference.md | 3 + .../CsvEmptyCellsInteropIntegrationTest.java | 123 ---------- .../CsvImportInteropIntegrationTest.java | 213 ++++++++++++++++++ 8 files changed, 412 insertions(+), 127 deletions(-) delete mode 100644 integration/src/test/java/io/github/dfa1/vortex/integration/CsvEmptyCellsInteropIntegrationTest.java create mode 100644 integration/src/test/java/io/github/dfa1/vortex/integration/CsvImportInteropIntegrationTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index bf4b0891c..6ce9ac18f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- A CSV import schema can declare any integer type from `I8` to `U64`, range-checked per value, and `F32`; an unsupported type fails in `withSchema` instead of at the first chunk. ([#593](https://github.com/dfa1/vortex-java/pull/593)) + ### Changed - **Breaking:** CSV import stores an empty cell in a number or boolean column as null instead of `0` or `false`, so inferred number and boolean columns are now nullable (read through `MaskedArray`), and an empty cell or a missing column fails the import where the schema declares the column non-nullable. ([#592](https://github.com/dfa1/vortex-java/issues/592)) diff --git a/README.md b/README.md index 8708e9012..041cf4844 100644 --- a/README.md +++ b/README.md @@ -133,7 +133,8 @@ would otherwise guess. Working *on* this repository instead? Read [AGENTS.md](AG bounded memory; `ParquetImporter` and `JdbcImporter` do the same for Parquet and JDBC sources. - **Column types are inferred from the first 65 536 rows** (long, then double, then boolean, else text). A later value that does not fit fails the import, so for a column that mixes types pass - an explicit schema: `ImportOptions.defaults().withSchema(DType.structBuilder()...build())`. + an explicit schema: `ImportOptions.defaults().withSchema(DType.structBuilder()...build())`. A + schema can also pick a narrower type: any integer from `I8` to `U64` (range-checked) or `F32`. Inferred number and boolean columns are nullable: an empty cell is null there, and `""` in a text column. The first row is a header unless `withHeader(false)`; the delimiter must be an ASCII character. diff --git a/csv/src/main/java/io/github/dfa1/vortex/csv/CsvImporter.java b/csv/src/main/java/io/github/dfa1/vortex/csv/CsvImporter.java index 741053ae0..4eaf2ea19 100644 --- a/csv/src/main/java/io/github/dfa1/vortex/csv/CsvImporter.java +++ b/csv/src/main/java/io/github/dfa1/vortex/csv/CsvImporter.java @@ -413,14 +413,30 @@ private static Object buildColumn(DType dtype, String name, ColumnChunk rows, in // text carries its nulls in the byte[][] itself, so only numbers and booleans need a mask boolean[] valid = dtype.nullable() && !(dtype instanceof DType.Utf8) ? new boolean[n] : null; return switch (dtype) { - case DType.Primitive p when p.ptype() == PType.I64 -> { + case DType.Primitive p when p.ptype().isInteger() -> { long[] arr = new long[n]; for (int i = 0; i < n; i++) { if (isEmpty(bytes, rows, column, i)) { requireNullable(valid, name, bytes); continue; } - arr[i] = parseLong(bytes, rows.start(column, i), rows.end(column, i)); + arr[i] = parseInteger(p.ptype(), name, bytes, rows.start(column, i), rows.end(column, i)); + markValid(valid, i); + } + Object values = narrow(p.ptype(), arr); + yield valid == null ? values : new NullableData(values, valid); + } + case DType.Primitive p when p.ptype() == PType.F32 -> { + float[] arr = new float[n]; + for (int i = 0; i < n; i++) { + if (isEmpty(bytes, rows, column, i)) { + requireNullable(valid, name, bytes); + continue; + } + // ponytail: no byte fast path; rounding the double parse to float would round + // twice and can differ from Float.parseFloat, so F32 pays a String per value + int from = rows.start(column, i); + arr[i] = Float.parseFloat(new String(bytes, from, rows.end(column, i) - from, UTF_8)); markValid(valid, i); } yield valid == null ? arr : new NullableData(arr, valid); @@ -498,6 +514,65 @@ private static void requireNullable(boolean[] valid, String name, byte[] bytes) private static final byte[] EMPTY = new byte[0]; + /// The field's integer, checked to fit `ptype`; an unsigned value is returned as its bits, so + /// a `U64` above `Long.MAX_VALUE` is negative here. + static long parseInteger(PType ptype, String name, byte[] b, int from, int to) { + // more than 18 bytes may overflow a long, where only Long.parseUnsignedLong reads U64's top half + if (ptype == PType.U64 && to - from > 18) { + return Long.parseUnsignedLong(new String(b, from, to - from, UTF_8)); + } + long value = parseLong(b, from, to); + long min = switch (ptype) { + case I8 -> Byte.MIN_VALUE; + case I16 -> Short.MIN_VALUE; + case I32 -> Integer.MIN_VALUE; + case I64 -> Long.MIN_VALUE; + default -> 0; + }; + long max = switch (ptype) { + case I8 -> Byte.MAX_VALUE; + case U8 -> 0xFFL; + case I16 -> Short.MAX_VALUE; + case U16 -> 0xFFFFL; + case I32 -> Integer.MAX_VALUE; + case U32 -> 0xFFFF_FFFFL; + default -> Long.MAX_VALUE; + }; + if (value < min || value > max) { + throw new IllegalArgumentException("column '" + name + "' has " + value + ", which is out of range for " + + ptype); + } + return value; + } + + /// The parsed values in the array the writer takes for `ptype`; each value already fits. + private static Object narrow(PType ptype, long[] values) { + return switch (ptype) { + case I8, U8 -> { + byte[] out = new byte[values.length]; + for (int i = 0; i < values.length; i++) { + out[i] = (byte) values[i]; + } + yield out; + } + case I16, U16 -> { + short[] out = new short[values.length]; + for (int i = 0; i < values.length; i++) { + out[i] = (short) values[i]; + } + yield out; + } + case I32, U32 -> { + int[] out = new int[values.length]; + for (int i = 0; i < values.length; i++) { + out[i] = (int) values[i]; + } + yield out; + } + default -> values; + }; + } + /// `Long.parseLong` on the field's non-empty text. Up to 18 ASCII digits cannot overflow a /// long, so they are summed in place; anything else takes the `String` path. static long parseLong(byte[] b, int from, int to) { diff --git a/csv/src/main/java/io/github/dfa1/vortex/csv/ImportOptions.java b/csv/src/main/java/io/github/dfa1/vortex/csv/ImportOptions.java index 1316e8a4a..0ca873158 100644 --- a/csv/src/main/java/io/github/dfa1/vortex/csv/ImportOptions.java +++ b/csv/src/main/java/io/github/dfa1/vortex/csv/ImportOptions.java @@ -1,6 +1,7 @@ package io.github.dfa1.vortex.csv; import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.core.model.PType; import io.github.dfa1.vortex.writer.WriteOptions; /// Options controlling CSV → Vortex import. @@ -75,11 +76,21 @@ public static ImportOptions defaults() { } /// Override the inferred schema. The struct's field names become column names; - /// types control how each CSV column is parsed (positionally). + /// types control how each CSV column is parsed (positionally). A column may be any integer + /// type (`I8` to `U64`, range-checked per value), `F32`, `F64`, `Bool` or `Utf8`, nullable or + /// not. /// /// @param overrideSchema the schema to use instead of inference /// @return a copy of this options with the schema applied + /// @throws IllegalArgumentException if a column has a type CSV import cannot produce public ImportOptions withSchema(DType.Struct overrideSchema) { + for (int c = 0; c < overrideSchema.fieldTypes().size(); c++) { + DType type = overrideSchema.fieldTypes().get(c); + if (!isSupported(type)) { + throw new IllegalArgumentException("column '" + overrideSchema.fieldNames().get(c).value() + + "' has type " + type + ", which CSV import does not support"); + } + } return new ImportOptions(delimiter, chunkSize, header, overrideSchema, progressListener, writeOptions); } @@ -115,4 +126,13 @@ public ImportOptions withWriteOptions(WriteOptions options) { public ImportOptions withHeader(boolean header) { return new ImportOptions(delimiter, chunkSize, header, schema, progressListener, writeOptions); } + + /// The types `CsvImporter` parses text into: every primitive but `F16`, booleans and text. + private static boolean isSupported(DType type) { + return switch (type) { + case DType.Primitive p -> p.ptype() != PType.F16; + case DType.Bool _, DType.Utf8 _ -> true; + default -> false; + }; + } } diff --git a/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java b/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java index 767744e4b..bac3107a9 100644 --- a/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java +++ b/csv/src/test/java/io/github/dfa1/vortex/csv/CsvImporterTest.java @@ -2,6 +2,7 @@ import io.github.dfa1.vortex.core.model.ColumnName; import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.core.model.PType; import io.github.dfa1.vortex.reader.array.LongArray; import io.github.dfa1.vortex.reader.array.MaskedArray; import io.github.dfa1.vortex.reader.array.VarBinArray; @@ -13,6 +14,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; @@ -315,6 +317,97 @@ void aBadValueAfterTheFirstChunkFailsWithItsOwnException(@TempDir Path tmp) thro assertThat(result).isInstanceOf(NumberFormatException.class).hasMessageContaining("N/A"); } + @ParameterizedTest + @CsvSource({ + "I8, -128, -128", "I8, 127, 127", "U8, 255, 255", "I16, -32768, -32768", "U16, 65535, 65535", + "I32, -2147483648, -2147483648", "U32, 4294967295, 4294967295", + "U64, 18446744073709551615, -1", "U64, 9223372036854775808, -9223372036854775808", + "U64, 123, 123", "I64, -9223372036854775808, -9223372036854775808"}) + void parseInteger_acceptsEachTypesBounds(PType ptype, String text, long expected) { + // Given: the extremes of each type; a U64 above Long.MAX_VALUE comes back as its bits + byte[] bytes = text.getBytes(StandardCharsets.UTF_8); + + // When + long result = CsvImporter.parseInteger(ptype, "c", bytes, 0, bytes.length); + + // Then + assertThat(result).isEqualTo(expected); + } + + @ParameterizedTest + @CsvSource({"I8, -129", "I8, 128", "U8, -1", "U8, 256", "I16, 32768", "U16, 65536", "U16, -1", + "I32, 2147483648", "U32, 4294967296", "U32, -1", "U64, -1"}) + void parseInteger_rejectsAValueOutOfRange(PType ptype, String text) { + // Given: one past each bound, which a cast would silently wrap + byte[] bytes = text.getBytes(StandardCharsets.UTF_8); + + // When + Throwable result = catchThrowable(() -> CsvImporter.parseInteger(ptype, "c", bytes, 0, bytes.length)); + + // Then + assertThat(result).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("column 'c' has " + text + ", which is out of range for " + ptype); + } + + @Test + void parseInteger_rejectsANegativeLongU64() { + // Given: 19+ bytes take Long.parseUnsignedLong, which refuses a sign + byte[] bytes = "-9223372036854775808".getBytes(StandardCharsets.UTF_8); + + // When + Throwable result = catchThrowable(() -> CsvImporter.parseInteger(PType.U64, "c", bytes, 0, bytes.length)); + + // Then + assertThat(result).isInstanceOf(NumberFormatException.class); + } + + @Test + void aSchemaWithNarrowTypesStoresThem(@TempDir Path tmp) throws Exception { + // Given: every integer width, signed and not, and F32, with an empty cell in a nullable one + Path csv = tmp.resolve("narrow.csv"); + Files.writeString(csv, "a,b,c,d,e,f,g,h,i\n-1,255,-2,65535,-3,4294967295,-4,18446744073709551615,1.5\n" + + "1,,2,0,3,0,4,0,-0.25\n"); + Path vortex = tmp.resolve("narrow.vortex"); + DType.Struct schema = DType.structBuilder() + .field(ColumnName.of("a"), DType.I8) + .field(ColumnName.of("b"), DType.U8.asNullable()) + .field(ColumnName.of("c"), DType.I16) + .field(ColumnName.of("d"), DType.U16) + .field(ColumnName.of("e"), DType.I32) + .field(ColumnName.of("f"), DType.U32) + .field(ColumnName.of("g"), DType.I64) + .field(ColumnName.of("h"), DType.U64) + .field(ColumnName.of("i"), DType.F32) + .build(); + Path exported = tmp.resolve("narrow-exported.csv"); + + // When + CsvImporter.importCsv(csv, vortex, ImportOptions.defaults().withSchema(schema)); + CsvExporter.exportCsv(vortex, exported, ExportOptions.defaults()); + + // Then: the file keeps the schema's types, and every value, unsigned ones included, reads back + try (VortexReader reader = VortexReader.open(vortex)) { + assertThat(reader.dtype()).isEqualTo(schema); + } + assertThat(Files.readAllLines(exported)).isEqualTo(Files.readAllLines(csv)); + } + + @Test + void withSchemaRejectsATypeCsvCannotProduce() { + // Given: a type no CSV text parses into, refused when the schema is set, not at the first chunk + DType.Struct schema = DType.structBuilder() + .field(ColumnName.of("id"), DType.I64) + .field(ColumnName.of("half"), DType.F16) + .build(); + + // When + Throwable result = catchThrowable(() -> ImportOptions.defaults().withSchema(schema)); + + // Then + assertThat(result).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("column 'half'"); + } + @ParameterizedTest @ValueSource(strings = {"0", "-0", "+5", "-42", "123456789012345678", "1234567890123456789", "9223372036854775807", "-9223372036854775808", "9223372036854775808", "1_0", " 1", "1e3", diff --git a/docs/reference.md b/docs/reference.md index 2371e551c..86aabbe2d 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -549,6 +549,9 @@ own, so it is only ever an `import` *source*: never a destination (there is no C `import`), and never a source for the other subcommands, which all expect a Vortex or Parquet file. +A schema column may be any integer type (`I8` to `U64`; a value out of the type's range fails the +import), `F32`, `F64`, `Bool` or `Utf8`. `withSchema` rejects any other type. + An empty cell is a missing value. Inferred number and boolean columns are nullable and store it as null; an inferred text column stores `""`. With a schema, a nullable column stores null (text too), and a non-nullable number or boolean column fails the import with diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/CsvEmptyCellsInteropIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/CsvEmptyCellsInteropIntegrationTest.java deleted file mode 100644 index 2a27d5321..000000000 --- a/integration/src/test/java/io/github/dfa1/vortex/integration/CsvEmptyCellsInteropIntegrationTest.java +++ /dev/null @@ -1,123 +0,0 @@ -package io.github.dfa1.vortex.integration; - -import dev.vortex.api.DataSource; -import dev.vortex.api.Expression; -import dev.vortex.api.Partition; -import dev.vortex.api.Scan; -import dev.vortex.api.ScanOptions; -import dev.vortex.api.Session; -import dev.vortex.arrow.ArrowAllocation; -import dev.vortex.jni.NativeLoader; -import io.github.dfa1.vortex.csv.CsvImporter; -import org.apache.arrow.memory.BufferAllocator; -import org.apache.arrow.vector.FieldVector; -import org.apache.arrow.vector.VectorSchemaRoot; -import org.apache.arrow.vector.ipc.ArrowReader; -import org.apache.arrow.vector.util.Text; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; - -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; - -/// An empty CSV cell in a numeric or boolean column is a missing value, so vortex-jni must read it -/// back as null (#592): the import stored `0`, `0.0` or `false` instead, indistinguishable from a -/// real zero. An inferred text column keeps its empty cells as `""`, as Arrow's CSV reader does. -class CsvEmptyCellsInteropIntegrationTest { - - private static final Session SESSION = Session.create(); - private static final BufferAllocator ALLOCATOR = ArrowAllocation.rootAllocator(); - private static final int ROWS = 3_000; - - static { - NativeLoader.loadJni(); - } - - @TempDir - Path tmp; - - @Test - void emptyCells_readAsNullByVortexJni() throws IOException { - // Given: every 7th row empty in each typed column, `bc` empty in every row, and enough rows - // for several zones, so the filtered read below prunes and slices - StringBuilder csv = new StringBuilder("id,qty,price,active,name,bc\n"); - for (int i = 0; i < ROWS; i++) { - boolean empty = i % 7 == 3; - csv.append(i).append(',') - .append(empty ? "" : String.valueOf(i % 50)).append(',') - .append(empty ? "" : (i % 50) + ".5").append(',') - .append(empty ? "" : String.valueOf(i % 2 == 0)).append(',') - .append(empty ? "" : "n" + i).append(',') - .append('\n'); - } - Path csvFile = tmp.resolve("empties.csv"); - Files.writeString(csvFile, csv); - Path vortex = tmp.resolve("empties.vortex"); - - // When - CsvImporter.importCsv(csvFile, vortex); - List> result = readJni(vortex, null); - List> filtered = readJni(vortex, Expression.binary(Expression.BinaryOp.GTE, - Expression.column("id"), Expression.literal(2_000L))); - // a filter on the nullable columns themselves goes through our zone stats for zones with - // nulls: a wrong min/max or null count prunes rows Rust should return, or keeps a null - List> byQty = readJni(vortex, Expression.binary(Expression.BinaryOp.GTE, - Expression.column("qty"), Expression.literal(25L))); - List> byAllNull = readJni(vortex, Expression.binary(Expression.BinaryOp.GTE, - Expression.column("bc"), Expression.literal(0L))); - - // Then - List> expected = new ArrayList<>(); - for (int i = 0; i < ROWS; i++) { - boolean empty = i % 7 == 3; - expected.add(Arrays.asList((long) i, - empty ? null : (long) (i % 50), - empty ? null : (i % 50) + 0.5, - empty ? null : i % 2 == 0, - empty ? "" : "n" + i, - null)); - } - assertThat(result).isEqualTo(expected); - assertThat(filtered).isEqualTo(expected.subList(2_000, ROWS)); - // a null qty never satisfies the comparison - assertThat(byQty).isEqualTo(expected.stream() - .filter(row -> row.get(1) instanceof Long qty && qty >= 25) - .toList()); - assertThat(byAllNull).isEmpty(); - } - - private static List> readJni(Path file, Expression filter) throws IOException { - String[] columns = {"id", "qty", "price", "active", "name", "bc"}; - var options = ScanOptions.builder() - .projection(Expression.select(columns, Expression.root())); - if (filter != null) { - options.filter(filter); - } - List> rows = new ArrayList<>(); - Scan scan = DataSource.open(SESSION, file.toAbsolutePath().toUri().toString()).scan(options.build()); - while (scan.hasNext()) { - Partition partition = scan.next(); - try (ArrowReader reader = partition.scanArrow(ALLOCATOR)) { - while (reader.loadNextBatch()) { - VectorSchemaRoot root = reader.getVectorSchemaRoot(); - for (int i = 0; i < root.getRowCount(); i++) { - List row = new ArrayList<>(); - for (String column : columns) { - FieldVector vector = root.getVector(column); - Object value = vector.isNull(i) ? null : vector.getObject(i); - row.add(value instanceof Text text ? text.toString() : value); - } - rows.add(row); - } - } - } - } - return rows; - } -} diff --git a/integration/src/test/java/io/github/dfa1/vortex/integration/CsvImportInteropIntegrationTest.java b/integration/src/test/java/io/github/dfa1/vortex/integration/CsvImportInteropIntegrationTest.java new file mode 100644 index 000000000..55067bf94 --- /dev/null +++ b/integration/src/test/java/io/github/dfa1/vortex/integration/CsvImportInteropIntegrationTest.java @@ -0,0 +1,213 @@ +package io.github.dfa1.vortex.integration; + +import dev.vortex.api.DataSource; +import dev.vortex.api.Expression; +import dev.vortex.api.Partition; +import dev.vortex.api.Scan; +import dev.vortex.api.ScanOptions; +import dev.vortex.api.Session; +import dev.vortex.arrow.ArrowAllocation; +import dev.vortex.jni.NativeLoader; +import io.github.dfa1.vortex.core.model.ColumnName; +import io.github.dfa1.vortex.core.model.DType; +import io.github.dfa1.vortex.csv.CsvImporter; +import io.github.dfa1.vortex.csv.ImportOptions; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.vector.FieldVector; +import org.apache.arrow.vector.IntVector; +import org.apache.arrow.vector.SmallIntVector; +import org.apache.arrow.vector.TinyIntVector; +import org.apache.arrow.vector.UInt1Vector; +import org.apache.arrow.vector.UInt2Vector; +import org.apache.arrow.vector.UInt4Vector; +import org.apache.arrow.vector.UInt8Vector; +import org.apache.arrow.vector.VectorSchemaRoot; +import org.apache.arrow.vector.ipc.ArrowReader; +import org.apache.arrow.vector.types.FloatingPointPrecision; +import org.apache.arrow.vector.types.pojo.ArrowType; +import org.apache.arrow.vector.types.pojo.Field; +import org.apache.arrow.vector.util.Text; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/// CSV imports read back by vortex-jni. An empty cell in a numeric or boolean column is a missing +/// value, so vortex-jni must read it back as null (#592): the import stored `0`, `0.0` or `false` +/// instead, indistinguishable from a real zero. An inferred text column keeps its empty cells as +/// `""`, as Arrow's CSV reader does. A schema's narrow and unsigned integer types and `F32` are +/// written as those types, which vortex-jni reads with the same values. +class CsvImportInteropIntegrationTest { + + private static final Session SESSION = Session.create(); + private static final BufferAllocator ALLOCATOR = ArrowAllocation.rootAllocator(); + private static final int ROWS = 3_000; + + static { + NativeLoader.loadJni(); + } + + @TempDir + Path tmp; + + @Test + void emptyCells_readAsNullByVortexJni() throws IOException { + // Given: every 7th row empty in each typed column, `bc` empty in every row, and enough rows + // for several zones, so the filtered read below prunes and slices + StringBuilder csv = new StringBuilder("id,qty,price,active,name,bc\n"); + for (int i = 0; i < ROWS; i++) { + boolean empty = i % 7 == 3; + csv.append(i).append(',') + .append(empty ? "" : String.valueOf(i % 50)).append(',') + .append(empty ? "" : (i % 50) + ".5").append(',') + .append(empty ? "" : String.valueOf(i % 2 == 0)).append(',') + .append(empty ? "" : "n" + i).append(',') + .append('\n'); + } + Path csvFile = tmp.resolve("empties.csv"); + Files.writeString(csvFile, csv); + Path vortex = tmp.resolve("empties.vortex"); + String[] columns = {"id", "qty", "price", "active", "name", "bc"}; + + // When + CsvImporter.importCsv(csvFile, vortex); + List> result = readJni(vortex, columns, null); + List> filtered = readJni(vortex, columns, Expression.binary(Expression.BinaryOp.GTE, + Expression.column("id"), Expression.literal(2_000L))); + // a filter on the nullable columns themselves goes through our zone stats for zones with + // nulls: a wrong min/max or null count prunes rows Rust should return, or keeps a null + List> byQty = readJni(vortex, columns, Expression.binary(Expression.BinaryOp.GTE, + Expression.column("qty"), Expression.literal(25L))); + List> byAllNull = readJni(vortex, columns, Expression.binary(Expression.BinaryOp.GTE, + Expression.column("bc"), Expression.literal(0L))); + + // Then + List> expected = new ArrayList<>(); + for (int i = 0; i < ROWS; i++) { + boolean empty = i % 7 == 3; + expected.add(Arrays.asList((long) i, + empty ? null : (long) (i % 50), + empty ? null : (i % 50) + 0.5, + empty ? null : i % 2 == 0, + empty ? "" : "n" + i, + null)); + } + assertThat(result).isEqualTo(expected); + assertThat(filtered).isEqualTo(expected.subList(2_000, ROWS)); + // a null qty never satisfies the comparison + assertThat(byQty).isEqualTo(expected.stream() + .filter(row -> row.get(1) instanceof Long qty && qty >= 25) + .toList()); + assertThat(byAllNull).isEmpty(); + } + + @Test + void schemaTypes_readByVortexJniWithTheirValues() throws IOException { + // Given: each integer width at both ends of its range, unsigned values above the signed + // maximum (a sign mix-up reads them negative), an F32 and a nullable U8 with empty cells + StringBuilder csv = new StringBuilder("id,i8,u8,i16,u16,u32,u64,f32\n"); + List> expected = new ArrayList<>(); + for (int i = 0; i < ROWS; i++) { + boolean empty = i % 7 == 3; + long i8 = i % 256 - 128; + long u8 = i % 256; + long i16 = i * 20L - 32_768; + long u16 = 65_535 - i; + long u32 = 4_294_967_295L - i; + String u64 = Long.toUnsignedString(-1L - i); + float f32 = i + 0.25f; + csv.append(i).append(',').append(i8).append(',').append(empty ? "" : String.valueOf(u8)).append(',') + .append(i16).append(',').append(u16).append(',').append(u32).append(',').append(u64).append(',') + .append(f32).append('\n'); + expected.add(Arrays.asList((long) i, i8, empty ? null : u8, i16, u16, u32, u64, f32)); + } + Path csvFile = tmp.resolve("types.csv"); + Files.writeString(csvFile, csv); + Path vortex = tmp.resolve("types.vortex"); + DType.Struct schema = DType.structBuilder() + .field(ColumnName.of("id"), DType.I64) + .field(ColumnName.of("i8"), DType.I8) + .field(ColumnName.of("u8"), DType.U8.asNullable()) + .field(ColumnName.of("i16"), DType.I16) + .field(ColumnName.of("u16"), DType.U16) + .field(ColumnName.of("u32"), DType.U32) + .field(ColumnName.of("u64"), DType.U64) + .field(ColumnName.of("f32"), DType.F32) + .build(); + String[] columns = {"id", "i8", "u8", "i16", "u16", "u32", "u64", "f32"}; + + // When + CsvImporter.importCsv(csvFile, vortex, ImportOptions.defaults().withSchema(schema)); + List types = DataSource.open(SESSION, vortex.toAbsolutePath().toUri().toString()) + .arrowSchema(ALLOCATOR).getFields().stream().map(Field::getType).toList(); + List> result = readJni(vortex, columns, null); + // a filter on a narrow column, so its zone stats prune and the read slices the others + List> filtered = readJni(vortex, columns, Expression.binary(Expression.BinaryOp.GTE, + Expression.column("i16"), Expression.literal((short) 0))); + + // Then + assertThat(types).isEqualTo(List.of(new ArrowType.Int(64, true), new ArrowType.Int(8, true), + new ArrowType.Int(8, false), new ArrowType.Int(16, true), new ArrowType.Int(16, false), + new ArrowType.Int(32, false), new ArrowType.Int(64, false), + new ArrowType.FloatingPoint(FloatingPointPrecision.SINGLE))); + assertThat(result).isEqualTo(expected); + assertThat(filtered).isEqualTo(expected.stream().filter(row -> (long) row.get(3) >= 0).toList()); + } + + private static List> readJni(Path file, String[] columns, Expression filter) throws IOException { + var options = ScanOptions.builder() + .projection(Expression.select(columns, Expression.root())); + if (filter != null) { + options.filter(filter); + } + List> rows = new ArrayList<>(); + Scan scan = DataSource.open(SESSION, file.toAbsolutePath().toUri().toString()).scan(options.build()); + while (scan.hasNext()) { + Partition partition = scan.next(); + try (ArrowReader reader = partition.scanArrow(ALLOCATOR)) { + while (reader.loadNextBatch()) { + VectorSchemaRoot root = reader.getVectorSchemaRoot(); + for (int i = 0; i < root.getRowCount(); i++) { + List row = new ArrayList<>(); + for (String column : columns) { + row.add(value(root.getVector(column), i)); + } + rows.add(row); + } + } + } + } + return rows; + } + + /// The cell as a `long` for every integer width (a `U64` as its unsigned decimal text, since + /// it may not fit), a `String` for text, else Arrow's own object. + private static Object value(FieldVector vector, int i) { + if (vector.isNull(i)) { + return null; + } + return switch (vector) { + case TinyIntVector v -> (long) v.get(i); + case SmallIntVector v -> (long) v.get(i); + case IntVector v -> (long) v.get(i); + case UInt1Vector v -> (long) Byte.toUnsignedInt(v.get(i)); + case UInt2Vector v -> (long) v.get(i); + case UInt4Vector v -> Integer.toUnsignedLong(v.get(i)); + case UInt8Vector v -> Long.toUnsignedString(v.get(i)); + default -> { + Object value = vector.getObject(i); + if (value instanceof Text text) { + value = text.toString(); + } + yield value; + } + }; + } +}