Skip to content

Commit 5a947e8

Browse files
dfa1claude
andcommitted
fix: ChunkedArrayCombiner.combineLists handles all-null NullArray chunks
An entirely-null chunk decodes to a NullArray rather than a typed array (vortex.null flat, or vortex.constant with a null scalar). The sibling #269 fix taught VarBinArray.ChunkedMode.of to materialize such a chunk as an all-null run, but combineLists never got the equivalent treatment: it hit its else branch and threw "chunk is not a ListArray: NullArray". Treat a NullArray list chunk as n zero-length list rows: it contributes no elements (skipped by the recursive element combine) and its outer offsets repeat the running element count. Row-level nullability stays out-of-band via the caller's validity bitmap / MaskedArray wrapping, same convention as VarBinArray.allNull. An all-null column (every chunk a NullArray) yields a zero-length elements placeholder since the elements array is never indexed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent d378b2f commit 5a947e8

3 files changed

Lines changed: 86 additions & 19 deletions

File tree

CHANGELOG.md

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

1212
- A chunked `List` column spanning several flat chunks now decodes into a single stitched array instead of throwing a raw `ClassCastException`; any other unhandled dtype now fails with `VortexException`. ([#268](https://github.com/dfa1/vortex-java/issues/268))
1313
- A chunked `Utf8`/`Binary` column with an entirely-null chunk (`NullArray`) no longer throws `chunk is not a VarBinArray`; the chunk materializes as an all-null run. ([#269](https://github.com/dfa1/vortex-java/issues/269))
14+
- A chunked `List` column with an entirely-null chunk (`NullArray`) no longer throws `chunk is not a ListArray`; the chunk's rows become zero-length lists with out-of-band nulls. ([#269](https://github.com/dfa1/vortex-java/issues/269))
1415
- Nullable low-cardinality columns now share one global dictionary across chunks instead of re-emitting a per-chunk dictionary. ([5fe8b544](https://github.com/dfa1/vortex-java/commit/5fe8b544))
1516
- `MaskedEncodingEncoder` encodes an all-valid or all-invalid validity bitmap as `vortex.constant` instead of a raw per-row bitmap. ([ecd47ead](https://github.com/dfa1/vortex-java/commit/ecd47ead))
1617
- `MaskedEncodingEncoder` tries `vortex.sparse` for a mixed-validity bitmap and keeps it over the raw bitmap when smaller. ([506d036f](https://github.com/dfa1/vortex-java/commit/506d036f))

reader/src/main/java/io/github/dfa1/vortex/reader/array/ChunkedArrayCombiner.java

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -75,52 +75,76 @@ private static Array combinePrimitive(PType ptype, DType dtype, long totalRows,
7575
/// number of elements contributed by earlier chunks, since per-chunk offsets each restart at
7676
/// zero.
7777
///
78+
/// An entirely-null chunk decodes to a [NullArray] rather than a [ListArray] (e.g. a
79+
/// `vortex.null` flat, or `vortex.constant` with a null scalar, #269). Mirroring the sibling
80+
/// [VarBinArray.ChunkedMode] fix, such a chunk contributes `n` zero-length list rows: it adds no
81+
/// elements, so it is skipped by the recursive element combine, and its `n` outer offsets simply
82+
/// repeat the running element count. Row-level nullability is preserved separately by the
83+
/// caller's validity bitmap.
84+
///
7885
/// @param dtype the list's logical type
7986
/// @param totalRows the total outer-list row count across all chunks
80-
/// @param chunks the per-chunk list arrays (each a [ListArray], possibly wrapped [MaskedArray])
87+
/// @param chunks the per-chunk arrays (each a [ListArray] or [NullArray], possibly wrapped
88+
/// [MaskedArray])
8189
/// @param arena allocator for the rebuilt offsets segment
8290
/// @return a single [ListArray] over the combined chunks
91+
/// @throws VortexException on a chunk that is neither a [ListArray] nor a [NullArray], or a
92+
/// row-count mismatch
8393
private static ListArray combineLists(DType.List dtype, long totalRows, List<Array> chunks,
8494
SegmentAllocator arena) {
85-
var listChunks = new ArrayList<ListArray>(chunks.size());
95+
// A ListArray keeps its offsets/elements; a NullArray keeps only its row count (its outer
96+
// rows are all zero-length lists, contributing no elements).
97+
var listChunks = new ArrayList<Array>(chunks.size());
98+
long outerRows = 0;
8699
for (Array chunk : chunks) {
87100
Array unwrapped = chunk instanceof MaskedArray m ? m.inner() : chunk;
88-
if (unwrapped instanceof ListArray la) {
89-
listChunks.add(la);
101+
if (unwrapped instanceof ListArray || unwrapped instanceof NullArray) {
102+
listChunks.add(unwrapped);
103+
outerRows += unwrapped.length();
90104
} else {
91105
throw new VortexException("chunked list: chunk is not a ListArray: "
92106
+ unwrapped.getClass().getSimpleName());
93107
}
94108
}
95-
long outerRows = 0;
96-
for (ListArray la : listChunks) {
97-
outerRows += la.length();
98-
}
99109
if (outerRows != totalRows) {
100110
throw new VortexException("chunked list: chunk rows sum to " + outerRows
101111
+ ", expected " + totalRows);
102112
}
103113

104114
var elementChunks = new ArrayList<Array>(listChunks.size());
105-
for (ListArray la : listChunks) {
106-
elementChunks.add(la.elements());
115+
for (Array chunk : listChunks) {
116+
if (chunk instanceof ListArray la) {
117+
elementChunks.add(la.elements());
118+
}
107119
}
108-
Array combinedElements = combine(dtype.elementType(), sumLengths(elementChunks),
109-
elementChunks, arena);
120+
// Every chunk all-null: no chunk contributes elements. All offsets are zero, so the
121+
// elements array is never indexed; a zero-length placeholder of the element dtype suffices
122+
// (combine() rejects an empty chunk list, so it cannot be called here).
123+
Array combinedElements = elementChunks.isEmpty()
124+
? new NullArray(dtype.elementType(), 0)
125+
: combine(dtype.elementType(), sumLengths(elementChunks), elementChunks, arena);
110126

111127
MemorySegment offsets = arena.allocate((totalRows + 1) * Long.BYTES, Long.BYTES);
112128
offsets.setAtIndex(VortexFormat.LE_LONG, 0, 0L);
113129
long outRow = 0;
114130
long elementBase = 0;
115-
for (ListArray la : listChunks) {
116-
long localRows = la.length();
117-
Array localOffsets = la.offsets();
118-
for (long i = 0; i < localRows; i++) {
119-
long localEnd = readOffset(localOffsets, i + 1);
120-
offsets.setAtIndex(VortexFormat.LE_LONG, outRow + i + 1, elementBase + localEnd);
131+
for (Array chunk : listChunks) {
132+
long localRows = chunk.length();
133+
if (chunk instanceof ListArray la) {
134+
Array localOffsets = la.offsets();
135+
for (long i = 0; i < localRows; i++) {
136+
long localEnd = readOffset(localOffsets, i + 1);
137+
offsets.setAtIndex(VortexFormat.LE_LONG, outRow + i + 1, elementBase + localEnd);
138+
}
139+
elementBase += readOffset(localOffsets, localRows);
140+
} else {
141+
// All-null chunk: every one of its rows is a zero-length list, so the offset never
142+
// advances past the running element base.
143+
for (long i = 0; i < localRows; i++) {
144+
offsets.setAtIndex(VortexFormat.LE_LONG, outRow + i + 1, elementBase);
145+
}
121146
}
122147
outRow += localRows;
123-
elementBase += readOffset(localOffsets, localRows);
124148
}
125149
Array offsetsArray = new MaterializedLongArray(DType.I64, totalRows + 1, offsets.asReadOnly());
126150
return new ListArray(dtype, totalRows, combinedElements, offsetsArray);

reader/src/test/java/io/github/dfa1/vortex/reader/array/ChunkedArrayCombinerTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,48 @@ void nullableListChunksKeepTheirNulls() {
7474
}
7575
}
7676

77+
@Test
78+
void allNullNullArrayChunkCombinesAsZeroLengthLists() {
79+
// Given — a chunked list<int> column whose middle chunk decoded to a bare NullArray, not a
80+
// ListArray (a vortex.null flat or vortex.constant null-scalar chunk, #269). Before the fix
81+
// combineLists hit its else branch and threw "chunk is not a ListArray: NullArray"; the
82+
// #269-parity fix must treat those rows as zero-length lists carrying out-of-band nulls.
83+
ListArray chunk0 = new ListArray(LIST_OF_INT, 2, ints(10, 11, 12), longs(0L, 2L, 3L));
84+
NullArray chunk1 = new NullArray(LIST_OF_INT, 2);
85+
ListArray chunk2 = new ListArray(LIST_OF_INT, 1, ints(20, 21), longs(0L, 2L));
86+
87+
// When
88+
try (Arena arena = Arena.ofConfined()) {
89+
Array result = ChunkedArrayCombiner.combine(LIST_OF_INT, 5,
90+
List.of(chunk0, chunk1, chunk2), arena);
91+
92+
// Then — a NullArray chunk makes the whole column nullable, so the combiner wraps the
93+
// stitched ListArray in a MaskedArray marking the null chunk's rows invalid while the
94+
// real chunks' rows stay valid and their elements/offsets stay intact.
95+
assertThat(result).isInstanceOf(MaskedArray.class);
96+
MaskedArray masked = (MaskedArray) result;
97+
assertThat(masked.inner()).isInstanceOf(ListArray.class);
98+
assertThat(masked.isValid(0)).isTrue();
99+
assertThat(masked.isValid(1)).isTrue();
100+
assertThat(masked.isValid(2)).isFalse();
101+
assertThat(masked.isValid(3)).isFalse();
102+
assertThat(masked.isValid(4)).isTrue();
103+
104+
ListArray list = (ListArray) masked.inner();
105+
assertThat(list.length()).isEqualTo(5);
106+
LongArray offsets = (LongArray) list.offsets();
107+
// Rows 0-1 span [0,2) and [2,3); the null rows 2-3 are zero-length (offset stays at 3);
108+
// row 4 (chunk2) contributes 2 elements, shifted past chunk0's 3 to span [3,5).
109+
assertThat(readAll(offsets, 6)).containsExactly(0L, 2L, 3L, 3L, 3L, 5L);
110+
111+
IntArray elements = (IntArray) list.elements();
112+
assertThat(elements.length()).isEqualTo(5);
113+
assertThat(elements.getInt(0)).isEqualTo(10);
114+
assertThat(elements.getInt(3)).isEqualTo(20);
115+
assertThat(elements.getInt(4)).isEqualTo(21);
116+
}
117+
}
118+
77119
@Test
78120
void emptyChunkListThrowsVortexException() {
79121
// Given / When / Then

0 commit comments

Comments
 (0)