diff --git a/Cargo.lock b/Cargo.lock index 61c277226dd..cfcf47d3a9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3830,8 +3830,7 @@ checksum = "9afc2bd4d5a73106dd53d10d73d3401c2f32730ba2c0b93ddb888a8983680471" [[package]] name = "fastlanes" version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "414cb755aee48ff7b0907995d2949c68c8c17900970076dff6a808e18e592d71" +source = "git+https://github.com/spiraldb/fastlanes.git?branch=adamg%2Factually-compare-packed-bools#56236df6286acca69d63036d7d8681f615d135ac" dependencies = [ "arrayref", "const_for", diff --git a/Cargo.toml b/Cargo.toml index 90436b4f1a9..5211c993ded 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -382,3 +382,6 @@ lto = false [profile.bench_assert] debug-assertions = true inherits = "bench" + +[patch.crates-io] +fastlanes = { git = "https://github.com/spiraldb/fastlanes.git", branch = "adamg/actually-compare-packed-bools" } diff --git a/encodings/fastlanes/public-api.lock b/encodings/fastlanes/public-api.lock index 98354d33f0b..84414d16e23 100644 --- a/encodings/fastlanes/public-api.lock +++ b/encodings/fastlanes/public-api.lock @@ -196,6 +196,14 @@ impl vortex_array::arrays::slice::SliceReduce for vortex_fastlanes::BitPacked pub fn vortex_fastlanes::BitPacked::slice(array: vortex_array::array::view::ArrayView<'_, Self>, range: core::ops::range::Range) -> vortex_error::VortexResult> +impl vortex_array::scalar_fn::fns::between::kernel::BetweenKernel for vortex_fastlanes::BitPacked + +pub fn vortex_fastlanes::BitPacked::between(array: vortex_array::array::view::ArrayView<'_, Self>, lower: &vortex_array::array::erased::ArrayRef, upper: &vortex_array::array::erased::ArrayRef, options: &vortex_array::scalar_fn::fns::between::BetweenOptions, ctx: &mut vortex_array::executor::ExecutionCtx) -> vortex_error::VortexResult> + +impl vortex_array::scalar_fn::fns::binary::compare::CompareKernel for vortex_fastlanes::BitPacked + +pub fn vortex_fastlanes::BitPacked::compare(lhs: vortex_array::array::view::ArrayView<'_, Self>, rhs: &vortex_array::array::erased::ArrayRef, operator: vortex_array::scalar_fn::fns::operators::CompareOperator, ctx: &mut vortex_array::executor::ExecutionCtx) -> vortex_error::VortexResult> + impl vortex_array::scalar_fn::fns::cast::kernel::CastReduce for vortex_fastlanes::BitPacked pub fn vortex_fastlanes::BitPacked::cast(array: vortex_array::array::view::ArrayView<'_, Self>, dtype: &vortex_array::dtype::DType) -> vortex_error::VortexResult> diff --git a/encodings/fastlanes/src/bitpacking/array/mod.rs b/encodings/fastlanes/src/bitpacking/array/mod.rs index 9e8c98b1c04..4dfb895fd0f 100644 --- a/encodings/fastlanes/src/bitpacking/array/mod.rs +++ b/encodings/fastlanes/src/bitpacking/array/mod.rs @@ -518,6 +518,7 @@ mod test { use vortex_array::ToCanonical; use vortex_array::arrays::PrimitiveArray; use vortex_array::assert_arrays_eq; + use vortex_array::validity::Validity; use vortex_buffer::Buffer; use crate::BitPackedData; @@ -562,7 +563,7 @@ mod test { ); assert_arrays_eq!( packed_with_patches.as_array().to_primitive(), - PrimitiveArray::new(values, vortex_array::validity::Validity::NonNullable) + PrimitiveArray::new(values, Validity::NonNullable) ); } } diff --git a/encodings/fastlanes/src/bitpacking/compute/compare.rs b/encodings/fastlanes/src/bitpacking/compute/compare.rs new file mode 100644 index 00000000000..df05b143d3c --- /dev/null +++ b/encodings/fastlanes/src/bitpacking/compute/compare.rs @@ -0,0 +1,647 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +use fastlanes::BitPacking; +use fastlanes::BitPackingCompare; +use fastlanes::FastLanesComparable; +use num_traits::AsPrimitive; +use vortex_array::ArrayRef; +use vortex_array::ArrayView; +use vortex_array::ExecutionCtx; +use vortex_array::IntoArray; +use vortex_array::arrays::BoolArray; +use vortex_array::arrays::PrimitiveArray; +use vortex_array::dtype::NativePType; +use vortex_array::dtype::Nullability; +use vortex_array::dtype::UnsignedPType; +use vortex_array::match_each_integer_ptype; +use vortex_array::match_each_unsigned_integer_ptype; +use vortex_array::patches::Patches; +use vortex_array::scalar_fn::fns::between::BetweenKernel; +use vortex_array::scalar_fn::fns::between::BetweenOptions; +use vortex_array::scalar_fn::fns::between::StrictComparison; +use vortex_array::scalar_fn::fns::binary::CompareKernel; +use vortex_array::scalar_fn::fns::operators::CompareOperator; +use vortex_buffer::BitBufferMut; +use vortex_buffer::BufferMut; +use vortex_error::VortexResult; + +use crate::BitPacked; +use crate::BitPackedArrayExt; +use crate::BitPackedData; + +impl CompareKernel for BitPacked { + fn compare( + lhs: ArrayView<'_, Self>, + rhs: &ArrayRef, + operator: CompareOperator, + ctx: &mut ExecutionCtx, + ) -> VortexResult> { + let Some(constant) = rhs.as_constant() else { + return Ok(None); + }; + + if !constant.dtype().is_int() { + return Ok(None); + } + + match_each_integer_ptype!(lhs.dtype().as_ptype(), |T| { + let value = T::try_from(&constant)?; + compare_constant::(lhs, value, rhs.dtype().nullability(), operator, ctx).map(Some) + }) + } +} + +impl BetweenKernel for BitPacked { + fn between( + array: ArrayView<'_, Self>, + lower: &ArrayRef, + upper: &ArrayRef, + options: &BetweenOptions, + ctx: &mut ExecutionCtx, + ) -> VortexResult> { + let (Some(lower), Some(upper)) = (lower.as_constant(), upper.as_constant()) else { + return Ok(None); + }; + + if !lower.dtype().is_int() || !upper.dtype().is_int() { + return Ok(None); + } + + let nullability = + array.dtype().nullability() | lower.dtype().nullability() | upper.dtype().nullability(); + + match_each_integer_ptype!(array.dtype().as_ptype(), |T| { + let lower = T::try_from(&lower)?; + let upper = T::try_from(&upper)?; + between_constant::(array, lower, upper, nullability, options, ctx).map(Some) + }) + } +} + +fn compare_constant( + array: ArrayView<'_, BitPacked>, + value: T, + nullability: Nullability, + operator: CompareOperator, + ctx: &mut ExecutionCtx, +) -> VortexResult +where + T: NativePType + FastLanesComparable, + ::Bitpacked: UnsignedPType + BitPacking + BitPackingCompare, +{ + compare_constant_typed::<::Bitpacked, T>( + array, + value, + nullability, + operator, + ctx, + ) +} + +fn compare_constant_typed( + array: ArrayView<'_, BitPacked>, + value: T, + nullability: Nullability, + operator: CompareOperator, + ctx: &mut ExecutionCtx, +) -> VortexResult +where + T: NativePType + FastLanesComparable, + U: UnsignedPType + BitPacking + BitPackingCompare, +{ + match operator { + CompareOperator::Eq => { + compare_constant_with::(array, value, nullability, ctx, T::is_eq) + } + CompareOperator::NotEq => { + compare_constant_with::(array, value, nullability, ctx, is_ne::) + } + CompareOperator::Gt => { + compare_constant_with::(array, value, nullability, ctx, T::is_gt) + } + CompareOperator::Gte => { + compare_constant_with::(array, value, nullability, ctx, T::is_ge) + } + CompareOperator::Lt => { + compare_constant_with::(array, value, nullability, ctx, T::is_lt) + } + CompareOperator::Lte => { + compare_constant_with::(array, value, nullability, ctx, T::is_le) + } + } +} + +fn compare_constant_with( + array: ArrayView<'_, BitPacked>, + value: T, + nullability: Nullability, + ctx: &mut ExecutionCtx, + compare: C, +) -> VortexResult +where + T: NativePType + FastLanesComparable, + U: UnsignedPType + BitPacking + BitPackingCompare, + C: Fn(T, T) -> bool + Copy, +{ + let mut bits = collect_chunk_masks::( + array.data(), + array.len(), + array.offset(), + |bit_width, packed_chunk, chunk_matches| unsafe { + U::unchecked_unpack_cmp(bit_width, packed_chunk, chunk_matches, compare, value); + }, + ); + + if let Some(patches) = array.patches() { + apply_patch_predicate::(&mut bits, &patches, ctx, |patched| compare(patched, value))?; + } + + Ok(BoolArray::new( + bits.freeze(), + array.validity()?.union_nullability(nullability), + ) + .into_array()) +} + +fn between_constant( + array: ArrayView<'_, BitPacked>, + lower: T, + upper: T, + nullability: Nullability, + options: &BetweenOptions, + ctx: &mut ExecutionCtx, +) -> VortexResult +where + T: NativePType + FastLanesComparable, + ::Bitpacked: UnsignedPType + BitPacking + BitPackingCompare, +{ + between_constant_typed::<::Bitpacked, T>( + array, + lower, + upper, + nullability, + options, + ctx, + ) +} + +fn between_constant_typed( + array: ArrayView<'_, BitPacked>, + lower: T, + upper: T, + nullability: Nullability, + options: &BetweenOptions, + ctx: &mut ExecutionCtx, +) -> VortexResult +where + T: NativePType + FastLanesComparable, + U: UnsignedPType + BitPacking + BitPackingCompare, +{ + let mut bits = match (options.lower_strict, options.upper_strict) { + (StrictComparison::Strict, StrictComparison::Strict) => { + collect_between_masks::( + array.data(), + array.len(), + array.offset(), + lower, + upper, + NativePType::is_lt, + NativePType::is_lt, + ) + } + (StrictComparison::Strict, StrictComparison::NonStrict) => { + collect_between_masks::( + array.data(), + array.len(), + array.offset(), + lower, + upper, + NativePType::is_lt, + NativePType::is_le, + ) + } + (StrictComparison::NonStrict, StrictComparison::Strict) => { + collect_between_masks::( + array.data(), + array.len(), + array.offset(), + lower, + upper, + NativePType::is_le, + NativePType::is_lt, + ) + } + (StrictComparison::NonStrict, StrictComparison::NonStrict) => { + collect_between_masks::( + array.data(), + array.len(), + array.offset(), + lower, + upper, + NativePType::is_le, + NativePType::is_le, + ) + } + }; + + if let Some(patches) = array.patches() { + apply_patch_predicate::(&mut bits, &patches, ctx, |patched| { + lower_matches_bound(lower, patched, options.lower_strict) + && upper_matches_bound(patched, upper, options.upper_strict) + })?; + } + + Ok(BoolArray::new( + bits.freeze(), + array.validity()?.union_nullability(nullability), + ) + .into_array()) +} + +fn collect_between_masks( + array: &BitPackedData, + len: usize, + offset: u16, + lower: T, + upper: T, + lower_matches: LF, + upper_matches: UF, +) -> BitBufferMut +where + T: NativePType + FastLanesComparable, + U: UnsignedPType + BitPacking, + LF: Fn(T, T) -> bool + Copy, + UF: Fn(T, T) -> bool + Copy, +{ + collect_unpacked_chunk_masks::(array, len, offset, |unpacked, chunk_matches| { + fill_between_chunk::( + unpacked, + chunk_matches, + lower, + upper, + lower_matches, + upper_matches, + ); + }) +} + +fn collect_chunk_masks( + array: &BitPackedData, + len: usize, + offset: u16, + mut fill_chunk: impl FnMut(usize, &[U], &mut [u64; 16]), +) -> BitBufferMut +where + U: UnsignedPType + BitPacking, +{ + if len == 0 { + return BitBufferMut::empty(); + } + + let bit_width = array.bit_width() as usize; + let packed = array.packed_slice::(); + let elems_per_chunk = 128 * bit_width / size_of::(); + let num_chunks = (offset as usize + len).div_ceil(1024); + let mut output = BufferMut::::with_capacity(num_chunks * 16); + + for chunk_idx in 0..num_chunks { + let packed_chunk = &packed[chunk_idx * elems_per_chunk..][..elems_per_chunk]; + append_chunk_matches(&mut output, |chunk_matches| { + fill_chunk(bit_width, packed_chunk, chunk_matches); + }); + } + + let total_len = num_chunks * 1024; + let mut output = BitBufferMut::from_buffer(output.into_byte_buffer(), 0, total_len); + + if offset == 0 { + output.truncate(len); + return output; + } + + BitBufferMut::copy_from( + &output + .freeze() + .slice(offset as usize..offset as usize + len), + ) +} + +fn collect_unpacked_chunk_masks( + array: &BitPackedData, + len: usize, + offset: u16, + mut fill_chunk: impl FnMut(&[U; 1024], &mut [u64; 16]), +) -> BitBufferMut +where + U: UnsignedPType + BitPacking, +{ + if len == 0 { + return BitBufferMut::empty(); + } + + let bit_width = array.bit_width() as usize; + let packed = array.packed_slice::(); + let elems_per_chunk = 128 * bit_width / size_of::(); + let num_chunks = (offset as usize + len).div_ceil(1024); + let mut output = BufferMut::::with_capacity(num_chunks * 16); + let mut unpacked = [U::default(); 1024]; + + for chunk_idx in 0..num_chunks { + let packed_chunk = &packed[chunk_idx * elems_per_chunk..][..elems_per_chunk]; + + unsafe { + U::unchecked_unpack(bit_width, packed_chunk, &mut unpacked); + } + + append_chunk_matches(&mut output, |chunk_matches| { + fill_chunk(&unpacked, chunk_matches); + }); + } + + let total_len = num_chunks * 1024; + let mut output = BitBufferMut::from_buffer(output.into_byte_buffer(), 0, total_len); + + if offset == 0 { + output.truncate(len); + return output; + } + + BitBufferMut::copy_from( + &output + .freeze() + .slice(offset as usize..offset as usize + len), + ) +} + +#[inline] +fn append_chunk_matches(output: &mut BufferMut, fill_chunk: impl FnOnce(&mut [u64; 16])) { + let base_len = output.len(); + + let spare = output.spare_capacity_mut(); + debug_assert!(spare.len() >= 16); + let chunk_matches = unsafe { &mut *(spare.as_mut_ptr().cast::<[u64; 16]>()) }; + + fill_chunk(chunk_matches); + + // SAFETY: `fill_chunk` initializes all 16 words before we expose them via `set_len`. + unsafe { + output.set_len(base_len + 16); + } +} + +#[inline] +fn fill_between_chunk( + unpacked: &[U; 1024], + chunk_matches: &mut [u64; 16], + lower: T, + upper: T, + lower_matches: LF, + upper_matches: UF, +) where + T: NativePType + FastLanesComparable, + U: UnsignedPType, + LF: Fn(T, T) -> bool, + UF: Fn(T, T) -> bool, +{ + for (word_idx, word) in chunk_matches.iter_mut().enumerate() { + let start = word_idx * 64; + let mut mask = 0u64; + + for bit_idx in 0..64 { + let value = T::as_unpacked(unpacked[start + bit_idx]); + mask |= + u64::from(lower_matches(lower, value) && upper_matches(value, upper)) << bit_idx; + } + + *word = mask; + } +} + +fn apply_patch_predicate( + bits: &mut BitBufferMut, + patches: &Patches, + ctx: &mut ExecutionCtx, + mut predicate: impl FnMut(T) -> bool, +) -> VortexResult<()> +where + T: NativePType, +{ + let indices = patches.indices().clone().execute::(ctx)?; + let values = patches.values().clone().execute::(ctx)?; + let values = values.as_slice::(); + let offset = patches.offset(); + + match_each_unsigned_integer_ptype!(indices.ptype(), |I| { + for (&index, &value) in indices.as_slice::().iter().zip(values) { + let absolute_index: usize = index.as_(); + if absolute_index < offset { + continue; + } + + let bit_index = absolute_index - offset; + if bit_index >= bits.len() { + break; + } + + bits.set_to(bit_index, predicate(value)); + } + Ok(()) + }) +} + +#[inline] +fn is_ne(lhs: T, rhs: T) -> bool { + !lhs.is_eq(rhs) +} + +#[inline] +fn lower_matches_bound(lower: T, value: T, strict: StrictComparison) -> bool { + match strict { + StrictComparison::Strict => lower.is_lt(value), + StrictComparison::NonStrict => lower.is_le(value), + } +} + +#[inline] +fn upper_matches_bound(value: T, upper: T, strict: StrictComparison) -> bool { + match strict { + StrictComparison::Strict => value.is_lt(upper), + StrictComparison::NonStrict => value.is_le(upper), + } +} + +#[cfg(test)] +mod tests { + use vortex_array::Canonical; + use vortex_array::IntoArray; + use vortex_array::LEGACY_SESSION; + use vortex_array::VortexSessionExecute; + use vortex_array::arrays::BoolArray; + use vortex_array::arrays::ConstantArray; + use vortex_array::arrays::PrimitiveArray; + use vortex_array::assert_arrays_eq; + use vortex_array::builtins::ArrayBuiltins; + use vortex_array::scalar_fn::fns::between::BetweenOptions; + use vortex_array::scalar_fn::fns::between::StrictComparison; + use vortex_array::scalar_fn::fns::binary::CompareKernel; + use vortex_array::scalar_fn::fns::operators::CompareOperator; + use vortex_array::validity::Validity; + + use crate::BitPacked; + use crate::BitPackedArrayExt; + use crate::bitpack_compress::bitpack_encode; + + fn bp(array: &PrimitiveArray, bit_width: u8) -> crate::BitPackedArray { + bitpack_encode(array, bit_width, None).unwrap() + } + + #[test] + fn compare_unsigned_constant() { + let array = bp(&PrimitiveArray::from_iter([1u32, 2, 3, 4, 5]), 3); + let rhs = ConstantArray::new(3u32, array.len()).into_array(); + + let result = ::compare( + array.as_view(), + &rhs, + CompareOperator::Gt, + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap() + .unwrap(); + + assert_arrays_eq!( + result, + BoolArray::from_iter([false, false, false, true, true]) + ); + } + + #[test] + fn compare_signed_constant() { + let array = bp(&PrimitiveArray::from_iter([1i32, 2, 3, 4, 5]), 3); + let rhs = ConstantArray::new(2i32, array.len()).into_array(); + + let result = ::compare( + array.as_view(), + &rhs, + CompareOperator::Gte, + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap() + .unwrap(); + + assert_arrays_eq!( + result, + BoolArray::from_iter([false, true, true, true, true]) + ); + } + + #[test] + fn compare_with_patches() { + let array = bp(&PrimitiveArray::from_iter(0u32..257), 8); + assert!(array.patches().is_some()); + + let rhs = ConstantArray::new(256u32, array.len()).into_array(); + let result = ::compare( + array.as_view(), + &rhs, + CompareOperator::Eq, + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap() + .unwrap(); + + assert_arrays_eq!( + result, + BoolArray::from_indices(array.len(), [256usize], Validity::NonNullable,) + ); + } + + #[test] + fn compare_nullable() { + let array = bp( + &PrimitiveArray::from_option_iter([Some(1u16), None, Some(3), Some(4), None]), + 3, + ); + let rhs = ConstantArray::new(3u16, array.len()).into_array(); + + let result = ::compare( + array.as_view(), + &rhs, + CompareOperator::Eq, + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap() + .unwrap(); + + assert_arrays_eq!( + result, + BoolArray::from_iter([Some(false), None, Some(true), Some(false), None]) + ); + } + + #[test] + fn binary_compare_pushdown_executes() { + let array = bp(&PrimitiveArray::from_iter([1u32, 2, 3, 4, 5]), 3).into_array(); + let rhs = ConstantArray::new(4u32, array.len()).into_array(); + + let result = array + .binary(rhs, CompareOperator::Lt.into()) + .unwrap() + .execute::(&mut LEGACY_SESSION.create_execution_ctx()) + .unwrap() + .into_array(); + + assert_arrays_eq!( + result, + BoolArray::from_iter([true, true, true, false, false]) + ); + } + + #[test] + fn between_executes_in_encoded_space() { + let array = bp(&PrimitiveArray::from_iter(0u32..257), 8).into_array(); + let len = array.len(); + + let result = array + .between( + ConstantArray::new(255u32, len).into_array(), + ConstantArray::new(256u32, len).into_array(), + BetweenOptions { + lower_strict: StrictComparison::NonStrict, + upper_strict: StrictComparison::NonStrict, + }, + ) + .unwrap() + .execute::(&mut LEGACY_SESSION.create_execution_ctx()) + .unwrap() + .into_array(); + + assert_arrays_eq!( + result, + BoolArray::from_indices(len, [255usize, 256], Validity::NonNullable,) + ); + } + + #[test] + fn between_strict_upper() { + let array = bp(&PrimitiveArray::from_iter([10i32, 11, 12, 13]), 4).into_array(); + let len = array.len(); + + let result = array + .between( + ConstantArray::new(10i32, len).into_array(), + ConstantArray::new(12i32, len).into_array(), + BetweenOptions { + lower_strict: StrictComparison::NonStrict, + upper_strict: StrictComparison::Strict, + }, + ) + .unwrap() + .execute::(&mut LEGACY_SESSION.create_execution_ctx()) + .unwrap() + .into_array(); + + assert_arrays_eq!(result, BoolArray::from_iter([true, true, false, false])); + } +} diff --git a/encodings/fastlanes/src/bitpacking/compute/mod.rs b/encodings/fastlanes/src/bitpacking/compute/mod.rs index f404102c019..c408b3b0fba 100644 --- a/encodings/fastlanes/src/bitpacking/compute/mod.rs +++ b/encodings/fastlanes/src/bitpacking/compute/mod.rs @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright the Vortex contributors mod cast; +mod compare; mod filter; pub(crate) mod is_constant; mod slice; diff --git a/encodings/fastlanes/src/bitpacking/vtable/kernels.rs b/encodings/fastlanes/src/bitpacking/vtable/kernels.rs index 128ff77d99c..d792877202a 100644 --- a/encodings/fastlanes/src/bitpacking/vtable/kernels.rs +++ b/encodings/fastlanes/src/bitpacking/vtable/kernels.rs @@ -4,10 +4,14 @@ use vortex_array::arrays::dict::TakeExecuteAdaptor; use vortex_array::arrays::filter::FilterExecuteAdaptor; use vortex_array::kernel::ParentKernelSet; +use vortex_array::scalar_fn::fns::between::BetweenExecuteAdaptor; +use vortex_array::scalar_fn::fns::binary::CompareExecuteAdaptor; use crate::BitPacked; pub(crate) const PARENT_KERNELS: ParentKernelSet = ParentKernelSet::new(&[ + ParentKernelSet::lift(&CompareExecuteAdaptor(BitPacked)), + ParentKernelSet::lift(&BetweenExecuteAdaptor(BitPacked)), ParentKernelSet::lift(&FilterExecuteAdaptor(BitPacked)), ParentKernelSet::lift(&TakeExecuteAdaptor(BitPacked)), ]); diff --git a/encodings/fastlanes/src/delta/compute/cast.rs b/encodings/fastlanes/src/delta/compute/cast.rs index a9af6d09d60..1fa615021e6 100644 --- a/encodings/fastlanes/src/delta/compute/cast.rs +++ b/encodings/fastlanes/src/delta/compute/cast.rs @@ -57,6 +57,7 @@ mod tests { use vortex_array::dtype::Nullability; use vortex_array::dtype::PType; use vortex_array::session::ArraySession; + use vortex_array::validity::Validity; use vortex_buffer::buffer; use vortex_session::VortexSession; @@ -88,10 +89,7 @@ mod tests { fn test_cast_delta_nullable() { // DeltaArray doesn't support nullable arrays - the validity is handled at the DeltaArray level // Create a non-nullable array and then add validity to the DeltaArray - let values = PrimitiveArray::new( - buffer![100u16, 0, 200, 300, 0], - vortex_array::validity::Validity::NonNullable, - ); + let values = PrimitiveArray::new(buffer![100u16, 0, 200, 300, 0], Validity::NonNullable); let array = Delta::try_from_primitive_array(&values, &mut SESSION.create_execution_ctx()).unwrap(); @@ -109,25 +107,25 @@ mod tests { #[case::u8( PrimitiveArray::new( buffer![0u8, 10, 20, 30, 40, 50], - vortex_array::validity::Validity::NonNullable, + Validity::NonNullable, ) )] #[case::u16( PrimitiveArray::new( buffer![0u16, 100, 200, 300, 400, 500], - vortex_array::validity::Validity::NonNullable, + Validity::NonNullable, ) )] #[case::u32( PrimitiveArray::new( buffer![0u32, 1000, 2000, 3000, 4000], - vortex_array::validity::Validity::NonNullable, + Validity::NonNullable, ) )] #[case::u64( PrimitiveArray::new( buffer![0u64, 10000, 20000, 30000], - vortex_array::validity::Validity::NonNullable, + Validity::NonNullable, ) )] fn test_cast_delta_conformance(#[case] primitive: PrimitiveArray) { diff --git a/encodings/sequence/src/compute/compare.rs b/encodings/sequence/src/compute/compare.rs index c0c9d1367d6..7a0a69cea20 100644 --- a/encodings/sequence/src/compute/compare.rs +++ b/encodings/sequence/src/compute/compare.rs @@ -14,6 +14,7 @@ use vortex_array::scalar::PValue; use vortex_array::scalar::Scalar; use vortex_array::scalar_fn::fns::binary::CompareKernel; use vortex_array::scalar_fn::fns::operators::CompareOperator; +use vortex_array::validity::Validity; use vortex_buffer::BitBuffer; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -51,8 +52,8 @@ impl CompareKernel for Sequence { let nullability = lhs.dtype().nullability() | rhs.dtype().nullability(); let validity = match nullability { - Nullability::NonNullable => vortex_array::validity::Validity::NonNullable, - Nullability::Nullable => vortex_array::validity::Validity::AllValid, + Nullability::NonNullable => Validity::NonNullable, + Nullability::Nullable => Validity::AllValid, }; if let Ok(set_idx) = set_idx { diff --git a/encodings/zstd/src/compute/mod.rs b/encodings/zstd/src/compute/mod.rs index adf6937c246..99f3c21966b 100644 --- a/encodings/zstd/src/compute/mod.rs +++ b/encodings/zstd/src/compute/mod.rs @@ -9,6 +9,7 @@ mod tests { use vortex_array::IntoArray; use vortex_array::arrays::PrimitiveArray; use vortex_array::compute::conformance::consistency::test_array_consistency; + use vortex_array::validity; use vortex_buffer::buffer; use crate::Zstd; @@ -36,26 +37,17 @@ mod tests { } fn zstd_single() -> ZstdArray { - let values = PrimitiveArray::new( - buffer![42i64], - vortex_array::validity::Validity::NonNullable, - ); + let values = PrimitiveArray::new(buffer![42i64], validity::Validity::NonNullable); Zstd::from_primitive(&values, 0, 0).unwrap() } fn zstd_large() -> ZstdArray { - let values = PrimitiveArray::new( - buffer![0u32..1000], - vortex_array::validity::Validity::NonNullable, - ); + let values = PrimitiveArray::new(buffer![0u32..1000], validity::Validity::NonNullable); Zstd::from_primitive(&values, 3, 0).unwrap() } fn zstd_all_same() -> ZstdArray { - let values = PrimitiveArray::new( - buffer![42i32; 100], - vortex_array::validity::Validity::NonNullable, - ); + let values = PrimitiveArray::new(buffer![42i32; 100], validity::Validity::NonNullable); Zstd::from_primitive(&values, 0, 0).unwrap() } diff --git a/encodings/zstd/src/zstd_buffers.rs b/encodings/zstd/src/zstd_buffers.rs index 1e74a69a9a0..9eb77211154 100644 --- a/encodings/zstd/src/zstd_buffers.rs +++ b/encodings/zstd/src/zstd_buffers.rs @@ -21,6 +21,7 @@ use vortex_array::dtype::DType; use vortex_array::scalar::Scalar; use vortex_array::serde::ArrayChildren; use vortex_array::session::ArraySessionExt; +use vortex_array::validity::Validity; use vortex_array::vtable; use vortex_array::vtable::OperationsVTable; use vortex_array::vtable::VTable; @@ -490,11 +491,9 @@ impl OperationsVTable for ZstdBuffers { } impl ValidityVTable for ZstdBuffers { - fn validity( - array: ArrayView<'_, ZstdBuffers>, - ) -> VortexResult { + fn validity(array: ArrayView<'_, ZstdBuffers>) -> VortexResult { if !array.dtype().is_nullable() { - return Ok(vortex_array::validity::Validity::NonNullable); + return Ok(Validity::NonNullable); } let inner_array = array.data().decompress_and_build_inner( diff --git a/vortex-array/src/expr/transform/match_between.rs b/vortex-array/src/expr/transform/match_between.rs index 56f03dbac4d..5d66e7d1c67 100644 --- a/vortex-array/src/expr/transform/match_between.rs +++ b/vortex-array/src/expr/transform/match_between.rs @@ -1,10 +1,11 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +use vortex_error::VortexExpect; + use crate::expr::Expression; use crate::expr::and_collect; use crate::expr::forms::conjuncts; -use crate::expr::lit; use crate::scalar_fn::ScalarFnVTableExt; use crate::scalar_fn::fns::between::Between; use crate::scalar_fn::fns::between::BetweenOptions; @@ -45,7 +46,8 @@ pub fn find_between(expr: Expression) -> Expression { } } - and_collect(rest).unwrap_or_else(|| lit(true)) + debug_assert!(!rest.is_empty()); + and_collect(rest).vortex_expect("find_between must produce at least one conjunct") } fn maybe_match(lhs: &Expression, rhs: &Expression) -> Option {