Skip to content

Commit 530cc95

Browse files
committed
Optimize scanned arrays before the scan returns them
Layout readers apply the projection lazily, and apply no longer optimizes, so a scan of a struct returned a pack expression rather than a struct. vx_array_get_field, and the C++ Array::field built on it, require a struct and rejected it. The scan now optimizes each split's array with its session, as does the physical plan's eval stage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018SWbErbGQPZvkCK9ZXH87w Signed-off-by: Claude <noreply@anthropic.com>
1 parent 4350dca commit 530cc95

4 files changed

Lines changed: 82 additions & 4 deletions

File tree

‎vortex-ffi/src/scan.rs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ mod tests {
448448
use std::ptr;
449449
use std::sync::LazyLock;
450450

451+
use tempfile::NamedTempFile;
451452
use vortex::VortexSessionDefault;
452453
use vortex::array::IntoArray;
453454
use vortex::array::arrays::PrimitiveArray;
@@ -462,9 +463,12 @@ mod tests {
462463

463464
use crate::array::vx_array;
464465
use crate::array::vx_array_free;
466+
use crate::array::vx_array_get_field;
465467
use crate::data_source::vx_data_source_free;
466468
use crate::data_source::vx_data_source_new;
467469
use crate::data_source::vx_data_source_options;
470+
use crate::dtype::vx_dtype;
471+
use crate::dtype::vx_dtype_free;
468472
use crate::expression::vx_binary_operator;
469473
use crate::expression::vx_expression_binary;
470474
use crate::expression::vx_expression_free;
@@ -486,6 +490,9 @@ mod tests {
486490
use crate::scan::vx_scan_selection_include;
487491
use crate::session::vx_session_free;
488492
use crate::session::vx_session_new;
493+
use crate::sink::vx_array_sink_close;
494+
use crate::sink::vx_array_sink_open_file;
495+
use crate::sink::vx_array_sink_push;
489496
use crate::string::vx_view;
490497
use crate::tests::SAMPLE_ROWS;
491498
use crate::tests::assert_no_error;
@@ -579,6 +586,62 @@ mod tests {
579586
unsafe { vx_array_free(array) };
580587
}
581588

589+
/// C callers read fields with `vx_array_get_field`, which needs a struct rather than the
590+
/// lazy pack expression a layout reader builds for the projection.
591+
#[test]
592+
#[cfg_attr(miri, ignore)]
593+
fn test_scan_returns_struct() {
594+
let mut ctx = SESSION.create_execution_ctx();
595+
let strings = VarBinViewArray::from_iter_nullable_str([Some("inlined"), Some("y")]);
596+
let struct_array = StructArray::try_new(
597+
["s"].into(),
598+
vec![strings.clone().into_array()],
599+
strings.len(),
600+
Validity::NonNullable,
601+
)
602+
.unwrap();
603+
unsafe {
604+
let session = vx_session_new();
605+
let file = NamedTempFile::new().unwrap();
606+
let path = vx_view::from_str(file.path().to_str().unwrap());
607+
let dtype = vx_dtype::new(struct_array.dtype().clone());
608+
let mut error = ptr::null_mut();
609+
let sink = vx_array_sink_open_file(session, path, dtype, &raw mut error);
610+
assert_no_error(error);
611+
let input = vx_array::new(struct_array.into_array());
612+
vx_array_sink_push(sink, input, &raw mut error);
613+
assert_no_error(error);
614+
vx_array_sink_close(sink, &raw mut error);
615+
assert_no_error(error);
616+
vx_array_free(input);
617+
vx_dtype_free(dtype);
618+
619+
let ds_options = vx_data_source_options {
620+
paths: &raw const path,
621+
paths_len: 1,
622+
};
623+
let ds = vx_data_source_new(session, &raw const ds_options, &raw mut error);
624+
assert_no_error(error);
625+
let scan = vx_data_source_scan(ds, ptr::null(), ptr::null_mut(), &raw mut error);
626+
assert_no_error(error);
627+
let partition = vx_scan_next_partition(scan, &raw mut error);
628+
assert_no_error(error);
629+
let array = vx_partition_next(partition, &raw mut error);
630+
assert_no_error(error);
631+
632+
let field = vx_array_get_field(array, 0, &raw mut error);
633+
assert_no_error(error);
634+
assert_arrays_eq!(vx_array::as_ref(field), strings, &mut ctx);
635+
636+
vx_array_free(field);
637+
vx_array_free(array);
638+
vx_partition_free(partition);
639+
vx_scan_free(scan);
640+
vx_data_source_free(ds);
641+
vx_session_free(session);
642+
}
643+
}
644+
582645
#[test]
583646
#[cfg_attr(miri, ignore)]
584647
fn test_project_single_field() {

‎vortex-layout/src/plan/physical/operators/eval.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! Applying an expression to each batch.
55
66
use vortex_array::expr::BoundExpression;
7+
use vortex_array::optimizer::ArrayOptimizer;
78
use vortex_error::VortexResult;
89

910
use crate::plan::physical::operator::Cx;
@@ -23,10 +24,13 @@ impl EvalStage {
2324
}
2425

2526
impl Operator for EvalStage {
26-
fn compute(&mut self, input: Input, _cx: &mut Cx<'_>) -> VortexResult<Step> {
27+
fn compute(&mut self, input: Input, cx: &mut Cx<'_>) -> VortexResult<Step> {
2728
let Input::Batch(batch) = input else {
2829
return Ok(Step::End);
2930
};
30-
Ok(Step::Final(batch.apply_bound(&self.expression)?))
31+
// `apply_bound` builds the expression lazily. Optimizing it here hands consumers the
32+
// reduced array, such as a struct for a pack of fields, rather than the expression node.
33+
let applied = batch.apply_bound(&self.expression)?;
34+
Ok(Step::Final(applied.optimize(cx.session())?))
3135
}
3236
}

‎vortex-layout/src/scan/repeated_scan.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ impl<A: 'static + Send> RepeatedScan<A> {
165165
.map(|filter| Arc::new(FilterExpr::new(filter))),
166166
reader: Arc::clone(&self.layout_reader),
167167
projection: self.projection.clone(),
168+
session: self.session.clone(),
168169
})
169170
}
170171

‎vortex-layout/src/scan/tasks.rs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ use futures::future::BoxFuture;
1313
use vortex_array::ArrayRef;
1414
use vortex_array::MaskFuture;
1515
use vortex_array::expr::BoundExpression;
16+
use vortex_array::optimizer::ArrayOptimizer;
1617
use vortex_error::VortexResult;
1718
use vortex_mask::Mask;
1819
use vortex_scan::row_mask::RowMask;
20+
use vortex_session::VortexSession;
1921

2022
use crate::LayoutReaderRef;
2123
use crate::scan::filter::FilterExpr;
@@ -50,11 +52,12 @@ pub(crate) fn split_exec(ctx: &TaskContext, read_mask: RowMask) -> VortexResult<
5052
let projection =
5153
ctx.reader
5254
.projection_evaluation(&row_range, &ctx.projection, filter_mask.clone())?;
55+
let session = ctx.session.clone();
5356
Ok(async move {
5457
if filter_mask.await?.all_false() {
5558
return Ok(None);
5659
}
57-
projection.await.map(Some)
60+
projection.await?.optimize(&session).map(Some)
5861
}
5962
.boxed())
6063
}
@@ -71,7 +74,10 @@ pub(crate) fn split_projection(
7174
let projection =
7275
ctx.reader
7376
.projection_evaluation(row_range, &ctx.projection, MaskFuture::ready(mask))?;
74-
Ok(projection.map(|array| array.map(Some)).boxed())
77+
let session = ctx.session.clone();
78+
Ok(projection
79+
.map(move |array| array?.optimize(&session).map(Some))
80+
.boxed())
7581
}
7682

7783
/// Build the filtered mask for a split.
@@ -168,6 +174,10 @@ pub(crate) struct TaskContext {
168174
pub(crate) reader: LayoutReaderRef,
169175
/// The projection expression to apply to gather the scanned rows.
170176
pub(crate) projection: BoundExpression,
177+
/// The session the projected arrays are optimized with before the scan returns them. Readers
178+
/// apply the projection lazily, so without this a caller would see expression nodes, such as
179+
/// a pack, instead of the arrays they reduce to.
180+
pub(crate) session: VortexSession,
171181
}
172182

173183
#[cfg(test)]

0 commit comments

Comments
 (0)