Skip to content

Commit aff0bd3

Browse files
authored
Split TableStrategy dispatcher from StructStrategy (#8638)
Splits the `TableStrategy` writer into a dtype and override dependent dispatcher, and a struct writer. - **`StructStrategy`** — a *structural* writer that only shreds struct streams. It transposes a struct chunk stream into one ordered stream per field (plus validity when nullable) and writes each through a child strategy resolved **by direct field name** (`field_writers` entry, else `default`; validity through `validity`). It is intentionally dtype-unaware and knows nothing about nested dtypes or field-path overrides; it bails on non-struct input. - **`TableStrategy`** — now a *dispatcher*. `write_stream` routes on the stream's dtype and configured per-field overrides. `TableStrategy` continues to own the field-path overrides and resolves the per-field child strategies before handing them to `StructStrategy`. This lets the writer dispatch different layout kinds per dtype — e.g. a list layout for list columns when a list strategy is configured — instead of `TableStrategy` hard-coding struct shredding as its only nested behavior. Signed-off-by: Matt Katz <mhkatz97@gmail.com>
1 parent 07e1b52 commit aff0bd3

5 files changed

Lines changed: 465 additions & 203 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vortex-layout/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ vortex-utils = { workspace = true, features = ["dashmap"] }
5555

5656
[dev-dependencies]
5757
futures = { workspace = true, features = ["executor"] }
58+
insta = { workspace = true }
5859
rstest = { workspace = true }
5960
temp-env = { workspace = true }
6061
tokio = { workspace = true, features = ["rt", "macros"] }

vortex-layout/src/layouts/struct_/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
mod reader;
5+
pub mod writer;
56

67
use std::sync::Arc;
78

@@ -21,6 +22,7 @@ use vortex_error::vortex_err;
2122
use vortex_session::SessionExt;
2223
use vortex_session::VortexSession;
2324
use vortex_session::registry::CachedId;
25+
pub use writer::StructStrategy;
2426

2527
use crate::LayoutBuildContext;
2628
use crate::LayoutChildType;
Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// SPDX-FileCopyrightText: Copyright the Vortex contributors
3+
4+
//! A writer strategy for struct-typed arrays.
5+
//!
6+
//! [`StructStrategy`] transposes a stream of struct chunks into one ordered stream per field
7+
//! (plus a validity stream when the struct is nullable) and writes each through a configurable
8+
//! child strategy, producing a single [`StructLayout`]. It is a *structural* writer: it does not
9+
//! inspect child dtypes or resolve field-path overrides itself. Dispatching a child to the right
10+
//! layout kind is the job of the caller (see [`TableStrategy`]).
11+
//!
12+
//! [`TableStrategy`]: crate::layouts::table::TableStrategy
13+
14+
use std::sync::Arc;
15+
16+
use async_trait::async_trait;
17+
use futures::StreamExt;
18+
use futures::TryStreamExt;
19+
use futures::future::try_join;
20+
use futures::future::try_join_all;
21+
use futures::pin_mut;
22+
use itertools::Itertools;
23+
use vortex_array::ArrayContext;
24+
use vortex_array::ArrayRef;
25+
use vortex_array::IntoArray;
26+
use vortex_array::VortexSessionExecute;
27+
use vortex_array::arrays::StructArray;
28+
use vortex_array::arrays::struct_::StructArrayExt;
29+
use vortex_array::dtype::DType;
30+
use vortex_array::dtype::FieldName;
31+
use vortex_array::dtype::Nullability;
32+
use vortex_error::VortexError;
33+
use vortex_error::VortexResult;
34+
use vortex_error::vortex_bail;
35+
use vortex_io::kanal_ext::KanalExt;
36+
use vortex_io::session::RuntimeSessionExt;
37+
use vortex_session::VortexSession;
38+
use vortex_utils::aliases::DefaultHashBuilder;
39+
use vortex_utils::aliases::hash_map::HashMap;
40+
use vortex_utils::aliases::hash_set::HashSet;
41+
42+
use crate::IntoLayout;
43+
use crate::LayoutRef;
44+
use crate::LayoutStrategy;
45+
use crate::layouts::struct_::StructLayout;
46+
use crate::segments::SegmentSinkRef;
47+
use crate::sequence::SendableSequentialStream;
48+
use crate::sequence::SequenceId;
49+
use crate::sequence::SequencePointer;
50+
use crate::sequence::SequentialStreamAdapter;
51+
use crate::sequence::SequentialStreamExt;
52+
53+
/// Writes struct-typed arrays into a [`StructLayout`], one child layout per field.
54+
///
55+
/// Each field is written through a strategy resolved by direct field name: an entry in
56+
/// `field_writers` if present, otherwise `default`. When the struct is nullable, its validity
57+
/// bitmap is written through `validity`.
58+
///
59+
/// `StructStrategy` is intentionally unaware of nested dtypes and field-path overrides. To write
60+
/// arbitrarily nested struct trees with per-path overrides, drive it from
61+
/// [`TableStrategy`][crate::layouts::table::TableStrategy], which dispatches on dtype and resolves
62+
/// the per-field child strategies before handing them here.
63+
#[derive(Clone)]
64+
pub struct StructStrategy {
65+
/// Per-field child strategies, keyed by direct field name. Fields without an entry use
66+
/// `default`.
67+
field_writers: HashMap<FieldName, Arc<dyn LayoutStrategy>>,
68+
/// Strategy for fields that have no entry in `field_writers`.
69+
default: Arc<dyn LayoutStrategy>,
70+
/// Strategy for the struct's own validity bitmap, used only when the struct is nullable.
71+
validity: Arc<dyn LayoutStrategy>,
72+
}
73+
74+
impl StructStrategy {
75+
/// Create a new struct writer that writes every field through `default` and the validity
76+
/// bitmap (when present) through `validity`.
77+
pub fn new(validity: Arc<dyn LayoutStrategy>, default: Arc<dyn LayoutStrategy>) -> Self {
78+
Self {
79+
field_writers: HashMap::default(),
80+
default,
81+
validity,
82+
}
83+
}
84+
85+
/// Override the strategy for a single field by name.
86+
pub fn with_field_writer(
87+
mut self,
88+
name: impl Into<FieldName>,
89+
writer: Arc<dyn LayoutStrategy>,
90+
) -> Self {
91+
self.field_writers.insert(name.into(), writer);
92+
self
93+
}
94+
95+
/// Override the strategy for several fields by name at once.
96+
pub fn with_field_writers(
97+
mut self,
98+
writers: impl IntoIterator<Item = (FieldName, Arc<dyn LayoutStrategy>)>,
99+
) -> Self {
100+
self.field_writers.extend(writers);
101+
self
102+
}
103+
}
104+
105+
#[async_trait]
106+
impl LayoutStrategy for StructStrategy {
107+
async fn write_stream(
108+
&self,
109+
ctx: ArrayContext,
110+
segment_sink: SegmentSinkRef,
111+
stream: SendableSequentialStream,
112+
mut eof: SequencePointer,
113+
session: &VortexSession,
114+
) -> VortexResult<LayoutRef> {
115+
let dtype = stream.dtype().clone();
116+
117+
let Some(struct_dtype) = dtype.as_struct_fields_opt() else {
118+
vortex_bail!("StructStrategy can only write struct-typed streams, got {dtype}");
119+
};
120+
121+
// Check for unique field names at write time.
122+
if HashSet::<_, DefaultHashBuilder>::from_iter(struct_dtype.names().iter()).len()
123+
!= struct_dtype.names().len()
124+
{
125+
vortex_bail!("StructLayout must have unique field names");
126+
}
127+
let is_nullable = dtype.is_nullable();
128+
129+
// Optimization: when there are no fields, don't spawn any work and just write a trivial
130+
// StructLayout.
131+
if struct_dtype.nfields() == 0 && !is_nullable {
132+
let row_count = stream
133+
.try_fold(
134+
0u64,
135+
|acc, (_, arr)| async move { Ok(acc + arr.len() as u64) },
136+
)
137+
.await?;
138+
return Ok(StructLayout::new(row_count, dtype, vec![]).into_layout());
139+
}
140+
141+
// stream<struct_chunk> -> stream<vec<column_chunk>>
142+
let columns_session = session.clone();
143+
let columns_vec_stream = stream.map(move |chunk| {
144+
let (sequence_id, chunk) = chunk?;
145+
let mut sequence_pointer = sequence_id.descend();
146+
let mut ctx = columns_session.create_execution_ctx();
147+
let struct_chunk = chunk.clone().execute::<StructArray>(&mut ctx)?;
148+
let mut columns: Vec<(SequenceId, ArrayRef)> = Vec::new();
149+
if is_nullable {
150+
columns.push((
151+
sequence_pointer.advance(),
152+
chunk
153+
.validity()?
154+
.execute_mask(chunk.len(), &mut ctx)?
155+
.into_array(),
156+
));
157+
}
158+
159+
columns.extend(
160+
struct_chunk
161+
.iter_unmasked_fields()
162+
.map(|field| (sequence_pointer.advance(), field.clone())),
163+
);
164+
165+
Ok(columns)
166+
});
167+
168+
let mut stream_count = struct_dtype.nfields();
169+
if is_nullable {
170+
stream_count += 1;
171+
}
172+
173+
let (column_streams_tx, column_streams_rx): (Vec<_>, Vec<_>) =
174+
(0..stream_count).map(|_| kanal::bounded_async(1)).unzip();
175+
176+
// Fan out column chunks to their respective transposed streams. Keep this future joined
177+
// with the column writers so producer panics/errors cannot be hidden as channel EOF.
178+
let handle = session.handle();
179+
let fanout_fut = async move {
180+
pin_mut!(columns_vec_stream);
181+
while let Some(result) = columns_vec_stream.next().await {
182+
match result {
183+
Ok(columns) => {
184+
for (tx, column) in column_streams_tx.iter().zip_eq(columns.into_iter()) {
185+
if tx.send(Ok(column)).await.is_err() {
186+
vortex_bail!(
187+
"struct column writer finished before all chunks were sent"
188+
);
189+
}
190+
}
191+
}
192+
Err(e) => {
193+
let e: Arc<VortexError> = Arc::new(e);
194+
for tx in column_streams_tx.iter() {
195+
let _ = tx.send(Err(VortexError::from(Arc::clone(&e)))).await;
196+
}
197+
return Err(VortexError::from(e));
198+
}
199+
}
200+
}
201+
Ok(())
202+
};
203+
204+
// First child column is the validity, subsequent children are the individual struct fields
205+
let column_dtypes: Vec<DType> = if is_nullable {
206+
std::iter::once(DType::Bool(Nullability::NonNullable))
207+
.chain(struct_dtype.fields())
208+
.collect()
209+
} else {
210+
struct_dtype.fields().collect()
211+
};
212+
213+
let column_names: Vec<FieldName> = if is_nullable {
214+
std::iter::once(FieldName::from("__validity"))
215+
.chain(struct_dtype.names().iter().cloned())
216+
.collect()
217+
} else {
218+
struct_dtype.names().iter().cloned().collect()
219+
};
220+
221+
let layout_futures: Vec<_> = column_dtypes
222+
.into_iter()
223+
.zip_eq(column_streams_rx)
224+
.zip_eq(column_names)
225+
.enumerate()
226+
.map(move |(index, ((dtype, recv), name))| {
227+
let column_stream =
228+
SequentialStreamAdapter::new(dtype, recv.into_stream().boxed()).sendable();
229+
let child_eof = eof.split_off();
230+
let session = session.clone();
231+
let ctx = ctx.clone();
232+
let segment_sink = Arc::clone(&segment_sink);
233+
handle.spawn_nested(move |h| {
234+
// Validity is written through the validity strategy; every other field
235+
// resolves to its named override or the default strategy.
236+
let writer = if index == 0 && is_nullable {
237+
Arc::clone(&self.validity)
238+
} else {
239+
self.field_writers
240+
.get(&name)
241+
.cloned()
242+
.unwrap_or_else(|| Arc::clone(&self.default))
243+
};
244+
let session = session.with_handle(h);
245+
246+
async move {
247+
writer
248+
.write_stream(ctx, segment_sink, column_stream, child_eof, &session)
249+
.await
250+
}
251+
})
252+
})
253+
.collect();
254+
255+
let (_success, column_layouts) = try_join(fanout_fut, try_join_all(layout_futures)).await?;
256+
// TODO(os): transposed stream could count row counts as well,
257+
// This must hold though, all columns must have the same row count of the struct layout
258+
let row_count = column_layouts.first().map(|l| l.row_count()).unwrap_or(0);
259+
Ok(StructLayout::new(row_count, dtype, column_layouts).into_layout())
260+
}
261+
}

0 commit comments

Comments
 (0)