From d997574c5b99f6c830b8b6078de0f3edc1b40aa2 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Thu, 10 Sep 2026 16:50:11 +0100 Subject: [PATCH] Convert trivial filters into slices during reduction, MaskValues::last uses BitBuffer:last_set_index Signed-off-by: Robert Kruszewski --- vortex-array/src/arrays/filter/execute/mod.rs | 9 +++++---- vortex-array/src/arrays/filter/rules.rs | 5 ++++- vortex-array/src/test_harness/trace/tests.rs | 16 +++++++--------- vortex-mask/src/lib.rs | 17 +---------------- vortex-mask/src/tests.rs | 16 ++++++++++++++++ 5 files changed, 33 insertions(+), 30 deletions(-) diff --git a/vortex-array/src/arrays/filter/execute/mod.rs b/vortex-array/src/arrays/filter/execute/mod.rs index 5dc7a2663ec..ca61d34e12f 100644 --- a/vortex-array/src/arrays/filter/execute/mod.rs +++ b/vortex-array/src/arrays/filter/execute/mod.rs @@ -181,6 +181,7 @@ mod tests { use super::*; use crate::VortexSessionExecute; use crate::array_session; + use crate::arrays::Primitive; use crate::arrays::PrimitiveArray; #[test] @@ -194,10 +195,10 @@ mod tests { values.cached_indices().is_none() && values.cached_slices().is_none() })); - let filtered = array - .into_array() - .filter(mask)? - .execute::(&mut array_session().create_execution_ctx())?; + let filtered = array.into_array().filter(mask)?; + assert!(filtered.is::()); + let filtered = + filtered.execute::(&mut array_session().create_execution_ctx())?; let filtered_values = filtered.to_buffer::(); assert_eq!(filtered_values.as_slice(), &(37..91).collect::>()); diff --git a/vortex-array/src/arrays/filter/rules.rs b/vortex-array/src/arrays/filter/rules.rs index 53f35c154cd..5535d2cb52e 100644 --- a/vortex-array/src/arrays/filter/rules.rs +++ b/vortex-array/src/arrays/filter/rules.rs @@ -15,6 +15,7 @@ use crate::arrays::filter::FilterArraySlotsExt; use crate::arrays::filter::FilterReduce; use crate::arrays::filter::FilterReduceAdaptor; use crate::arrays::filter::execute::buffer::prepare_mask_for_reuse; +use crate::arrays::filter::execute::contiguous_filter_range; use crate::arrays::scalar_fn::ExactScalarFn; use crate::arrays::scalar_fn::ScalarFnArrayView; use crate::arrays::struct_::StructDataParts; @@ -67,7 +68,9 @@ impl ArrayReduceRule for TrivialFilterRule { match array.filter_mask() { Mask::AllTrue(_) => Ok(Some(array.child().clone())), Mask::AllFalse(_) => Ok(Some(Canonical::empty(array.dtype()).into_array())), - Mask::Values(_) => Ok(None), + Mask::Values(_) => contiguous_filter_range(array.filter_mask()) + .map(|range| array.child().slice(range)) + .transpose(), } } } diff --git a/vortex-array/src/test_harness/trace/tests.rs b/vortex-array/src/test_harness/trace/tests.rs index bfe99c3857b..9ee2485af0e 100644 --- a/vortex-array/src/test_harness/trace/tests.rs +++ b/vortex-array/src/test_harness/trace/tests.rs @@ -40,9 +40,10 @@ use crate::arrays::Filter; use crate::arrays::FilterArray; use crate::arrays::Primitive; use crate::arrays::PrimitiveArray; +use crate::arrays::Slice; use crate::arrays::StructArray; use crate::arrays::VarBinViewArray; -use crate::arrays::filter::FilterArraySlotsExt; +use crate::arrays::slice::SliceArraySlotsExt; use crate::assert_arrays_eq; use crate::buffer::BufferHandle; use crate::dtype::DType; @@ -385,7 +386,7 @@ optimize root=vortex.filter(i32, len=4) session=false } #[test] -fn trace_optimize_parent_reduce_fixpoint_attempts() -> VortexResult<()> { +fn trace_optimize_contiguous_filter() -> VortexResult<()> { let values = PrimitiveArray::from_iter([0i32, 1, 2, 3, 4, 5]).into_array(); let inner = FilterArray::try_new( values, @@ -402,8 +403,8 @@ fn trace_optimize_parent_reduce_fixpoint_attempts() -> VortexResult<()> { || outer.optimize(), )?; - let optimized_filter = traced.output.as_::(); - assert!(optimized_filter.child().is::()); + let optimized_slice = traced.output.as_::(); + assert!(optimized_slice.child().is::()); assert_arrays_eq!( traced.output, PrimitiveArray::from_iter([2i32, 3]), @@ -411,8 +412,8 @@ fn trace_optimize_parent_reduce_fixpoint_attempts() -> VortexResult<()> { ); insta::assert_snapshot!(traced.trace.to_string(), @r" optimize root=vortex.filter(i32, len=2) session=false - reduce_parent static:FilterReduceAdaptor(Filter) slot=0 parent=vortex.filter(i32, len=2) child=vortex.filter(i32, len=4) -> vortex.filter(i32, len=2) - done output=vortex.filter(i32, len=2) + reduce TrivialFilterRule: vortex.filter(i32, len=2) -> vortex.slice(i32, len=2) + done output=vortex.slice(i32, len=2) "); let mut ctx = ExecutionCtx::new(VortexSession::empty().with::()); @@ -457,9 +458,6 @@ fn trace_optimize_parent_reduce_fixpoint_attempts() -> VortexResult<()> { /// A filter whose mask is not one contiguous run cannot be answered as a slice, so it has to /// execute through its child. -/// -/// The test above happens to build a contiguous combined mask, which short-circuits before the -/// child is reached; without this case no trace would cover an executed filter at all. #[test] fn trace_execute_filter_with_scattered_mask() -> VortexResult<()> { let values = PrimitiveArray::from_iter([0i32, 1, 2, 3, 4, 5]).into_array(); diff --git a/vortex-mask/src/lib.rs b/vortex-mask/src/lib.rs index ac680289ac1..3ac2f0c75ef 100644 --- a/vortex-mask/src/lib.rs +++ b/vortex-mask/src/lib.rs @@ -483,22 +483,7 @@ impl Mask { return slices.last().map(|(_, end)| end - 1); } - if values.true_count == 0 { - return None; - } - - Some( - values - .buffer - .select(values.true_count - 1) - .unwrap_or_else(|| { - vortex_panic!( - "Rank {} out of bounds for mask with true count {}", - values.true_count - 1, - values.true_count - ) - }), - ) + values.buffer.last_set_index() } } } diff --git a/vortex-mask/src/tests.rs b/vortex-mask/src/tests.rs index c1a4928f548..626418852e7 100644 --- a/vortex-mask/src/tests.rs +++ b/vortex-mask/src/tests.rs @@ -104,6 +104,22 @@ fn test_mask_first() { assert_eq!(values_slices.first(), Some(2)); } +#[test] +fn test_mask_last() { + assert_eq!(Mask::new_true(5).last(), Some(4)); + assert_eq!(Mask::new_false(5).last(), None); + + let buffer = BitBuffer::from_iter([true, false, true, false, true, false, true]); + let values = Mask::from_buffer(buffer.slice(1..6)); + assert_eq!(values.last(), Some(3)); + + let values_indices = Mask::from_indices(5, vec![1, 3]); + assert_eq!(values_indices.last(), Some(3)); + + let values_slices = Mask::from_slices(5, vec![(1, 2), (3, 4)]); + assert_eq!(values_slices.last(), Some(3)); +} + #[test] fn test_mask_false_count() { assert_eq!(Mask::new_true(5).false_count(), 0);