From 5b3016885c278275bc612373ebd474eeccd21c9b Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 23 Sep 2026 16:57:26 -0400 Subject: [PATCH] Preserve Boolean buffer handles during mask reduction Signed-off-by: Connor Tsui --- vortex-array/src/arrays/bool/compute/mask.rs | 86 ++++++++++++++++++-- 1 file changed, 81 insertions(+), 5 deletions(-) diff --git a/vortex-array/src/arrays/bool/compute/mask.rs b/vortex-array/src/arrays/bool/compute/mask.rs index 783c933fe5a..43890c42bc8 100644 --- a/vortex-array/src/arrays/bool/compute/mask.rs +++ b/vortex-array/src/arrays/bool/compute/mask.rs @@ -8,31 +8,107 @@ use crate::IntoArray; use crate::array::ArrayView; use crate::arrays::Bool; use crate::arrays::BoolArray; -use crate::arrays::bool::BoolArrayExt; use crate::scalar_fn::fns::mask::MaskReduce; use crate::validity::Validity; impl MaskReduce for Bool { fn mask(array: ArrayView<'_, Bool>, mask: &ArrayRef) -> VortexResult> { Ok(Some( - BoolArray::new( - array.to_bit_buffer(), + BoolArray::try_new_from_handle( + array.bits.clone(), + array.meta.offset(), + array.len(), array.validity()?.and(Validity::Array(mask.clone()))?, - ) + )? .into_array(), )) } } #[cfg(test)] -mod test { +mod tests { + use std::any::Any; + use std::ops::Range; + use std::sync::Arc; + + use futures::future::BoxFuture; use rstest::rstest; + use vortex_buffer::Alignment; + use vortex_buffer::ByteBuffer; + use vortex_error::VortexResult; + use vortex_error::vortex_bail; + use crate::ArrayEq; + use crate::ArrayRef; + use crate::EqMode; use crate::IntoArray; use crate::VortexSessionExecute; use crate::array_session; + use crate::arrays::Bool; use crate::arrays::BoolArray; + use crate::buffer::BufferHandle; + use crate::buffer::DeviceBuffer; use crate::compute::conformance::mask::test_mask_conformance; + use crate::scalar_fn::fns::mask::MaskReduce; + use crate::validity::Validity; + + // Any access beyond buffer metadata would invalidate the metadata-only reduction. + #[derive(Debug, PartialEq, Eq, Hash)] + struct MetadataOnlyDeviceBuffer(usize); + + impl DeviceBuffer for MetadataOnlyDeviceBuffer { + fn as_any(&self) -> &dyn Any { + self + } + + fn len(&self) -> usize { + self.0 + } + + fn alignment(&self) -> Alignment { + Alignment::of::() + } + + fn copy_to_host_sync(&self, _alignment: Alignment) -> VortexResult { + vortex_bail!("mask reduction must not copy device values") + } + + fn copy_to_host( + &self, + _alignment: Alignment, + ) -> VortexResult>> { + vortex_bail!("mask reduction must not copy device values") + } + + fn slice(&self, _range: Range) -> Arc { + panic!("mask reduction must preserve the original buffer"); + } + + fn aligned(self: Arc, alignment: Alignment) -> VortexResult> { + assert_eq!(self.alignment(), alignment); + Ok(self) + } + } + + #[test] + fn mask_preserves_device_buffer_and_bit_offset() -> VortexResult<()> { + let handle = BufferHandle::new_device(Arc::new(MetadataOnlyDeviceBuffer(1))); + let input = BoolArray::try_new_from_handle(handle, 3, 5, Validity::AllValid)?; + let mask = BoolArray::from_iter([true, false, true, false, true]).into_array(); + let masked = ::mask(input.as_ref().as_::(), &mask)? + .expect("Boolean masking must reduce"); + let output = masked.as_::(); + + assert!(output.bits.array_eq(&input.bits, EqMode::Ptr)); + assert_eq!(output.meta.offset(), input.meta.offset()); + assert_eq!(output.len(), input.len()); + let Validity::Array(validity) = masked.validity()? else { + panic!("mask must remain array-backed"); + }; + assert!(ArrayRef::ptr_eq(&validity, &mask)); + + Ok(()) + } #[rstest] #[case(BoolArray::from_iter([true, false, true, true, false]))]