From 88e956d98bd0f7ea77cc6365181783ef200b83e8 Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Wed, 5 Aug 2026 12:10:42 +0100 Subject: [PATCH 1/3] Use RowIdxLayoutReader only when #row_idx is referenced Signed-off-by: Mikhail Kot --- vortex-layout/src/scan/scan_builder.rs | 28 ++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/vortex-layout/src/scan/scan_builder.rs b/vortex-layout/src/scan/scan_builder.rs index 03f8c49649d..1a3f52c0388 100644 --- a/vortex-layout/src/scan/scan_builder.rs +++ b/vortex-layout/src/scan/scan_builder.rs @@ -39,6 +39,7 @@ use vortex_utils::parallelism::get_available_parallelism; use crate::LayoutReader; use crate::LayoutReaderRef; +use crate::layouts::row_idx::RowIdx; use crate::layouts::row_idx::RowIdxLayoutReader; use crate::scan::repeated_scan::RepeatedScan; use crate::scan::split_by::SplitBy; @@ -273,14 +274,18 @@ impl ScanBuilder { // conjunction splitting if a filter is provided. let mut layout_reader = self.layout_reader; - // Enrich the layout reader to support RowIdx expressions. + // Enrich the layout reader to support RowIdx expressions if scan uses #row_idx. // Note that this is applied below the filter layout reader since it can perform // better over individual conjunctions. - layout_reader = Arc::new(RowIdxLayoutReader::new( - self.row_offset, - layout_reader, - self.session.clone(), - )); + if references_row_idx(&self.projection) + || self.filter.as_ref().is_some_and(references_row_idx) + { + layout_reader = Arc::new(RowIdxLayoutReader::new( + self.row_offset, + layout_reader, + self.session.clone(), + )); + } // Normalize and simplify the expressions. let projection = self.projection.optimize_recursive(layout_reader.dtype())?; @@ -431,6 +436,10 @@ impl Stream for LazyScanStream { } } +fn references_row_idx(expr: &Expression) -> bool { + expr.is::() || expr.children().iter().any(references_row_idx) +} + /// Compute masks of field paths referenced by the projection and filter in the scan. /// /// Projection and filter must be pre-simplified. @@ -494,6 +503,7 @@ mod test { use crate::LayoutReader; use crate::RowSplits; use crate::SplitRange; + use crate::layouts::row_idx::row_idx; use crate::scan::test::SCAN_SESSION; use crate::scan::test::session_with_handle; @@ -896,4 +906,10 @@ mod test { Ok(()) } + + #[test] + fn references_row_idx() { + assert!(super::references_row_idx(&eq(row_idx(), lit(3u64)))); + assert!(!super::references_row_idx(&eq(root(), lit(1i32)))); + } } From 92bf395554555923d9b17f92c0600c56dff7c62a Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Wed, 5 Aug 2026 14:12:07 +0100 Subject: [PATCH 2/3] fix Signed-off-by: Mikhail Kot --- vortex-layout/src/scan/scan_builder.rs | 34 +++++++++++++++----------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/vortex-layout/src/scan/scan_builder.rs b/vortex-layout/src/scan/scan_builder.rs index 1a3f52c0388..02a7688b415 100644 --- a/vortex-layout/src/scan/scan_builder.rs +++ b/vortex-layout/src/scan/scan_builder.rs @@ -19,6 +19,8 @@ use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; use vortex_array::expr::analysis::referenced_field_paths; use vortex_array::expr::root; +use vortex_array::expr::traversal::TraversalOrder; +use vortex_array::expr::traversal::pre_order_visit_down; use vortex_array::iter::ArrayIterator; use vortex_array::iter::ArrayIteratorAdapter; use vortex_array::stats::StatsSet; @@ -274,12 +276,27 @@ impl ScanBuilder { // conjunction splitting if a filter is provided. let mut layout_reader = self.layout_reader; + let mut found_row_idx = false; + pre_order_visit_down(&self.projection, |node| { + if node.is::() { + found_row_idx = true; + return Ok(TraversalOrder::Stop); + } + Ok(TraversalOrder::Continue) + })?; + if !found_row_idx && let Some(filter) = self.filter.as_ref() { + pre_order_visit_down(filter, |node| { + if node.is::() { + found_row_idx = true; + return Ok(TraversalOrder::Stop); + } + Ok(TraversalOrder::Continue) + })?; + } // Enrich the layout reader to support RowIdx expressions if scan uses #row_idx. // Note that this is applied below the filter layout reader since it can perform // better over individual conjunctions. - if references_row_idx(&self.projection) - || self.filter.as_ref().is_some_and(references_row_idx) - { + if found_row_idx { layout_reader = Arc::new(RowIdxLayoutReader::new( self.row_offset, layout_reader, @@ -436,10 +453,6 @@ impl Stream for LazyScanStream { } } -fn references_row_idx(expr: &Expression) -> bool { - expr.is::() || expr.children().iter().any(references_row_idx) -} - /// Compute masks of field paths referenced by the projection and filter in the scan. /// /// Projection and filter must be pre-simplified. @@ -503,7 +516,6 @@ mod test { use crate::LayoutReader; use crate::RowSplits; use crate::SplitRange; - use crate::layouts::row_idx::row_idx; use crate::scan::test::SCAN_SESSION; use crate::scan::test::session_with_handle; @@ -906,10 +918,4 @@ mod test { Ok(()) } - - #[test] - fn references_row_idx() { - assert!(super::references_row_idx(&eq(row_idx(), lit(3u64)))); - assert!(!super::references_row_idx(&eq(root(), lit(1i32)))); - } } From 0b819470d8a2ff27d2208a0fe1d4c8f794aade73 Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Wed, 5 Aug 2026 15:26:16 +0100 Subject: [PATCH 3/3] expr.contains Signed-off-by: Mikhail Kot --- vortex-array/src/expr/expression.rs | 27 ++++++++++++++++++++++++++ vortex-array/src/expr/mod.rs | 16 ++++++++++++--- vortex-layout/src/scan/scan_builder.rs | 23 ++++------------------ 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/vortex-array/src/expr/expression.rs b/vortex-array/src/expr/expression.rs index d7f85825dbe..e53474ad040 100644 --- a/vortex-array/src/expr/expression.rs +++ b/vortex-array/src/expr/expression.rs @@ -16,7 +16,10 @@ use vortex_session::VortexSession; use crate::dtype::DType; use crate::expr::display::DisplayTreeExpr; +use crate::expr::traversal::TraversalOrder; +use crate::expr::traversal::pre_order_visit_down; use crate::scalar_fn::ScalarFnRef; +use crate::scalar_fn::ScalarFnVTable; use crate::scalar_fn::fns::root::Root; use crate::stats::rewrite::StatsRewriteCtx; @@ -204,6 +207,30 @@ impl Expression { pub fn display_tree(&self) -> impl Display { DisplayTreeExpr(self) } + + /// Returns true if this expression contains expression E inside. + /// + /// # Example + /// + /// ```rust + /// # use vortex_array::scalar_fn::fns::literal::Literal; + /// # use vortex_array::expr::{eq, lit, root}; + /// let expression = &eq(root(), lit(3u64)); + /// assert!(expression.contains::().unwrap()); + /// let expression = root(); + /// assert!(!expression.contains::().unwrap()); + /// ``` + pub fn contains(&self) -> VortexResult { + let mut contains = false; + pre_order_visit_down(self, |node| { + if node.is::() { + contains = true; + return Ok(TraversalOrder::Stop); + } + Ok(TraversalOrder::Continue) + })?; + Ok(contains) + } } /// The default display implementation for expressions uses the 'SQL'-style format. diff --git a/vortex-array/src/expr/mod.rs b/vortex-array/src/expr/mod.rs index df7e3f59b97..2ae61509ceb 100644 --- a/vortex-array/src/expr/mod.rs +++ b/vortex-array/src/expr/mod.rs @@ -156,6 +156,10 @@ mod tests { use std::collections::hash_map::RandomState; use std::hash::BuildHasher; + use vortex_array::expr::eq; + use vortex_array::expr::lit; + use vortex_array::expr::root; + use super::*; use crate::dtype::DType; use crate::dtype::FieldNames; @@ -164,20 +168,18 @@ mod tests { use crate::dtype::StructFields; use crate::expr::and; use crate::expr::col; - use crate::expr::eq; use crate::expr::get_item; use crate::expr::gt; use crate::expr::gt_eq; - use crate::expr::lit; use crate::expr::lt; use crate::expr::lt_eq; use crate::expr::not; use crate::expr::not_eq; use crate::expr::or; - use crate::expr::root; use crate::expr::select; use crate::expr::select_exclude; use crate::scalar::Scalar; + use crate::scalar_fn::fns::literal::Literal; #[test] fn basic_expr_split_test() { @@ -311,4 +313,12 @@ mod tests { "{dog: 32u32, cat: \"rufus\"}" ); } + + #[test] + fn expr_contains() { + let expression = &eq(root(), lit(3u64)); + assert!(expression.contains::().unwrap()); + let expression = root(); + assert!(!expression.contains::().unwrap()); + } } diff --git a/vortex-layout/src/scan/scan_builder.rs b/vortex-layout/src/scan/scan_builder.rs index 02a7688b415..abce5047109 100644 --- a/vortex-layout/src/scan/scan_builder.rs +++ b/vortex-layout/src/scan/scan_builder.rs @@ -19,8 +19,6 @@ use vortex_array::dtype::FieldMask; use vortex_array::expr::Expression; use vortex_array::expr::analysis::referenced_field_paths; use vortex_array::expr::root; -use vortex_array::expr::traversal::TraversalOrder; -use vortex_array::expr::traversal::pre_order_visit_down; use vortex_array::iter::ArrayIterator; use vortex_array::iter::ArrayIteratorAdapter; use vortex_array::stats::StatsSet; @@ -276,26 +274,13 @@ impl ScanBuilder { // conjunction splitting if a filter is provided. let mut layout_reader = self.layout_reader; - let mut found_row_idx = false; - pre_order_visit_down(&self.projection, |node| { - if node.is::() { - found_row_idx = true; - return Ok(TraversalOrder::Stop); - } - Ok(TraversalOrder::Continue) - })?; - if !found_row_idx && let Some(filter) = self.filter.as_ref() { - pre_order_visit_down(filter, |node| { - if node.is::() { - found_row_idx = true; - return Ok(TraversalOrder::Stop); - } - Ok(TraversalOrder::Continue) - })?; - } // Enrich the layout reader to support RowIdx expressions if scan uses #row_idx. // Note that this is applied below the filter layout reader since it can perform // better over individual conjunctions. + let mut found_row_idx = self.projection.contains::()?; + if !found_row_idx && let Some(filter) = self.filter.as_ref() { + found_row_idx = filter.contains::()?; + } if found_row_idx { layout_reader = Arc::new(RowIdxLayoutReader::new( self.row_offset,