Skip to content

Commit 7ba2ebf

Browse files
committed
list: simplify list layout reader/writer
- Extract transpose_list_column with named senders in the writer - Fold use_experimental_list_layout into the builder default - Reader: fetch_raw_offsets/elements helpers, clearer names, doc fixes Signed-off-by: Matt Katz <mhkatz97@gmail.com>
1 parent 02ca612 commit 7ba2ebf

5 files changed

Lines changed: 173 additions & 192 deletions

File tree

vortex-file/src/strategy.rs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,10 @@ pub struct WriteStrategyBuilder {
158158
allow_encodings: Option<HashSet<ArrayId>>,
159159
flat_strategy: Option<Arc<dyn LayoutStrategy>>,
160160
probe_compressor: Option<Arc<dyn CompressorPlugin>>,
161-
/// Force list-column decomposition on, overriding the
162-
/// [`use_experimental_list_layout`] env-var default of off.
163-
list_layout: bool,
161+
/// Whether to write list fields using [`ListLayoutStrategy`].
162+
///
163+
/// [`ListLayoutStrategy`]: vortex_layout::layouts::list::writer::ListLayoutStrategy
164+
use_list_layout: bool,
164165
}
165166

166167
impl Default for WriteStrategyBuilder {
@@ -174,7 +175,7 @@ impl Default for WriteStrategyBuilder {
174175
allow_encodings: Some(ALLOWED_ENCODINGS.clone()),
175176
flat_strategy: None,
176177
probe_compressor: None,
177-
list_layout: false,
178+
use_list_layout: use_experimental_list_layout(),
178179
}
179180
}
180181
}
@@ -189,11 +190,11 @@ impl WriteStrategyBuilder {
189190
self
190191
}
191192

192-
/// Enable list-column decomposition, overriding the env-var default of off. When enabled, list
193-
/// columns are written as a `ListLayout` (independently compressed/chunked
194-
/// elements/offsets/validity) rather than as flat arrays.
193+
/// Enable writing list fields with [`ListLayoutStrategy`].
194+
///
195+
/// [`ListLayoutStrategy`]: vortex_layout::layouts::list::writer::ListLayoutStrategy
195196
pub fn with_list_layout(mut self) -> Self {
196-
self.list_layout = true;
197+
self.use_list_layout = true;
197198
self
198199
}
199200

@@ -263,9 +264,7 @@ impl WriteStrategyBuilder {
263264
Arc::new(FlatLayoutStrategy::default())
264265
};
265266

266-
// 7. for each chunk create a flat layout. List columns are decomposed above this point by
267-
// the `TableStrategy` dispatcher (into independently-compressed elements/offsets/validity
268-
// sub-columns), so the per-chunk leaf only ever sees flat, non-list chunks.
267+
// 7. for each chunk create a flat layout
269268
let chunked = ChunkedLayoutStrategy::new(Arc::clone(&flat));
270269
// 6. buffer chunks so they end up with closer segment ids physically
271270
let buffered = BufferedStrategy::new(chunked, 2 * ONE_MEG); // 2MB
@@ -353,9 +352,8 @@ impl WriteStrategyBuilder {
353352
let mut table_strategy =
354353
TableStrategy::new(Arc::new(validity_strategy), Arc::new(repartition))
355354
.with_field_writers(self.field_writers);
356-
// List decomposition is experimental: enabled by an explicit builder opt-in or the
357-
// `VORTEX_EXPERIMENTAL_LIST_LAYOUT` env var; off otherwise.
358-
if self.list_layout || use_experimental_list_layout() {
355+
356+
if self.use_list_layout {
359357
table_strategy = table_strategy.with_list_layout();
360358
}
361359

vortex-layout/src/layouts/list/expr.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ use vortex_array::scalar_fn::fns::is_null::IsNull;
1010
use vortex_array::scalar_fn::fns::list_length::ListLength;
1111
use vortex_error::VortexResult;
1212

13-
/// The deepest list child an expression needs, cheapest first, where I/O cost order is defined as
14-
/// `Validity < OffsetsAndValidity < All`.
13+
/// The minimal set of list children an expression needs for evaluation.
1514
///
1615
/// For example:
1716
/// - `is_null(root())` only needs the validity child.
@@ -21,13 +20,13 @@ use vortex_error::VortexResult;
2120
pub(super) enum ListChildrenNeeded {
2221
/// Only the validity child is needed (`is_null` / `is_not_null`).
2322
Validity,
24-
/// The offsets and validity children are needed, but not the element values (`list_length`).
23+
/// Only the offsets and validity children are needed (`list_length`).
2524
OffsetsAndValidity,
2625
/// All children are needed.
2726
All,
2827
}
2928

30-
/// The minimal list children needed to evaluate `expr`, where `root()` is a field with list dtype.
29+
/// The minimal set of list children needed to evaluate `expr`, where `root()` is a field with list dtype.
3130
pub(super) fn get_necessary_list_children(expr: &Expression) -> ListChildrenNeeded {
3231
if is_null_root(expr) {
3332
return ListChildrenNeeded::Validity;

vortex-layout/src/layouts/list/reader.rs

Lines changed: 49 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,8 @@ impl ListReader {
100100
})
101101
}
102102

103-
/// Projection for [`ListChildrenNeeded::Validity`] expressions (`is_null` / `is_not_null` of
104-
/// the list): reads only the validity child, synthesizing all-valid for a non-nullable list,
105-
/// and never touches the offsets or elements.
103+
/// Projection for [`ListChildrenNeeded::Validity`] expressions. Reads only the validity child,
104+
/// synthesizing all-valid for a non-nullable list, and never touches the offsets or elements.
106105
fn project_validity(
107106
&self,
108107
row_range: &Range<u64>,
@@ -133,28 +132,29 @@ impl ListReader {
133132
};
134133

135134
let validity = create_validity(validity_array, nullability).to_array(out_len);
135+
136136
validity.apply(&rewritten)
137137
}
138138
.boxed())
139139
}
140140

141-
/// Projection for [`ListChildrenNeeded::All`] expressions: materializes the list and applies
141+
/// Projection for [`ListChildrenNeeded::All`] expressions. Materializes the list and applies
142142
/// the expression.
143143
///
144144
/// Dispatches between a bounded read (a strict sub-range over chunked elements, fetching only
145145
/// the element-chunks the range overlaps) and a whole-chunk read (full range, or a single flat
146146
/// elements segment where there is nothing to skip).
147-
fn project_elements(
147+
fn project_all(
148148
&self,
149149
row_range: &Range<u64>,
150150
expr: &Expression,
151151
mask: MaskFuture,
152152
) -> VortexResult<ArrayFuture> {
153153
let is_full_range = row_range.start == 0 && row_range.end == self.layout.row_count();
154154
if is_full_range || !self.elements_are_chunked() {
155-
self.project_elements_whole_chunk(row_range, expr, mask)
155+
self.project_all_concurrent(row_range, expr, mask)
156156
} else {
157-
self.project_elements_bounded(row_range, expr, mask)
157+
self.project_all_elements_bounded(row_range, expr, mask)
158158
}
159159
}
160160

@@ -168,38 +168,28 @@ impl ListReader {
168168
.is_some()
169169
}
170170

171-
/// Whole-chunk read: the entire `elements` buffer, `offsets`, and `validity` are fetched
172-
/// concurrently — no offsets→elements round-trip, because the elements bound is the whole
173-
/// buffer. The assembled list is sliced to `row_range` and filtered by the caller mask in
174-
/// memory.
175-
fn project_elements_whole_chunk(
171+
/// Whole-chunk read: fetches the entire `elements`, `offsets`, and `validity` children
172+
/// concurrently — no offsets→elements round-trip, since the elements bound is the whole buffer.
173+
/// The materialized list is sliced to `row_range` and filtered by the caller mask.
174+
fn project_all_concurrent(
176175
&self,
177176
row_range: &Range<u64>,
178177
expr: &Expression,
179178
mask: MaskFuture,
180179
) -> VortexResult<ArrayFuture> {
181-
let chunk_row_count = self.layout.row_count();
180+
let row_count = self.layout.row_count();
182181
let elements_row_count = self.elements.row_count();
183182
let nullability = self.layout.dtype().nullability();
184183
let expr = expr.clone();
185184
let row_range = row_range.clone();
186185

187186
// Fire all three child reads up front so they run concurrently and overlap the mask await.
188-
// Offsets has one extra entry (`n + 1`).
189-
let offsets_fut = self.offsets.projection_evaluation(
190-
&(0..chunk_row_count + 1),
191-
&root(),
192-
MaskFuture::new_true(usize::try_from(chunk_row_count + 1)?),
193-
)?;
194-
let elements_fut = self.elements.projection_evaluation(
195-
&(0..elements_row_count),
196-
&root(),
197-
MaskFuture::new_true(usize::try_from(elements_row_count)?),
198-
)?;
187+
let offsets_fut = self.fetch_raw_offsets(&(0..row_count))?;
188+
let elements_fut = self.fetch_raw_elements(&(0..elements_row_count))?;
199189
let validity_fut = fetch_validity(
200190
self.validity.as_ref(),
201-
&(0..chunk_row_count),
202-
MaskFuture::new_true(usize::try_from(chunk_row_count)?),
191+
&(0..row_count),
192+
MaskFuture::new_true(usize::try_from(row_count)?),
203193
)?;
204194

205195
Ok(async move {
@@ -209,7 +199,7 @@ impl ListReader {
209199
.into_array();
210200

211201
// Slice the whole-chunk list down to the requested row range.
212-
let list = if row_range.start == 0 && row_range.end == chunk_row_count {
202+
let list = if row_range.start == 0 && row_range.end == row_count {
213203
list
214204
} else {
215205
list.slice(usize::try_from(row_range.start)?..usize::try_from(row_range.end)?)?
@@ -231,9 +221,8 @@ impl ListReader {
231221
/// Bounded read for a strict sub-range over chunked elements. Reads `offsets[row_range]`,
232222
/// decodes the first and last offset to bound the elements read to `[first..last)`, then
233223
/// rebases the offsets to index into that sliced buffer. With chunked elements this fetches
234-
/// only the element-chunks the range overlaps, at the cost of one offsets→elements round-trip
235-
/// (offsets bound the elements, so they are on the critical path; validity is read alongside).
236-
fn project_elements_bounded(
224+
/// only the element-chunks the range overlaps, at the cost of one offsets→elements round-trip.
225+
fn project_all_elements_bounded(
237226
&self,
238227
row_range: &Range<u64>,
239228
expr: &Expression,
@@ -242,33 +231,24 @@ impl ListReader {
242231
let nullability = self.layout.dtype().nullability();
243232
let expr = expr.clone();
244233
let row_count = usize::try_from(row_range.end - row_range.start)?;
245-
let session = self.session.clone();
246-
let elements_reader = Arc::clone(&self.elements);
234+
let reader = self.clone();
247235

248-
let offsets_fut = self.fetch_offsets(row_range)?;
236+
let offsets_fut = self.fetch_raw_offsets(row_range)?;
249237
let validity_fut = fetch_validity(
250238
self.validity.as_ref(),
251239
row_range,
252240
MaskFuture::new_true(row_count),
253241
)?;
254242

255243
Ok(async move {
256-
let offsets = offsets_fut.await?;
257-
let elements_range = calculate_elements_range(&offsets, &session)?;
258-
let elements_len = usize::try_from(elements_range.end - elements_range.start)?;
259-
260-
// Read only the elements this range covers; a chunked elements layout skips the rest.
261-
let elements = elements_reader
262-
.projection_evaluation(
263-
&elements_range,
264-
&root(),
265-
MaskFuture::new_true(elements_len),
266-
)?
267-
.await?;
244+
let (offsets, validity) = try_join!(offsets_fut, validity_fut)?;
245+
let elements_range = elements_range_from_offsets(&offsets, &reader.session)?;
246+
247+
// Read only the elements this range covers.
248+
let elements = reader.fetch_raw_elements(&elements_range)?.await?;
268249

269250
// Rebase the offsets to index into the sliced elements buffer.
270251
let offsets = rebase_offsets(offsets, elements_range.start)?;
271-
let validity = validity_fut.await?;
272252
let list =
273253
ListArray::try_new(elements, offsets, create_validity(validity, nullability))?
274254
.into_array();
@@ -285,16 +265,14 @@ impl ListReader {
285265
.boxed())
286266
}
287267

288-
/// Projection for [`ListChildrenNeeded::OffsetsAndValidity`] expressions (`list_length(root())`
289-
/// and expressions composed from it): reads offsets and list validity, but never touches
290-
/// element values.
291-
fn project_offsets(
268+
/// Projection for [`ListChildrenNeeded::OffsetsAndValidity`] expressions. Only reads offsets and validity children.
269+
fn project_offsets_validity(
292270
&self,
293271
row_range: &Range<u64>,
294272
expr: &Expression,
295273
mask: MaskFuture,
296274
) -> VortexResult<ArrayFuture> {
297-
let offsets = self.fetch_offsets(row_range)?;
275+
let offsets = self.fetch_raw_offsets(row_range)?;
298276
let reader = self.clone();
299277
let row_range = row_range.clone();
300278
let rewritten = rewrite_offsets_expr(expr)?;
@@ -326,9 +304,11 @@ impl ListReader {
326304
.boxed())
327305
}
328306

329-
/// Fire the offsets read for `row_range`. The offsets child has an extra entry, so reading
307+
/// Fire the offsets read for `row_range` in list row space. The offsets child has an extra entry, so reading
330308
/// `row_range` maps to offsets in `[row_range.start..row_range.end + 1)`.
331-
fn fetch_offsets(&self, row_range: &Range<u64>) -> VortexResult<ArrayFuture> {
309+
///
310+
/// No mask or expression is applied.
311+
fn fetch_raw_offsets(&self, row_range: &Range<u64>) -> VortexResult<ArrayFuture> {
332312
let offsets_range = row_range.start..(row_range.end + 1);
333313
let offsets_count = usize::try_from(offsets_range.end - offsets_range.start)?;
334314
self.offsets.projection_evaluation(
@@ -337,6 +317,15 @@ impl ListReader {
337317
MaskFuture::new_true(offsets_count),
338318
)
339319
}
320+
321+
/// Fire the elements read for `row_range` in element space.
322+
///
323+
/// No mask or expression is applied.
324+
fn fetch_raw_elements(&self, row_range: &Range<u64>) -> VortexResult<ArrayFuture> {
325+
let row_count = usize::try_from(row_range.end - row_range.start)?;
326+
self.elements
327+
.projection_evaluation(row_range, &root(), MaskFuture::new_true(row_count))
328+
}
340329
}
341330

342331
fn create_validity(validity_array: Option<ArrayRef>, nullability: Nullability) -> Validity {
@@ -446,8 +435,10 @@ impl LayoutReader for ListReader {
446435
// Read as little as possible based on which list children the expression needs.
447436
match get_necessary_list_children(expr) {
448437
ListChildrenNeeded::Validity => self.project_validity(row_range, expr, mask),
449-
ListChildrenNeeded::OffsetsAndValidity => self.project_offsets(row_range, expr, mask),
450-
ListChildrenNeeded::All => self.project_elements(row_range, expr, mask),
438+
ListChildrenNeeded::OffsetsAndValidity => {
439+
self.project_offsets_validity(row_range, expr, mask)
440+
}
441+
ListChildrenNeeded::All => self.project_all(row_range, expr, mask),
451442
}
452443
}
453444
}
@@ -471,8 +462,8 @@ fn fetch_validity(
471462
.boxed())
472463
}
473464

474-
/// Read `offsets[0]` and `offsets[-1]` and return the elements-buffer range they bound.
475-
fn calculate_elements_range(
465+
/// Read `offsets[0]` and `offsets[-1]` and return the elements range they bound.
466+
fn elements_range_from_offsets(
476467
offsets: &ArrayRef,
477468
session: &VortexSession,
478469
) -> VortexResult<Range<u64>> {
@@ -830,7 +821,7 @@ mod tests {
830821
.downcast_ref::<ListReader>()
831822
.expect("ListReader");
832823

833-
let offsets = reader.fetch_offsets(&(1..3))?.await?;
824+
let offsets = reader.fetch_raw_offsets(&(1..3))?.await?;
834825
assert_eq!(materialize_u64_array(offsets), vec![2u64, 4, 5]);
835826

836827
Ok(())

0 commit comments

Comments
 (0)