Skip to content

Commit 3cc1a5f

Browse files
authored
fix: write empty nested list streams with structural layouts (#10533)
## Summary Writing `list<list<int32>>` rows containing only empty or null outer lists fails with `ListLayoutStrategy needs at least one chunk` when structural list layouts are enabled. This reproduces in PyVortex 0.86.1 and 0.88.0 with `VORTEX_EXPERIMENTAL_LIST_LAYOUT=1`. ## Changes Handle an empty list stream before spawning child writers by returning the same typed zero-row chunked layout used by the flat writer. Pass the first chunk explicitly into the nonempty transposition path, preserving its sequence ID, bounded streaming, and existing layout behavior. Add file write/read regression cases for empty lists, nullable empty lists, all-null lists, and mixed empty/null rows. Replace the old empty-stream rejection test with nullable/nonnullable dtype and row-count checks. ## Validation - All four new file-roundtrip cases fail on unmodified `develop` (`18aebea`) with the reported error. - After the fix, 76 selected tests pass, including all four regression cases and both empty-stream nullability cases: ```sh cargo nextest run --locked -p vortex-file -p vortex-layout -E 'test(nested_) | test(writer_empty_chunks) | test(layouts::list::writer::tests)' ``` - Formatted both changed files with CI's pinned `nightly-2026-09-10` rustfmt; `git diff --check` passes. Implemented with OpenAI Codex assistance. Signed-off-by: Daniel King <dan@spiraldb.com>
1 parent ced05c2 commit 3cc1a5f

2 files changed

Lines changed: 60 additions & 13 deletions

File tree

‎vortex-file/src/tests.rs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2037,6 +2037,30 @@ async fn nested_list_of_list_roundtrip() -> VortexResult<()> {
20372037
Ok(())
20382038
}
20392039

2040+
#[rstest]
2041+
#[case::empty_lists(Validity::NonNullable)]
2042+
#[case::nullable_empty_lists(Validity::AllValid)]
2043+
#[case::null_lists(Validity::AllInvalid)]
2044+
#[case::mixed_empty_and_null(
2045+
Validity::Array(BoolArray::from_iter([true, false]).into_array())
2046+
)]
2047+
#[tokio::test]
2048+
#[cfg_attr(miri, ignore)]
2049+
async fn nested_empty_list_roundtrip(#[case] validity: Validity) -> VortexResult<()> {
2050+
let inner = ListArray::try_new(
2051+
PrimitiveArray::new::<i32>(buffer![], Validity::NonNullable).into_array(),
2052+
buffer![0u32].into_array(),
2053+
Validity::NonNullable,
2054+
)?
2055+
.into_array();
2056+
let outer = ListArray::try_new(inner, buffer![0u32, 0, 0].into_array(), validity)?.into_array();
2057+
let array = StructArray::from_fields(&[("nested", outer)])?.into_array();
2058+
2059+
let result = write_read_roundtrip(array.clone()).await?;
2060+
assert_arrays_eq!(result, array, &mut SESSION.create_execution_ctx());
2061+
Ok(())
2062+
}
2063+
20402064
type MapEntryFixture<'a> = (i32, Option<&'a str>);
20412065
type MapRowFixture<'a> = Option<Vec<MapEntryFixture<'a>>>;
20422066

‎vortex-layout/src/layouts/list/writer.rs‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ use vortex_session::VortexSession;
3333
use crate::LayoutRef;
3434
use crate::LayoutStrategy;
3535
use crate::LayoutWriterContext;
36+
use crate::children::OwnedLayoutChildren;
37+
use crate::layouts::chunked::ChunkedLayout;
3638
use crate::layouts::flat::writer::FlatLayoutStrategy;
3739
use crate::layouts::list::ListLayout;
3840
use crate::segments::SegmentSinkRef;
@@ -117,7 +119,7 @@ impl LayoutStrategy for ListLayoutStrategy {
117119
&self,
118120
ctx: LayoutWriterContext,
119121
segment_sink: SegmentSinkRef,
120-
stream: SendableSequentialStream,
122+
mut stream: SendableSequentialStream,
121123
mut eof: SequencePointer,
122124
session: &VortexSession,
123125
) -> VortexResult<LayoutRef> {
@@ -129,6 +131,20 @@ impl LayoutStrategy for ListLayoutStrategy {
129131
.await;
130132
}
131133

134+
// Empty nested element streams are valid below empty or all-null lists.
135+
// As in the flat writer, no input chunks require no child segments.
136+
let first_chunk = match stream.next().await {
137+
Some(chunk) => chunk?,
138+
None => {
139+
return Ok(ChunkedLayout::new(
140+
0,
141+
dtype,
142+
OwnedLayoutChildren::layout_children(vec![]),
143+
)
144+
.into_layout());
145+
}
146+
};
147+
132148
let is_nullable = dtype.is_nullable();
133149
let element_dtype = dtype
134150
.as_list_element_opt()
@@ -153,6 +169,7 @@ impl LayoutStrategy for ListLayoutStrategy {
153169
// positions. Kept joined with the child writers below so producer errors surface rather
154170
// than being hidden as an early channel close.
155171
let fanout_fut = transpose_list_column(
172+
first_chunk,
156173
stream,
157174
session.clone(),
158175
elements_tx,
@@ -214,10 +231,12 @@ impl LayoutStrategy for ListLayoutStrategy {
214231
/// sub-streams, rebasing each chunk's local `offsets` to global `u64` positions so the single
215232
/// `offsets` child indexes into the concatenated `elements` child.
216233
///
234+
/// `first_chunk` establishes nonempty input; `stream` contains the remaining chunks.
217235
/// `validity_tx` is `Some` exactly when the list is nullable. Errors surface to the caller, which
218236
/// joins this against the child writers, rather than being hidden as an early channel close.
219237
async fn transpose_list_column(
220-
mut stream: SendableSequentialStream,
238+
first_chunk: (SequenceId, ArrayRef),
239+
stream: SendableSequentialStream,
221240
session: VortexSession,
222241
elements_tx: kanal::AsyncSender<ChildChunk>,
223242
offsets_tx: kanal::AsyncSender<ChildChunk>,
@@ -226,10 +245,9 @@ async fn transpose_list_column(
226245
let mut exec_ctx = session.create_execution_ctx();
227246
let mut element_base: u64 = 0;
228247
let mut first = true;
229-
let mut saw_chunk = false;
230-
while let Some(chunk) = stream.next().await {
248+
let mut chunks = futures::stream::iter([Ok(first_chunk)]).chain(stream);
249+
while let Some(chunk) = chunks.next().await {
231250
let (sequence_id, array) = chunk?;
232-
saw_chunk = true;
233251
let mut sp = sequence_id.descend();
234252
let ListDataParts {
235253
elements,
@@ -264,9 +282,6 @@ async fn transpose_list_column(
264282
}
265283
}
266284
}
267-
if !saw_chunk {
268-
vortex_bail!("ListLayoutStrategy needs at least one chunk");
269-
}
270285
Ok(())
271286
}
272287

@@ -326,6 +341,7 @@ impl Matcher for AnyList {
326341
#[cfg(test)]
327342
mod tests {
328343
use futures::stream;
344+
use rstest::rstest;
329345
use vortex_array::ArrayContext;
330346
use vortex_array::VortexSessionExecute;
331347
use vortex_array::array_session;
@@ -440,24 +456,31 @@ mod tests {
440456
Ok(())
441457
}
442458

459+
#[rstest]
460+
#[case::non_nullable(false)]
461+
#[case::nullable(true)]
443462
#[tokio::test]
444-
async fn empty_stream_errors() {
463+
async fn empty_stream_preserves_dtype_and_row_count(
464+
#[case] nullable: bool,
465+
) -> VortexResult<()> {
445466
let segments = Arc::new(TestSegments::default());
446467
let (_, eof) = SequenceId::root().split();
447468
let empty = stream::empty::<VortexResult<(SequenceId, ArrayRef)>>().boxed();
448-
let stream = SequentialStreamAdapter::new(i32_list_dtype(false), empty).sendable();
469+
let stream = SequentialStreamAdapter::new(i32_list_dtype(nullable), empty).sendable();
449470
let session = layout_test_session();
450471

451-
let res = flat_list_strategy()
472+
let layout = flat_list_strategy()
452473
.write_stream(
453474
ArrayContext::empty().into(),
454475
segments,
455476
stream,
456477
eof,
457478
&session,
458479
)
459-
.await;
460-
assert!(res.is_err())
480+
.await?;
481+
assert_eq!(layout.dtype(), &i32_list_dtype(nullable));
482+
assert_eq!(layout.row_count(), 0);
483+
Ok(())
461484
}
462485

463486
#[tokio::test]

0 commit comments

Comments
 (0)