From 3f683b8f79878564c2b14df5d6163b96ab365a48 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Thu, 22 Oct 2026 22:24:59 +0200 Subject: [PATCH] refactor: remove dead code and duplicated helpers Found by method-level reference counts plus PMD CPD. No behavior change. Dead or test-only code: - ZonedStatsSchema.zoneLength: no caller; the scan finds legacy vortex.stats zones by physical chunk (ScanIterator#zoneStarts). - TimestampExtensionDecoder.zonedDateTime: only tests called it; it was just instant(...).atZone(timezone(ext).orElse(UTC)). - StatsOptions.distinctAndTop: only tests called it; they now keep a local ALL_STATS built with the withXxx methods. - LeBitReader.bitsConsumed: only tests called it; they now check the position by reading the next bits. - Ansi.CLEAR_LINE_RIGHT: never referenced. Duplicated code: - SparseEncodingDecoder.scalarToLong copied ConstantEncodingDecoder.scalarToRawBits. - ChunkImpl.adaptUtf8/adaptBinary now share adaptVarBin. - InspectorRender reuses GridRender.formatPrimitive and bytesToHex. - CsvExporter.jsonValue drops its copied list cases and falls back to cellValue; a new List[List[I64]] test covers this path. Co-Authored-By: Claude Sonnet 5 --- .../dfa1/vortex/cli/tui/GridRender.java | 54 +++++++++++-------- .../dfa1/vortex/cli/tui/InspectorRender.java | 53 ++---------------- .../github/dfa1/vortex/cli/tui/term/Ansi.java | 3 -- .../github/dfa1/vortex/csv/CsvExporter.java | 15 +----- .../dfa1/vortex/csv/CsvExporterTest.java | 30 +++++++++++ .../decode/ConstantEncodingDecoder.java | 7 ++- .../vortex/reader/decode/LeBitReader.java | 5 -- .../reader/decode/SparseEncodingDecoder.java | 22 +------- .../extension/TimestampExtensionDecoder.java | 13 ----- .../reader/layout/ZonedStatsSchema.java | 14 ----- .../vortex/reader/decode/LeBitReaderTest.java | 11 ++-- .../TimestampExtensionDecoderTest.java | 30 ----------- .../reader/layout/ZonedStatsSchemaTest.java | 37 ------------- .../github/dfa1/vortex/writer/ChunkImpl.java | 30 ++++------- .../vortex/writer/encode/StatsOptions.java | 7 --- .../writer/encode/ArrayAndStatsTest.java | 6 ++- .../vortex/writer/encode/ArrayStatsTest.java | 18 ++++--- .../writer/encode/SchemeVerdictTest.java | 11 ++-- 18 files changed, 110 insertions(+), 256 deletions(-) diff --git a/cli/src/main/java/io/github/dfa1/vortex/cli/tui/GridRender.java b/cli/src/main/java/io/github/dfa1/vortex/cli/tui/GridRender.java index 8e602f8d3..39b3c76a5 100644 --- a/cli/src/main/java/io/github/dfa1/vortex/cli/tui/GridRender.java +++ b/cli/src/main/java/io/github/dfa1/vortex/cli/tui/GridRender.java @@ -59,28 +59,9 @@ static String formatCell(Array array, long i, DType declared) { } } return switch (inner) { - // Long/IntArray have no dtype-aware getter, so gate the unsigned rendering on the - // ptype; a U64/U32 high-half value would otherwise show as a negative decimal - // (e.g. a U8 magnesium column widened to U64 with the sign bit set). - case LongArray a -> isUnsigned(a) - ? Long.toUnsignedString(a.getLong(i)) - : Long.toString(a.getLong(i)); - case IntArray a -> isUnsigned(a) - ? Integer.toUnsignedString(a.getInt(i)) - : Integer.toString(a.getInt(i)); - // Short/ByteArray.getInt already zero-extends U16/U8, so widening to int and - // printing signed decimal yields the correct unsigned value. - case ShortArray a -> Integer.toString(a.getInt(i)); - case ByteArray a -> Integer.toString(a.getInt(i)); - case DoubleArray a -> Double.toString(a.getDouble(i)); - case FloatArray a -> Float.toString(a.getFloat(i)); - case Float16Array a -> Float.toString(a.getFloat(i)); - case BoolArray a -> Boolean.toString(a.getBoolean(i)); - case VarBinArray a -> a.dtype() instanceof DType.Utf8 - ? a.getString(i) - : bytesToHex(a.getBytes(i)); + case VarBinArray a -> a.dtype() instanceof DType.Utf8 ? a.getString(i) : bytesToHex(a.getBytes(i)); case DecimalArray a -> a.getDecimal(i).toPlainString(); - default -> "<" + inner.getClass().getSimpleName() + ">"; + default -> formatPrimitive(inner, i).orElse("<" + inner.getClass().getSimpleName() + ">"); }; } catch (RuntimeException e) { String msg = e.getMessage(); @@ -89,6 +70,31 @@ static String formatCell(Array array, long i, DType declared) { } } + /// Formats the element at `i` of a numeric or boolean leaf array, shared by the grid and the + /// inspector, which differ only in how they render strings, bytes and decimals. + /// + /// @param array the leaf value array (never a [MaskedArray]; the caller unwraps first) + /// @param i element index + /// @return the formatted value, or empty for any other array shape + static Optional formatPrimitive(Array array, long i) { + return switch (array) { + // Long/IntArray have no dtype-aware getter, so gate the unsigned rendering on the + // ptype; a U64/U32 high-half value would otherwise show as a negative decimal + // (e.g. a U8 magnesium column widened to U64 with the sign bit set). + case LongArray a -> Optional.of(isUnsigned(a) ? Long.toUnsignedString(a.getLong(i)) : Long.toString(a.getLong(i))); + case IntArray a -> Optional.of(isUnsigned(a) ? Integer.toUnsignedString(a.getInt(i)) : Integer.toString(a.getInt(i))); + // Short/ByteArray.getInt already zero-extends U16/U8, so widening to int and + // printing signed decimal yields the correct unsigned value. + case ShortArray a -> Optional.of(Integer.toString(a.getInt(i))); + case ByteArray a -> Optional.of(Integer.toString(a.getInt(i))); + case DoubleArray a -> Optional.of(Double.toString(a.getDouble(i))); + case FloatArray a -> Optional.of(Float.toString(a.getFloat(i))); + case Float16Array a -> Optional.of(Float.toString(a.getFloat(i))); + case BoolArray a -> Optional.of(Boolean.toString(a.getBoolean(i))); + default -> Optional.empty(); + }; + } + /// Whether the array's dtype is an unsigned integer, so high-half values must render as /// unsigned decimal rather than the two's-complement negative the raw signed getter returns. /// @@ -112,7 +118,11 @@ private static Optional formatExtension(DType.Extension ext, Array stora }; } - private static String bytesToHex(byte[] bytes) { + /// Renders up to the first 16 bytes as `0x`-prefixed hex, with `...` when there are more. + /// + /// @param bytes the bytes to render + /// @return the hex preview + static String bytesToHex(byte[] bytes) { int n = Math.min(bytes.length, 16); StringBuilder sb = new StringBuilder(n * 2 + 2); sb.append("0x"); diff --git a/cli/src/main/java/io/github/dfa1/vortex/cli/tui/InspectorRender.java b/cli/src/main/java/io/github/dfa1/vortex/cli/tui/InspectorRender.java index 20cde0413..4d297f20a 100644 --- a/cli/src/main/java/io/github/dfa1/vortex/cli/tui/InspectorRender.java +++ b/cli/src/main/java/io/github/dfa1/vortex/cli/tui/InspectorRender.java @@ -3,17 +3,9 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.ExtensionId; import io.github.dfa1.vortex.reader.array.Array; -import io.github.dfa1.vortex.reader.array.BoolArray; -import io.github.dfa1.vortex.reader.array.ByteArray; import io.github.dfa1.vortex.reader.array.DecimalArray; -import io.github.dfa1.vortex.reader.array.DoubleArray; -import io.github.dfa1.vortex.reader.array.Float16Array; -import io.github.dfa1.vortex.reader.array.FloatArray; import io.github.dfa1.vortex.reader.array.GenericArray; -import io.github.dfa1.vortex.reader.array.IntArray; -import io.github.dfa1.vortex.reader.array.LongArray; import io.github.dfa1.vortex.reader.array.MaskedArray; -import io.github.dfa1.vortex.reader.array.ShortArray; import io.github.dfa1.vortex.reader.array.StructArray; import io.github.dfa1.vortex.reader.array.VarBinArray; import io.github.dfa1.vortex.reader.extension.DateExtensionDecoder; @@ -47,29 +39,12 @@ static String formatValue(Array array, int i, DType declared) { } } return switch (array) { - // Long/IntArray have no dtype-aware getter, so gate the unsigned rendering on the - // ptype; a U64/U32 high-half value would otherwise show as a negative decimal. - case LongArray a -> isUnsigned(a) - ? Long.toUnsignedString(a.getLong(i)) - : Long.toString(a.getLong(i)); - case IntArray a -> isUnsigned(a) - ? Integer.toUnsignedString(a.getInt(i)) - : Integer.toString(a.getInt(i)); - // Short/ByteArray.getInt already zero-extends U16/U8, so a signed-decimal print of the - // widened int is the correct unsigned value. - case ShortArray a -> Integer.toString(a.getInt(i)); - case ByteArray a -> Integer.toString(a.getInt(i)); - case DoubleArray a -> Double.toString(a.getDouble(i)); - case FloatArray a -> Float.toString(a.getFloat(i)); - case Float16Array a -> Float.toString(a.getFloat(i)); - case BoolArray a -> Boolean.toString(a.getBoolean(i)); - case VarBinArray a -> a.dtype() instanceof DType.Utf8 - ? "\"" + a.getString(i) + "\"" - : bytesToShortHex(a.getBytes(i)); + case VarBinArray a -> a.dtype() instanceof DType.Utf8 ? "\"" + a.getString(i) + "\"" : GridRender.bytesToHex(a.getBytes(i)); case GenericArray a when a.dtype() instanceof DType.Decimal -> tryDecimal(a::getDecimal, a, i); case DecimalArray a -> tryDecimal(a::getDecimal, a, i); - default -> "<" + array.getClass().getSimpleName() + " " + array.dtype() + ">"; + default -> GridRender.formatPrimitive(array, i) + .orElse("<" + array.getClass().getSimpleName() + " " + array.dtype() + ">"); }; } @@ -124,15 +99,6 @@ private static String formatStatsCell(Array field, int row, DType declared) { return formatValue(field, row, declared); } - /// Whether the array's dtype is an unsigned integer, so high-half values must render as - /// unsigned decimal rather than the two's-complement negative the raw signed getter returns. - /// - /// @param arr the value array under inspection - /// @return `true` when the dtype is a `U8`–`U64` primitive - private static boolean isUnsigned(Array arr) { - return arr.dtype() instanceof DType.Primitive p && p.ptype().isUnsigned(); - } - private static String tryDecimal(LongFunction reader, Array a, int i) { try { return reader.apply(i).toPlainString(); @@ -145,19 +111,6 @@ private static String tryDecimal(LongFunction reader, Array a, int i } } - private static String bytesToShortHex(byte[] bytes) { - int n = Math.min(bytes.length, 16); - StringBuilder sb = new StringBuilder(n * 3 + 2); - sb.append("0x"); - for (int i = 0; i < n; i++) { - sb.append(String.format("%02x", bytes[i] & 0xff)); - } - if (bytes.length > n) { - sb.append("..."); - } - return sb.toString(); - } - /// Formats one 16-byte row of a hex dump: offset, hex columns, ASCII gutter. /// /// @param data the bytes being dumped diff --git a/cli/src/main/java/io/github/dfa1/vortex/cli/tui/term/Ansi.java b/cli/src/main/java/io/github/dfa1/vortex/cli/tui/term/Ansi.java index 12bbe4d53..aa3bedce7 100644 --- a/cli/src/main/java/io/github/dfa1/vortex/cli/tui/term/Ansi.java +++ b/cli/src/main/java/io/github/dfa1/vortex/cli/tui/term/Ansi.java @@ -15,9 +15,6 @@ public final class Ansi { /// Clear entire screen. public static final String CLEAR_SCREEN = CSI + "2J"; - /// Erase from the cursor to the end of the current line. - public static final String CLEAR_LINE_RIGHT = CSI + "K"; - /// Move cursor to top-left. public static final String CURSOR_HOME = CSI + "H"; diff --git a/csv/src/main/java/io/github/dfa1/vortex/csv/CsvExporter.java b/csv/src/main/java/io/github/dfa1/vortex/csv/CsvExporter.java index ff9642923..015f2f4e8 100644 --- a/csv/src/main/java/io/github/dfa1/vortex/csv/CsvExporter.java +++ b/csv/src/main/java/io/github/dfa1/vortex/csv/CsvExporter.java @@ -295,18 +295,6 @@ private static String jsonValue(Array arr, long rowIdx) { return switch (arr) { case StructArray sa -> isNullStructRow(sa, rowIdx) ? "null" : jsonObject(sa, rowIdx); case MaskedArray ma -> ma.isValid(rowIdx) ? jsonValue(ma.inner(), rowIdx) : "null"; - case FixedSizeListArray fla -> jsonArray(fla.elements(), rowIdx * fla.fixedSize(), (rowIdx + 1L) * fla.fixedSize()); - case ListArray la -> { - long start = offsetAt(la.offsets(), rowIdx); - long end = offsetAt(la.offsets(), rowIdx + 1); - yield jsonArray(la.elements(), start, end); - } - // A list view addresses each row by its own (offset, size) pair: rows may overlap or - // appear out of order in `elements`, so the next row's offset is not this row's end. - case ListViewArray lv -> { - long start = offsetAt(lv.offsets(), rowIdx); - yield jsonArray(lv.elements(), start, start + offsetAt(lv.sizes(), rowIdx)); - } case NullArray _ -> "null"; case UnionArray ua -> ua.isValid(rowIdx) ? jsonValue(ua.variant(ua.variantIndex(rowIdx)), rowIdx) : "null"; case VarBinArray va -> { @@ -314,7 +302,8 @@ private static String jsonValue(Array arr, long rowIdx) { jsonString(sb, va.getString(rowIdx)); yield sb.toString(); } - // Numeric and boolean leaves render identically to a top-level cell, unquoted. + // Numeric and boolean leaves, and lists (already a JSON array), render identically to a + // top-level cell. default -> cellValue(arr, rowIdx); }; } diff --git a/csv/src/test/java/io/github/dfa1/vortex/csv/CsvExporterTest.java b/csv/src/test/java/io/github/dfa1/vortex/csv/CsvExporterTest.java index ebf7ca5a5..8031dac91 100644 --- a/csv/src/test/java/io/github/dfa1/vortex/csv/CsvExporterTest.java +++ b/csv/src/test/java/io/github/dfa1/vortex/csv/CsvExporterTest.java @@ -550,6 +550,36 @@ void rendersListColumnAsJsonArray(@TempDir Path tmp) throws Exception { "c,[]"); } + @Test + void rendersListOfListsAsNestedJsonArrays(@TempDir Path tmp) throws Exception { + // Given a List[List[I64]] column: the inner lists are rendered by jsonValue, not cellValue, + // so this pins that a list nested inside JSON renders the same as a top-level list cell. + Path vortex = tmp.resolve("listoflists.vortex"); + DType.Struct schema = new DType.Struct( + List.of(ColumnName.of("ids")), + List.of(new DType.List(new DType.List(DType.I64, false), false)), + false); + try (FileChannel ch = FileChannel.open(vortex, StandardOpenOption.CREATE, StandardOpenOption.WRITE); + VortexWriter writer = VortexWriter.create(ch, schema, WriteOptions.defaults())) { + writer.writeChunk(Map.of( + ColumnName.of("ids"), new ListData( + new ListData(new long[]{1L, 2L, 3L}, new long[]{0, 2, 3, 3}, 3), // [1,2] [3] [] + new long[]{0, 2, 3}, // row 0 → [[1,2],[3]], row 1 → [[]] + 2))); + } + Path csv = tmp.resolve("out.csv"); + + // When + CsvExporter.exportCsv(vortex, csv); + List result = Files.readAllLines(csv); + + // Then + assertThat(result).containsExactly( + "ids", + "\"[[1,2],[3]]\"", + "[[]]"); + } + @Test void rendersListOfStructsAsJsonArrayOfObjects(@TempDir Path tmp) throws Exception { // Given a List[Struct] column — the osm-germany `members` shape (issue #257); diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/ConstantEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/ConstantEncodingDecoder.java index 1fc1c485e..7e67e235e 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/ConstantEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/ConstantEncodingDecoder.java @@ -153,7 +153,12 @@ private static Array decodeString(ProtoScalarValue scalar, DType dtype, long n) return new VarBinConstantArray(dtype, n, strBytes); } - private static long scalarToRawBits(ProtoScalarValue scalar) { + /// Raw bits of a primitive scalar as a `long`: floats by their IEEE bit pattern, `0` when the + /// scalar carries no primitive value. Shared with [SparseEncodingDecoder]'s fill value. + /// + /// @param scalar the scalar to read + /// @return the value's raw bits + static long scalarToRawBits(ProtoScalarValue scalar) { if (scalar.int64_value() != null) { return scalar.int64_value(); } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/LeBitReader.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/LeBitReader.java index 7392c6023..365c47bdc 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/LeBitReader.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/LeBitReader.java @@ -68,9 +68,4 @@ public void alignToByte() { public long byteOffset() { return bitPos >>> 3; } - - /// @return total bits consumed since construction - public long bitsConsumed() { - return bitPos; - } } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java index 867abcf8e..cbdac9d00 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/decode/SparseEncodingDecoder.java @@ -140,7 +140,7 @@ public Array decode(DecodeContext ctx) { throw new VortexException(EncodingId.VORTEX_SPARSE, "expected primitive dtype, got " + ctx.dtype()); } PType valuePtype = ((DType.Primitive) ctx.dtype()).ptype(); - long fillBits = scalarToLong(fillScalar); + long fillBits = ConstantEncodingDecoder.scalarToRawBits(fillScalar); // Lazy path: keep fill bits + decoded patches; no n-sized buffer allocated. // When numPatches == 0 we still decode zero-length children so the record's @@ -330,24 +330,4 @@ private static byte[] fillBytes(ProtoScalarValue fill, boolean fillValid) { return new byte[0]; } - private static long scalarToLong(ProtoScalarValue scalar) { - if (scalar.int64_value() != null) { - return scalar.int64_value(); - } - if (scalar.uint64_value() != null) { - return scalar.uint64_value(); - } - if (scalar.f16_value() != null) { - // Rust writes an f16 as its 16 bits in a u64; the legacy uint64_value form is read above - return scalar.f16_value(); - } - if (scalar.f32_value() != null) { - return Float.floatToRawIntBits(scalar.f32_value()); - } - if (scalar.f64_value() != null) { - return Double.doubleToRawLongBits(scalar.f64_value()); - } - return 0L; - } - } diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoder.java b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoder.java index 6e064e81f..ceec2ce6b 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoder.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoder.java @@ -11,8 +11,6 @@ import java.time.Instant; import java.time.ZoneId; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -76,17 +74,6 @@ public Instant instant(DType.Extension ext, Array storage, long i) { return ExtensionStorage.instantFromRaw(ExtensionStorage.epochInteger(storage, i), unit); } - /// Decodes the timestamp cell at row `i` to a [ZonedDateTime] - /// using the timezone from the metadata, defaulting to UTC when absent. - /// - /// @param ext declared extension dtype - /// @param storage signed-integer storage array - /// @param i row index, `0 <= i < storage.length()` - /// @return decoded zoned date-time - public ZonedDateTime zonedDateTime(DType.Extension ext, Array storage, long i) { - return instant(ext, storage, i).atZone(timezone(ext).orElse(ZoneOffset.UTC)); - } - /// Returns the IANA timezone recorded in the extension metadata. /// /// @param ext declared extension dtype diff --git a/reader/src/main/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchema.java b/reader/src/main/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchema.java index f51399b4f..3f2624b2b 100644 --- a/reader/src/main/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchema.java +++ b/reader/src/main/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchema.java @@ -4,8 +4,6 @@ import io.github.dfa1.vortex.core.model.DType; import io.github.dfa1.vortex.core.model.LayoutId; -import static io.github.dfa1.vortex.core.io.VortexFormat.LE_INT; - import java.lang.foreign.MemorySegment; import java.lang.foreign.ValueLayout; import java.util.ArrayList; @@ -105,18 +103,6 @@ public ColumnName fieldName() { private ZonedStatsSchema() { } - /// Returns the zone length declared in the layout metadata (logical rows per zone), - /// or `0` if the metadata is too short to carry one. - /// - /// @param metadata raw `vortex.stats` layout metadata, possibly `null` - /// @return decoded zone length, or `0` when absent - public static long zoneLength(MemorySegment metadata) { - if (metadata == null || metadata.byteSize() < 4) { - return 0L; - } - return Integer.toUnsignedLong(metadata.get(LE_INT, 0)); - } - /// Returns the zone length declared in a newer `vortex.zoned` layout's aggregate-spec metadata /// (field 1, `uint32 zone_len`, of the `ZonedMetadataProto` described in [#aggregateIds]), or /// `0` when the blob is empty, carries an unsupported version, or is not well-formed protobuf. diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/LeBitReaderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/LeBitReaderTest.java index ca717da6e..cab15b224 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/decode/LeBitReaderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/decode/LeBitReaderTest.java @@ -41,7 +41,8 @@ void zero_bits_returns_zero() { // Then assertThat(result).isZero(); - assertThat(sut.bitsConsumed()).isZero(); + // a zero-width read must not advance: the whole byte is still there + assertThat(sut.readBits(8)).isEqualTo(0xFFL); } @Test @@ -54,7 +55,8 @@ void single_byte_low_nibble() { // Then — LSB first: bits [3:0] = 0b0011 = 3 assertThat(result).isEqualTo(3L); - assertThat(sut.bitsConsumed()).isEqualTo(4); + // advanced exactly 4 bits: the high nibble 0b1011 comes next + assertThat(sut.readBits(4)).isEqualTo(0xBL); } @Test @@ -148,7 +150,8 @@ void sequential_reads_advance_position() { assertThat(b1).isEqualTo(1); assertThat(b2).isZero(); assertThat(b3).isEqualTo(1); - assertThat(sut.bitsConsumed()).isEqualTo(4); + // advanced exactly 4 bits: the high nibble 0b1010 comes next + assertThat(sut.readBits(4)).isEqualTo(0xAL); } @Test @@ -190,7 +193,6 @@ void already_aligned_is_noop() { // Then — byte offset unchanged assertThat(sut.byteOffset()).isEqualTo(1); - assertThat(sut.bitsConsumed()).isEqualTo(8); } @Test @@ -204,7 +206,6 @@ void partial_byte_advances_to_next_byte() { // Then — aligned to byte 1 assertThat(sut.byteOffset()).isEqualTo(1); - assertThat(sut.bitsConsumed()).isEqualTo(8); } @Test diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoderTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoderTest.java index 647962683..1b4977d41 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoderTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/extension/TimestampExtensionDecoderTest.java @@ -19,7 +19,6 @@ import java.time.Instant; import java.time.ZoneId; import java.time.ZoneOffset; -import java.time.ZonedDateTime; import java.util.List; import static io.github.dfa1.vortex.reader.extension.ExtensionTestSupport.I64; @@ -95,35 +94,6 @@ void instant_microsecondsPath_handlesNegativeRaw() { } } - @Test - void zonedDateTime_withTimezone_appliesIt() { - // Given — ms since epoch + Europe/Paris tz in metadata - try (Arena arena = Arena.ofConfined()) { - DType.Extension dtype = ext("vortex.timestamp", I64, tzMeta((byte) 2, "Europe/Paris")); - - // When - ZonedDateTime got = sut.zonedDateTime(dtype, i64(arena, 1_000L), 0); - - // Then - assertThat(got.getZone()).isEqualTo(ZoneId.of("Europe/Paris")); - assertThat(got.toInstant()).isEqualTo(Instant.ofEpochMilli(1_000L)); - } - } - - @Test - void zonedDateTime_noTimezone_defaultsToUtc() { - // Given — tz_len = 0 should fall back to UTC for unambiguity - try (Arena arena = Arena.ofConfined()) { - DType.Extension dtype = ext("vortex.timestamp", I64, tzMeta((byte) 2, null)); - - // When - ZonedDateTime got = sut.zonedDateTime(dtype, i64(arena, 0L), 0); - - // Then - assertThat(got.getZone()).isEqualTo(ZoneOffset.UTC); - } - } - @Test void decodeAll_maskedArray_yieldsNullAtInvalidPositions() { // Given — Milliseconds storage with row 1 null. ext dtype uses I64 storage. diff --git a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchemaTest.java b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchemaTest.java index 40c96c29c..381d32f51 100644 --- a/reader/src/test/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchemaTest.java +++ b/reader/src/test/java/io/github/dfa1/vortex/reader/layout/ZonedStatsSchemaTest.java @@ -19,43 +19,6 @@ class ZonedStatsSchemaTest { - @Nested - class ZoneLength { - @Test - void readsLittleEndianU32() { - // Given — 8192 = 0x2000 stored as LE u32 - MemorySegment meta = MemorySegment.ofArray(new byte[4]); - meta.set(io.github.dfa1.vortex.core.io.VortexFormat.LE_INT, 0, 8192); - - // When - long result = ZonedStatsSchema.zoneLength(meta); - - // Then - assertThat(result).isEqualTo(8192L); - } - - @Test - void returnsZeroForNullMetadata() { - // Given / When - long result = ZonedStatsSchema.zoneLength(null); - - // Then — nothing to decode; caller treats as "no zones" - assertThat(result).isZero(); - } - - @Test - void returnsZeroForShortMetadata() { - // Given — only 2 bytes (insufficient for a u32) - MemorySegment meta = MemorySegment.ofArray(new byte[2]); - - // When - long result = ZonedStatsSchema.zoneLength(meta); - - // Then — defensively zero rather than throwing - assertThat(result).isZero(); - } - } - @Nested class StatLookup { @ParameterizedTest diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/ChunkImpl.java b/writer/src/main/java/io/github/dfa1/vortex/writer/ChunkImpl.java index 742e468b7..978c7eac5 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/ChunkImpl.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/ChunkImpl.java @@ -125,7 +125,15 @@ private static Object adaptUtf8(String column, DType.Utf8 dtype, Object value) { throw new IllegalArgumentException( "column '" + column + "' expects String[] for Utf8; got " + value.getClass().getSimpleName()); } - if (!dtype.nullable()) { + return adaptVarBin(column, dtype.nullable(), arr); + } + + /// Shared by Utf8 (`String[]`) and Binary (`byte[][]`): a non-nullable column rejects any + /// `null` element. A nullable one unifies on the NullableData carrier like the primitive + /// paths: the raw array (null elements preserved) plus a derived validity bitmap. The writer + /// then routes it through MaskedEncoding (or a nullable-capable encoder such as vortex.zstd). + private static Object adaptVarBin(String column, boolean nullable, Object[] arr) { + if (!nullable) { for (int i = 0; i < arr.length; i++) { if (arr[i] == null) { throw new IllegalArgumentException( @@ -134,9 +142,6 @@ private static Object adaptUtf8(String column, DType.Utf8 dtype, Object value) { } return arr; } - // Nullable utf8/binary unifies on the NullableData carrier like the primitive paths: the - // raw String[] (null elements preserved) plus a derived validity bitmap. The writer then - // routes it through MaskedEncoding (or a nullable-capable encoder such as vortex.zstd). boolean[] validity = new boolean[arr.length]; for (int i = 0; i < arr.length; i++) { validity[i] = arr[i] != null; @@ -183,22 +188,7 @@ private static Object adaptBinary(String column, DType.Binary dtype, Object valu throw new IllegalArgumentException( "column '" + column + "' expects byte[][] for Binary; got " + value.getClass().getSimpleName()); } - if (!dtype.nullable()) { - for (int i = 0; i < arr.length; i++) { - if (arr[i] == null) { - throw new IllegalArgumentException( - "non-nullable column '" + column + "' received null at row " + i); - } - } - return arr; - } - // Mirrors adaptUtf8: raw byte[][] (null elements preserved) plus a derived validity - // bitmap, routed through MaskedEncoding downstream. - boolean[] validity = new boolean[arr.length]; - for (int i = 0; i < arr.length; i++) { - validity[i] = arr[i] != null; - } - return new NullableData(arr, validity); + return adaptVarBin(column, dtype.nullable(), arr); } private static Object adaptBool(String column, DType.Bool dtype, Object value) { diff --git a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/StatsOptions.java b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/StatsOptions.java index ad5979bd2..ed428cf64 100644 --- a/writer/src/main/java/io/github/dfa1/vortex/writer/encode/StatsOptions.java +++ b/writer/src/main/java/io/github/dfa1/vortex/writer/encode/StatsOptions.java @@ -23,13 +23,6 @@ public static StatsOptions defaults() { return new StatsOptions(false, false); } - /// Track both distinct counts and most-frequent value. - /// - /// @return options requesting distinct counts and the most-frequent value - public static StatsOptions distinctAndTop() { - return new StatsOptions(true, true); - } - /// Whether the scan must count distinct value occurrences. /// /// @return `true` if distinct counts are requested diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ArrayAndStatsTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ArrayAndStatsTest.java index 0499d48b1..c53bca72e 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ArrayAndStatsTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ArrayAndStatsTest.java @@ -10,10 +10,12 @@ /// input never reaches `ArrayStats.compute` (which has no ptype for it). class ArrayAndStatsTest { + private static final StatsOptions ALL_STATS = StatsOptions.defaults().withCountDistinct(true).withTrackMostFrequent(true); + @Test void stats_computedOnce_andSharedByEveryCaller() { // Given - ArrayAndStats sut = new ArrayAndStats(DType.I64, new long[]{7, 7, 9}, StatsOptions.distinctAndTop()); + ArrayAndStats sut = new ArrayAndStats(DType.I64, new long[]{7, 7, 9}, ALL_STATS); // When ArrayStats result = sut.stats(); @@ -28,7 +30,7 @@ void stats_computedOnce_andSharedByEveryCaller() { @Test void stats_nonPrimitive_isEmpty() { // Given: strings go through the same competition, but no stats consumer reads them - ArrayAndStats sut = new ArrayAndStats(DType.UTF8, new String[]{"a", "b"}, StatsOptions.distinctAndTop()); + ArrayAndStats sut = new ArrayAndStats(DType.UTF8, new String[]{"a", "b"}, ALL_STATS); // When ArrayStats result = sut.stats(); diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ArrayStatsTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ArrayStatsTest.java index 5bc1cb25c..07e438374 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ArrayStatsTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/ArrayStatsTest.java @@ -21,13 +21,15 @@ /// grow past its initial capacity. class ArrayStatsTest { + private static final StatsOptions ALL_STATS = StatsOptions.defaults().withCountDistinct(true).withTrackMostFrequent(true); + @Test void compute_emptyArray_returnsEmptySentinel() { // Given int[] data = {}; // When - ArrayStats result = ArrayStats.compute(PType.I32, data, StatsOptions.distinctAndTop()); + ArrayStats result = ArrayStats.compute(PType.I32, data, ALL_STATS); // Then assertThat(result).isEqualTo(ArrayStats.EMPTY); @@ -52,7 +54,7 @@ void compute_withDuplicates_countsDistinctAndMostFrequent() { int[] data = {5, 5, 5, 7, 7, 9}; // When - ArrayStats result = ArrayStats.compute(PType.I32, data, StatsOptions.distinctAndTop()); + ArrayStats result = ArrayStats.compute(PType.I32, data, ALL_STATS); // Then assertThat(result.distinctCount()).isEqualTo(3); @@ -74,7 +76,7 @@ void compute_allDistinctSequence_stopsAtTheCapAndSaysSo() { } // When - ArrayStats result = ArrayStats.compute(PType.I64, data, StatsOptions.distinctAndTop()); + ArrayStats result = ArrayStats.compute(PType.I64, data, ALL_STATS); // Then — the count is a lower bound past the cap, never the exact n assertThat(result.distinctCapped()).isTrue(); @@ -95,7 +97,7 @@ void compute_lowCardinality_neverHitsTheCap() { } // When - ArrayStats result = ArrayStats.compute(PType.I64, data, StatsOptions.distinctAndTop()); + ArrayStats result = ArrayStats.compute(PType.I64, data, ALL_STATS); // Then assertThat(result.distinctCapped()).isFalse(); @@ -110,7 +112,7 @@ void compute_zeroValueIsNotMistakenForAnEmptySlot() { long[] data = new long[1000]; // When - ArrayStats result = ArrayStats.compute(PType.I64, data, StatsOptions.distinctAndTop()); + ArrayStats result = ArrayStats.compute(PType.I64, data, ALL_STATS); // Then assertThat(result.distinctCount()).isEqualTo(1); @@ -141,7 +143,7 @@ void compute_seededRandomData_matchesBruteForceReference(int n) { } // When - ArrayStats result = ArrayStats.compute(PType.I64, data, StatsOptions.distinctAndTop()); + ArrayStats result = ArrayStats.compute(PType.I64, data, ALL_STATS); // Then assertThat(result.distinctCount()).isEqualTo(reference.size()); @@ -230,7 +232,7 @@ void compute_everyIntegerCountingPath_matchesPerRowReference(PType ptype, Object .mapToLong(Map.Entry::getKey).min().orElseThrow(); // When - ArrayStats result = ArrayStats.compute(ptype, data, StatsOptions.distinctAndTop()); + ArrayStats result = ArrayStats.compute(ptype, data, ALL_STATS); // Then assertThat(result.distinctCapped()).isFalse(); @@ -283,7 +285,7 @@ void compute_everyFloatCountingPath_matchesPerRowReference(PType ptype, Object d int expectedTopFreq = reference.values().stream().max(Integer::compare).orElseThrow(); // When - ArrayStats result = ArrayStats.compute(ptype, data, StatsOptions.distinctAndTop()); + ArrayStats result = ArrayStats.compute(ptype, data, ALL_STATS); // Then assertThat(result.distinctCapped()).isFalse(); diff --git a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/SchemeVerdictTest.java b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/SchemeVerdictTest.java index c559cdb6c..51f1c6a93 100644 --- a/writer/src/test/java/io/github/dfa1/vortex/writer/encode/SchemeVerdictTest.java +++ b/writer/src/test/java/io/github/dfa1/vortex/writer/encode/SchemeVerdictTest.java @@ -21,9 +21,10 @@ class SchemeVerdictTest { private static final EncodeContext REAL = EncodeContext.ofDepth(3, Arena.ofAuto(), REGISTRY); private static final EncodeContext SAMPLE = REAL.withSampling(); private static final EncodeContext FINISHED = EncodeContext.ofDepth(0, Arena.ofAuto(), REGISTRY); + private static final StatsOptions ALL_STATS = StatsOptions.defaults().withCountDistinct(true).withTrackMostFrequent(true); private static ArrayAndStats longs(long... values) { - return new ArrayAndStats(DType.I64, values, StatsOptions.distinctAndTop()); + return new ArrayAndStats(DType.I64, values, ALL_STATS); } @Nested @@ -295,7 +296,7 @@ void skipsWhenMoreThanHalfAreDistinct() { @Test void floatsDeferToTheSample() { // Given: Rust's FloatDictScheme samples rather than estimating - ArrayAndStats data = new ArrayAndStats(DType.F64, new double[]{1.5, 1.5, 2.5, 2.5}, StatsOptions.distinctAndTop()); + ArrayAndStats data = new ArrayAndStats(DType.F64, new double[]{1.5, 1.5, 2.5, 2.5}, ALL_STATS); // When Estimate result = sut.expectedRatio(DType.F64, data, REAL); @@ -322,7 +323,7 @@ private static long[] dominated(long fill, int n, int others) { @Test void estimatesANonZeroDominantValue() { // Given: 95 of 100 values are 42 — Java used to require the fill to be 0 - ArrayAndStats data = new ArrayAndStats(DType.I64, dominated(42, 100, 5), StatsOptions.distinctAndTop()); + ArrayAndStats data = new ArrayAndStats(DType.I64, dominated(42, 100, 5), ALL_STATS); // When Estimate result = sut.expectedRatio(DType.I64, data, REAL); @@ -334,7 +335,7 @@ void estimatesANonZeroDominantValue() { @Test void skipsBelowNinetyPercent() { // Given: 89 of 100 - ArrayAndStats data = new ArrayAndStats(DType.I64, dominated(42, 100, 11), StatsOptions.distinctAndTop()); + ArrayAndStats data = new ArrayAndStats(DType.I64, dominated(42, 100, 11), ALL_STATS); // When Estimate result = sut.expectedRatio(DType.I64, data, REAL); @@ -348,7 +349,7 @@ void skipsFloats() { // Given: Rust's float sparse scheme only takes null-dominated arrays double[] values = new double[100]; values[3] = 1.5; - ArrayAndStats data = new ArrayAndStats(DType.F64, values, StatsOptions.distinctAndTop()); + ArrayAndStats data = new ArrayAndStats(DType.F64, values, ALL_STATS); // When Estimate result = sut.expectedRatio(DType.F64, data, REAL);