Skip to content

Commit 6fba232

Browse files
dfa1claude
andcommitted
fix(writer): give Fsst/VarBinView/Zstd a byte-safe Binary path
FsstEncodingEncoder, VarBinViewEncodingEncoder, and ZstdEncodingEncoder all cast data straight to String[], so a DType.Binary column (raw bytes, e.g. an embedded audio/image blob) threw ClassCastException if the cascade competition picked one of them. accepts() had been narrowed to Utf8-only as a stopgap. Add VarBinBytes, a small shared helper (mirroring VarBinEncodingEncoder's existing byte[][]-or-String[] handling) that normalizes both to byte[][] — UTF-8 encoding Utf8 rows, passing Binary rows through untouched, with two null-handling variants (substitute empty, or preserve for detect/strip). All three encoders now accept Binary and round-trip byte-for-byte, including non-UTF8 sequences and null entries (values child of a masked/nullable layout). CascadingCompressor previously routed Binary through a first-match findPrimitiveEncoding + spliceResult, not the real sample-and-measure competition it gives Utf8 — so even with accepts() fixed, whichever encoder happened to register first would always win. Binary now joins Utf8 in the same competeAndEncode path, with a byte[][] case added to dataLength/stratifiedSample. MaskedEncodingEncoder's denseValues (substituting a placeholder for null values-child entries before the cascade reads them) now handles byte[][] alongside String[]. Also fixes an adjacent gap this surfaced: VortexWriter's row-count validation (arrayLength) had no byte[][] case, so a non-nullable DType.Binary column could not be written at all — only nullable ones worked, because NullableData carries its own length. Fixes #352 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019NqSD9faDK9WHNektV8ZPH
1 parent bc81025 commit 6fba232

13 files changed

Lines changed: 370 additions & 111 deletions

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- `FsstEncodingEncoder`, `VarBinViewEncodingEncoder`, and `ZstdEncodingEncoder` now handle `DType.Binary` byte-for-byte instead of casting straight to `String[]` (a `ClassCastException` if the cascade competition picked one of them for an embedded blob column, e.g. Raincloud's `waxal-dagbani-asr-test` `audio.bytes`); `DType.Binary` also joins `DType.Utf8` in the cascade's real sample-and-measure competition instead of a first-match dispatch, so it gets the same Dict/FSST/VarBinView/Zstd contest Utf8 already had. A non-nullable `DType.Binary` column could not be written at all before this fix (`VortexWriter`'s row-count validation had no `byte[][]` case). ([#352](https://github.com/dfa1/vortex-java/issues/352))
13+
1014
### Changed
1115

1216
- `dev.vortex:vortex-jni` 0.84.0 → 0.85.0; vortex-jni's writer no longer emits a per-zone `SUM` in the `vortex.zoned` stats table (upstream: a zone sum prunes nothing and its null-on-empty semantics were unsettled), so `ZoneReducer#sum` now falls back to a full scan for Rust-written files instead of pushing the reduction down. ([#360](https://github.com/dfa1/vortex-java/pull/360))

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ private static long arrayLength(Object data) {
352352
case double[] a -> a.length;
353353
case boolean[] a -> a.length;
354354
case String[] a -> a.length;
355+
case byte[][] a -> a.length;
355356
// A struct column's row count is its fields' row count (all fields share length,
356357
// enforced by StructEncodingEncoder); an empty struct carries no rows.
357358
case StructData d -> d.fieldArrays().isEmpty() ? 0L : arrayLength(d.fieldArrays().getFirst());

writer/src/main/java/io/github/dfa1/vortex/writer/encode/CascadingCompressor.java

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ private static int dataLength(Object data) {
3838
case float[] a -> a.length;
3939
case double[] a -> a.length;
4040
case String[] a -> a.length;
41+
case byte[][] a -> a.length;
4142
default -> throw new IllegalArgumentException("unsupported data type: " + data.getClass());
4243
};
4344
}
@@ -97,6 +98,12 @@ case StructData(var fieldArrays) -> {
9798
System.arraycopy(a, srcOff, out, dstOff, len));
9899
yield out;
99100
}
101+
case byte[][] a -> {
102+
byte[][] out = new byte[sampleSize][];
103+
forEachStride(a.length, sampleSize, seed, (srcOff, dstOff, len) ->
104+
System.arraycopy(a, srcOff, out, dstOff, len));
105+
yield out;
106+
}
100107
default -> throw new IllegalArgumentException("unsupported data type: " + data.getClass());
101108
};
102109
}
@@ -160,24 +167,25 @@ private EncodeResult encodeWithCtx(DType dtype, Object data, EncodeContext ctx)
160167
return encodeStruct(structDtype, (StructData) data, ctx);
161168
}
162169

163-
// Utf8: same sample-and-measure competition as Primitive below (Dict, FSST, VarBin,
164-
// Zstd all genuinely compete on measured size) rather than the extension-type
165-
// first-match dispatch. No stats are computed — DictEncodingEncoder.expectedRatio()
166-
// already defers to this path for Utf8 rather than consuming them — and there is no
167-
// cheap analytic "no compression" baseline the way primitiveBytes is for fixed-width
168-
// types, so the competition simply keeps whichever accepting encoder measures
169-
// smallest (VarBinEncodingEncoder unconditionally accepts Utf8, so a winner always
170-
// exists in practice).
171-
if (dtype instanceof DType.Utf8) {
170+
// Utf8/Binary: same sample-and-measure competition as Primitive below (Dict/VarBin
171+
// compete on Utf8 too; FSST, VarBinView, and Zstd compete on both — all genuinely measured)
172+
// rather than the extension-type first-match dispatch. No stats are computed —
173+
// DictEncodingEncoder.expectedRatio() already defers to this path for Utf8 rather than
174+
// consuming them, Binary isn't a Dict candidate at all — and there is no cheap analytic
175+
// "no compression" baseline the way primitiveBytes is for fixed-width types, so the
176+
// competition simply keeps whichever accepting encoder measures smallest
177+
// (VarBinEncodingEncoder unconditionally accepts both, so a winner always exists in
178+
// practice).
179+
if (dtype instanceof DType.Utf8 || dtype instanceof DType.Binary) {
172180
return competeAndEncode(dtype, data, ctx, ArrayStats.EMPTY, sampleSize -> Long.MAX_VALUE);
173181
}
174182

175-
// Remaining non-primitives (extension types, Binary, List, ...): find the accepting
176-
// encoding and splice through it so its cascaded children (e.g. datetimeparts →
177-
// days/seconds/subseconds) are recursively compressed rather than stored as raw
178-
// primitives. Honor the excluded set so spliceResult's notApplicable retry can rotate
179-
// to the next accepting encoding (e.g. DateTimePartsEncoding → ExtEncoding when the
180-
// input is raw storage rather than DateTimePartsData).
183+
// Remaining non-primitives (extension types, List, ...): find the accepting encoding and
184+
// splice through it so its cascaded children (e.g. datetimeparts → days/seconds/
185+
// subseconds) are recursively compressed rather than stored as raw primitives. Honor the
186+
// excluded set so spliceResult's notApplicable retry can rotate to the next accepting
187+
// encoding (e.g. DateTimePartsEncoding → ExtEncoding when the input is raw storage rather
188+
// than DateTimePartsData).
181189
if (!(dtype instanceof DType.Primitive p)) {
182190
return spliceResult(findPrimitiveEncoding(dtype, ctx.excluded()), dtype, data, ctx);
183191
}
@@ -340,8 +348,8 @@ private EncodeResult encodeStruct(DType.Struct dtype, StructData data, EncodeCon
340348
DType fieldDtype = fieldTypes.get(i);
341349
Object fieldData = fields.get(i);
342350
// Mirrors StructEncodingEncoder's own field loop: a nullable field arrives as
343-
// NullableData(values, validity), not the dense array encodeWithCtx's per-dtype
344-
// dispatch expects (e.g. VarBinEncodingEncoder casts data straight to String[]).
351+
// NullableData(values, validity), not the dense array (String[], byte[][], ...)
352+
// encodeWithCtx's per-dtype dispatch expects.
345353
EncodeResult fieldResult = (fieldData instanceof NullableData && !(fieldDtype instanceof DType.Extension))
346354
? new MaskedEncodingEncoder().encode(fieldDtype, fieldData, ctx)
347355
: encodeWithCtx(fieldDtype, fieldData, ctx);

writer/src/main/java/io/github/dfa1/vortex/writer/encode/FsstEncodingEncoder.java

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,17 @@
1212
import java.lang.foreign.Arena;
1313
import java.lang.foreign.MemorySegment;
1414
import java.lang.foreign.ValueLayout;
15-
import java.nio.charset.StandardCharsets;
1615
import java.util.List;
1716

1817
/// Write-only encoder for `vortex.fsst`.
1918
///
2019
/// This class is a thin wire adapter over the standalone `vortex-fsst` module (issue #287): the
2120
/// FSST compression algorithm — symbol-table training and greedy longest-match compression — lives
22-
/// entirely in [CompressorBuilder]/[Compressor]. This adapter converts the input strings to UTF-8
23-
/// bytes, drives training, compresses each row, and lays the result out in the `vortex.fsst` wire
24-
/// format (symbol table buffers, remapped code stream, per-row uncompressed lengths and code
25-
/// offsets, plus the [ProtoFSSTMetadata] describing the two offset ptypes).
21+
/// entirely in [CompressorBuilder]/[Compressor]. This adapter normalizes the input (Utf8 `String[]`
22+
/// UTF-8 encoded, Binary `byte[][]` passed through — [VarBinBytes]) to raw row bytes, drives
23+
/// training, compresses each row, and lays the result out in the `vortex.fsst` wire format (symbol
24+
/// table buffers, remapped code stream, per-row uncompressed lengths and code offsets, plus the
25+
/// [ProtoFSSTMetadata] describing the two offset ptypes).
2626
///
2727
/// The wire format packs each symbol's bytes LSB-first into a `long` (first byte in the low byte)
2828
/// alongside a per-symbol length byte, and reserves code `0xFF` as the single-literal-byte escape.
@@ -49,15 +49,13 @@ public EncodingId encodingId() {
4949

5050
@Override
5151
public boolean accepts(DType dtype) {
52-
// Binary excluded: encode()/encodeCascade() cast data straight to String[], not byte-safe
53-
// for arbitrary bytes yet (#352).
54-
return dtype instanceof DType.Utf8;
52+
return dtype instanceof DType.Utf8 || dtype instanceof DType.Binary;
5553
}
5654

5755
@Override
5856
public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
5957
Arena arena = ctx.arena();
60-
Fsst c = compress((String[]) data, arena);
58+
Fsst c = compress(data, arena);
6159

6260
// Terminal layout: the per-row length and cumulative-offset children are raw primitive
6361
// segments (buffers 3 and 4). The cascading path (encodeCascade) instead exposes them as
@@ -95,15 +93,15 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
9593
/// to the terminal raw-primitive layout.
9694
///
9795
/// @param dtype the Utf8/Binary type being encoded
98-
/// @param data the string values
96+
/// @param data the Utf8 (`String[]`) or Binary (`byte[][]`) values
9997
/// @param ctx encoding context supplying the arena and cascade depth
10098
/// @return a cascade step with the two offset children left open, or a terminal step at depth 0
10199
@Override
102100
public CascadeStep encodeCascade(DType dtype, Object data, EncodeContext ctx) {
103101
if (ctx.allowedCascading() == 0) {
104102
return CascadeStep.terminal(encode(dtype, data, ctx));
105103
}
106-
Fsst c = compress((String[]) data, ctx.arena());
104+
Fsst c = compress(data, ctx.arena());
107105
Object uncompLens = typedUnsigned(c.uncompLenPType(), c.uncompLens());
108106
Object codesOffsets = typedUnsigned(c.codesOffPType(), c.codesOffsets());
109107
EncodeNode partialRoot = new EncodeNode(
@@ -128,14 +126,13 @@ private record Fsst(
128126
PType uncompLenPType, PType codesOffPType, int n) {
129127
}
130128

131-
private static Fsst compress(String[] strings, Arena arena) {
132-
int n = strings.length;
129+
private static Fsst compress(Object data, Arena arena) {
130+
byte[][] byteArrays = VarBinBytes.toByteArrays(data);
131+
int n = byteArrays.length;
133132

134-
byte[][] byteArrays = new byte[n][];
135133
long totalInput = 0;
136134
int maxUncompLen = 0;
137135
for (int i = 0; i < n; i++) {
138-
byteArrays[i] = strings[i].getBytes(StandardCharsets.UTF_8);
139136
totalInput += byteArrays[i].length;
140137
maxUncompLen = Math.max(maxUncompLen, byteArrays[i].length);
141138
}

writer/src/main/java/io/github/dfa1/vortex/writer/encode/MaskedEncodingEncoder.java

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
/// (primitive / varbin / fixed-size-list).
2020
public final class MaskedEncodingEncoder implements EncodingEncoder {
2121

22+
private static final byte[] EMPTY_BYTES = new byte[0];
23+
2224
private static final List<EncodingEncoder> INNER_FALLBACK = List.of(
2325
new PrimitiveEncodingEncoder(),
2426
new VarBinEncodingEncoder(),
@@ -69,11 +71,11 @@ public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
6971
/// never selects itself: its [#accepts] returns `false`, so the compressor cannot recurse into it.
7072
private static EncodeResult encodeValues(DType nonNullable, Object values, EncodeContext ctx) {
7173
if (ctx.allowedCascading() > 0) {
72-
// The NullableData Utf8/Binary carrier keeps null elements in the String[] at null
73-
// positions (validity masks them). Dict/FSST call getBytes() on every element, so
74-
// substitute the empty string for nulls first — matching VarBinEncodingEncoder, which
75-
// encodes a null as a zero-length slot. Those slots are never read: the enclosing
76-
// vortex.masked validity bitmap marks the rows null.
74+
// The NullableData Utf8/Binary carrier keeps null elements at null positions in the
75+
// String[]/byte[][] (validity masks them). Dict/FSST/Zstd call getBytes()/read the row
76+
// bytes of every element, so substitute an empty placeholder for nulls first —
77+
// matching VarBinEncodingEncoder, which encodes a null as a zero-length slot. Those
78+
// slots are never read: the enclosing vortex.masked validity bitmap marks the rows null.
7779
Object dense = denseValues(values);
7880
List<EncodingEncoder> candidates =
7981
List.copyOf(ctx.registry().encoderMap().values());
@@ -142,15 +144,23 @@ private static boolean isConstantValidity(boolean[] validity) {
142144
return true;
143145
}
144146

145-
/// Returns `values` unchanged, except a `String[]` with null elements is copied with each null
146-
/// replaced by the empty string so cascade encoders (Dict, FSST) can call `getBytes()` safely.
147+
/// Returns `values` unchanged, except a `String[]`/`byte[][]` with null elements is copied
148+
/// with each null replaced by an empty placeholder so cascade encoders (Dict, FSST, Zstd,
149+
/// VarBinView) can read every element's bytes safely.
147150
///
148151
/// @param values the non-nullable values carrier extracted from the [NullableData]
149-
/// @return the same array, or a null-free copy when it is a `String[]` containing nulls
152+
/// @return the same array, or a null-free copy when it is a `String[]`/`byte[][]` containing nulls
150153
private static Object denseValues(Object values) {
151-
if (!(values instanceof String[] strings)) {
152-
return values;
154+
if (values instanceof String[] strings) {
155+
return densifyStrings(strings);
156+
}
157+
if (values instanceof byte[][] raw) {
158+
return densifyBytes(raw);
153159
}
160+
return values;
161+
}
162+
163+
private static String[] densifyStrings(String[] strings) {
154164
String[] out = null;
155165
for (int i = 0; i < strings.length; i++) {
156166
if (strings[i] == null) {
@@ -163,6 +173,19 @@ private static Object denseValues(Object values) {
163173
return out != null ? out : strings;
164174
}
165175

176+
private static byte[][] densifyBytes(byte[][] raw) {
177+
byte[][] out = null;
178+
for (int i = 0; i < raw.length; i++) {
179+
if (raw[i] == null) {
180+
if (out == null) {
181+
out = raw.clone();
182+
}
183+
out[i] = EMPTY_BYTES;
184+
}
185+
}
186+
return out != null ? out : raw;
187+
}
188+
166189
private static EncodingEncoder pickInner(DType nonNullable) {
167190
for (EncodingEncoder e : INNER_FALLBACK) {
168191
if (e.accepts(nonNullable)) {
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package io.github.dfa1.vortex.writer.encode;
2+
3+
import java.nio.charset.StandardCharsets;
4+
5+
/// Normalizes Utf8 (`String[]`) or Binary (`byte[][]`) encoder input to a common `byte[][]`
6+
/// shape — Utf8 elements are UTF-8 encoded, Binary elements pass through unchanged. Shared by
7+
/// every varbin-family encoder ([VarBinEncodingEncoder], [VarBinViewEncodingEncoder],
8+
/// [FsstEncodingEncoder], [ZstdEncodingEncoder]) so a `DType.Binary` column gets the same
9+
/// byte-safe treatment `DType.Utf8` already had (issue #352).
10+
final class VarBinBytes {
11+
12+
private static final byte[] EMPTY = new byte[0];
13+
14+
private VarBinBytes() {
15+
}
16+
17+
/// Converts `data` to `byte[][]`, preserving `null` entries as Java `null` rather than
18+
/// substituting a placeholder.
19+
///
20+
/// @param data a `String[]` (UTF-8 encoded) or `byte[][]` (returned row-for-row unchanged)
21+
/// @return the row bytes, with any `null` entries preserved
22+
static byte[][] toRawByteArrays(Object data) {
23+
if (data instanceof byte[][] raw) {
24+
return raw;
25+
}
26+
String[] strings = (String[]) data;
27+
byte[][] out = new byte[strings.length][];
28+
for (int i = 0; i < strings.length; i++) {
29+
out[i] = strings[i] == null ? null : strings[i].getBytes(StandardCharsets.UTF_8);
30+
}
31+
return out;
32+
}
33+
34+
/// Like [#toRawByteArrays(Object)], but substitutes a zero-length array for every `null`
35+
/// entry — the values child of a masked/nullable layout, where validity (not this array)
36+
/// carries nullity, so a null entry's bytes are never read back.
37+
///
38+
/// @param data a `String[]` (UTF-8 encoded) or `byte[][]` (returned row-for-row unchanged)
39+
/// @return the row bytes, with `null` entries replaced by a zero-length array
40+
static byte[][] toByteArrays(Object data) {
41+
byte[][] raw = toRawByteArrays(data);
42+
byte[][] out = new byte[raw.length][];
43+
for (int i = 0; i < raw.length; i++) {
44+
out[i] = raw[i] == null ? EMPTY : raw[i];
45+
}
46+
return out;
47+
}
48+
}

writer/src/main/java/io/github/dfa1/vortex/writer/encode/VarBinEncodingEncoder.java

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,11 @@
99

1010
import java.lang.foreign.Arena;
1111
import java.lang.foreign.MemorySegment;
12-
import java.nio.charset.StandardCharsets;
1312
import java.util.List;
1413

1514
/// Write-only encoder for `vortex.varbin`.
1615
public final class VarBinEncodingEncoder implements EncodingEncoder {
1716

18-
private static final byte[] EMPTY_BYTES = new byte[0];
19-
2017
@Override
2118
public EncodingId encodingId() {
2219
return EncodingId.VORTEX_VARBIN;
@@ -30,24 +27,12 @@ public boolean accepts(DType dtype) {
3027
@Override
3128
public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
3229
// Binary (DType.Binary) arrives as raw byte[][] and must round-trip byte-for-byte —
33-
// routing it through the Utf8 String[] path below would corrupt any byte sequence that
34-
// isn't valid UTF-8 (e.g. an embedded audio/image blob). Utf8 arrives as String[] and is
35-
// UTF-8 encoded. Either way a null entry (this encoder is the values child of a
36-
// masked/nullable layout, where validity carries nullity) contributes a zero-length slot.
37-
byte[][] byteArrays;
38-
String[] strings = null;
39-
if (data instanceof byte[][] raw) {
40-
byteArrays = new byte[raw.length][];
41-
for (int i = 0; i < raw.length; i++) {
42-
byteArrays[i] = raw[i] == null ? EMPTY_BYTES : raw[i];
43-
}
44-
} else {
45-
strings = (String[]) data;
46-
byteArrays = new byte[strings.length][];
47-
for (int i = 0; i < strings.length; i++) {
48-
byteArrays[i] = strings[i] == null ? EMPTY_BYTES : strings[i].getBytes(StandardCharsets.UTF_8);
49-
}
50-
}
30+
// routing it through the Utf8 String[] path would corrupt any byte sequence that isn't
31+
// valid UTF-8 (e.g. an embedded audio/image blob). Utf8 arrives as String[] and is UTF-8
32+
// encoded. Either way a null entry (this encoder is the values child of a masked/nullable
33+
// layout, where validity carries nullity) contributes a zero-length slot.
34+
String[] strings = data instanceof String[] s ? s : null;
35+
byte[][] byteArrays = VarBinBytes.toByteArrays(data);
5136
int n = byteArrays.length;
5237
int totalBytes = 0;
5338
for (byte[] b : byteArrays) {

writer/src/main/java/io/github/dfa1/vortex/writer/encode/VarBinViewEncodingEncoder.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
import java.lang.foreign.Arena;
88
import java.lang.foreign.MemorySegment;
9-
import java.nio.charset.StandardCharsets;
109
import java.util.List;
1110

1211
/// Write-only encoder for `vortex.varbinview`.
@@ -22,20 +21,16 @@ public EncodingId encodingId() {
2221

2322
@Override
2423
public boolean accepts(DType dtype) {
25-
// Binary excluded: encode() casts data straight to String[], not byte-safe for
26-
// arbitrary bytes yet (#352).
27-
return dtype instanceof DType.Utf8;
24+
return dtype instanceof DType.Utf8 || dtype instanceof DType.Binary;
2825
}
2926

3027
@Override
3128
public EncodeResult encode(DType dtype, Object data, EncodeContext ctx) {
32-
String[] strings = (String[]) data;
33-
int n = strings.length;
29+
byte[][] bytes = VarBinBytes.toByteArrays(data);
30+
int n = bytes.length;
3431

35-
byte[][] bytes = new byte[n][];
3632
int totalDataBytes = 0;
3733
for (int i = 0; i < n; i++) {
38-
bytes[i] = strings[i].getBytes(StandardCharsets.UTF_8);
3934
if (bytes[i].length > MAX_INLINED_SIZE) {
4035
totalDataBytes += bytes[i].length;
4136
}

0 commit comments

Comments
 (0)