From 65925dd257303a87395e6e4f4d14cb017c8d90f9 Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Wed, 18 Dec 2024 22:03:39 -0500 Subject: [PATCH 1/8] fix: BitPackedArray ptype changed by compute funcs --- .../src/bitpacking/compute/filter.rs | 16 +++++++----- .../fastlanes/src/bitpacking/compute/take.rs | 26 ++++++++++++++++--- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/encodings/fastlanes/src/bitpacking/compute/filter.rs b/encodings/fastlanes/src/bitpacking/compute/filter.rs index 3ea778de5..91dfbee79 100644 --- a/encodings/fastlanes/src/bitpacking/compute/filter.rs +++ b/encodings/fastlanes/src/bitpacking/compute/filter.rs @@ -131,7 +131,7 @@ mod test { use itertools::Itertools; use vortex_array::array::PrimitiveArray; use vortex_array::compute::{filter, slice, FilterMask}; - use vortex_array::{ArrayLen, IntoArrayVariant}; + use vortex_array::{ArrayDType, ArrayLen, IntoArrayVariant}; use crate::BitPackedArray; @@ -188,11 +188,15 @@ mod test { bitpacked.as_ref(), FilterMask::from_indices(values.len(), 0..500), ) - .unwrap() - .into_primitive() - .unwrap() - .into_maybe_null_slice::(); + .unwrap(); - assert_eq!(filtered, values); + assert_eq!(filtered.dtype(), bitpacked.dtype()); + + let filtered_values = filtered + .into_primitive() + .unwrap() + .into_maybe_null_slice::(); + + assert_eq!(filtered_values, values); } } diff --git a/encodings/fastlanes/src/bitpacking/compute/take.rs b/encodings/fastlanes/src/bitpacking/compute/take.rs index 3e085cd99..f3ed90402 100644 --- a/encodings/fastlanes/src/bitpacking/compute/take.rs +++ b/encodings/fastlanes/src/bitpacking/compute/take.rs @@ -28,12 +28,12 @@ impl TakeFn for BitPackedEncoding { // NOTE: we use the unsigned PType because all values in the BitPackedArray must // be non-negative (pre-condition of creating the BitPackedArray). - let ptype: PType = PType::try_from(array.dtype())?.to_unsigned(); + let ptype: PType = PType::try_from(array.dtype())?; let validity = array.validity(); let taken_validity = validity.take(indices)?; let indices = indices.clone().into_primitive()?; - let taken = match_each_unsigned_integer_ptype!(ptype, |$T| { + let taken = match_each_unsigned_integer_ptype!(ptype.to_unsigned(), |$T| { match_each_integer_ptype!(indices.ptype(), |$I| { take_primitive::<$T, $I>(array, &indices, taken_validity)? }) @@ -125,7 +125,7 @@ mod test { use rand::{thread_rng, Rng}; use vortex_array::array::PrimitiveArray; use vortex_array::compute::{scalar_at, slice, take}; - use vortex_array::{IntoArrayData, IntoArrayVariant}; + use vortex_array::{ArrayDType, IntoArrayData, IntoArrayVariant}; use crate::BitPackedArray; @@ -210,4 +210,24 @@ mod test { ); }); } + + #[test] + #[cfg_attr(miri, ignore)] + fn take_signed() { + let start = BitPackedArray::encode( + &PrimitiveArray::from(vec![1i32, 2i32, 3i32]).into_array(), + 2, + ) + .unwrap(); + + let taken = take(&start, PrimitiveArray::from(vec![0u64, 1, 2])).unwrap(); + assert_eq!(taken.dtype(), start.dtype()); + + let actual = taken + .into_primitive() + .unwrap() + .into_maybe_null_slice::(); + let expected = vec![1i32, 2, 3]; + assert_eq!(actual, expected); + } } From 11761b528a6ee11cc59e25e1f490fcb1cbf40219 Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Wed, 18 Dec 2024 23:19:36 -0500 Subject: [PATCH 2/8] print query when panicking --- bench-vortex/src/bin/clickbench.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bench-vortex/src/bin/clickbench.rs b/bench-vortex/src/bin/clickbench.rs index 9bfe74248..3bc8ddb06 100644 --- a/bench-vortex/src/bin/clickbench.rs +++ b/bench-vortex/src/bin/clickbench.rs @@ -163,7 +163,9 @@ fn main() { for _ in 0..args.iterations { let exec_duration = runtime.block_on(async { let start = Instant::now(); - execute_query(&context, &query).await.unwrap(); + execute_query(&context, &query) + .await + .expect(&format!("executing query {query_idx}")); start.elapsed() }); From 34be58d3f391c03daf35efd79e0543e5d0088b7a Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Thu, 19 Dec 2024 09:23:47 -0500 Subject: [PATCH 3/8] turn it up to 11 --- .github/workflows/bench-pr.yml | 1 + bench-vortex/src/bin/clickbench.rs | 2 +- encodings/fastlanes/src/bitpacking/compress.rs | 2 +- encodings/fastlanes/src/bitpacking/compute/filter.rs | 7 +++++-- encodings/fastlanes/src/bitpacking/compute/slice.rs | 11 ++++++++--- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/.github/workflows/bench-pr.yml b/.github/workflows/bench-pr.yml index 48c1d2c01..5bb42716d 100644 --- a/.github/workflows/bench-pr.yml +++ b/.github/workflows/bench-pr.yml @@ -181,6 +181,7 @@ jobs: BENCH_VORTEX_RATIOS: '.*' RUSTFLAGS: '-C target-cpu=native' HOME: /home/ci-runner + RUST_BACKTRACE: full run: | cargo run --bin clickbench --release -- -d gh-json | tee clickbench.json - name: Setup AWS CLI diff --git a/bench-vortex/src/bin/clickbench.rs b/bench-vortex/src/bin/clickbench.rs index 3bc8ddb06..c9f702508 100644 --- a/bench-vortex/src/bin/clickbench.rs +++ b/bench-vortex/src/bin/clickbench.rs @@ -165,7 +165,7 @@ fn main() { let start = Instant::now(); execute_query(&context, &query) .await - .expect(&format!("executing query {query_idx}")); + .unwrap_or_else(|e| panic!("executing query {query_idx}: {e}")); start.elapsed() }); diff --git a/encodings/fastlanes/src/bitpacking/compress.rs b/encodings/fastlanes/src/bitpacking/compress.rs index 346019042..715b51d2a 100644 --- a/encodings/fastlanes/src/bitpacking/compress.rs +++ b/encodings/fastlanes/src/bitpacking/compress.rs @@ -194,7 +194,7 @@ pub fn unpack(array: BitPackedArray) -> VortexResult { let length = array.len(); let offset = array.offset() as usize; let ptype = array.ptype(); - let mut unpacked = match_each_unsigned_integer_ptype!(array.ptype().to_unsigned(), |$P| { + let mut unpacked = match_each_unsigned_integer_ptype!(ptype.to_unsigned(), |$P| { PrimitiveArray::from_vec( unpack_primitive::<$P>(array.packed_slice::<$P>(), bit_width, offset, length), array.validity(), diff --git a/encodings/fastlanes/src/bitpacking/compute/filter.rs b/encodings/fastlanes/src/bitpacking/compute/filter.rs index 91dfbee79..8325f76d2 100644 --- a/encodings/fastlanes/src/bitpacking/compute/filter.rs +++ b/encodings/fastlanes/src/bitpacking/compute/filter.rs @@ -3,7 +3,7 @@ use fastlanes::BitPacking; use vortex_array::array::PrimitiveArray; use vortex_array::compute::{filter, FilterFn, FilterIter, FilterMask}; use vortex_array::variants::PrimitiveArrayTrait; -use vortex_array::{ArrayData, IntoArrayData, IntoArrayVariant}; +use vortex_array::{ArrayDType, ArrayData, IntoArrayData, IntoArrayVariant}; use vortex_dtype::{match_each_unsigned_integer_ptype, NativePType}; use vortex_error::VortexResult; @@ -12,11 +12,14 @@ use crate::bitpacking::compute::take::UNPACK_CHUNK_THRESHOLD; use crate::{BitPackedArray, BitPackedEncoding}; impl FilterFn for BitPackedEncoding { + #[allow(clippy::panic_in_result_fn)] fn filter(&self, array: &BitPackedArray, mask: FilterMask) -> VortexResult { let primitive = match_each_unsigned_integer_ptype!(array.ptype().to_unsigned(), |$I| { filter_primitive::<$I>(array, mask) }); - Ok(primitive?.into_array()) + let prim = primitive?; + assert_eq!(prim.dtype(), array.dtype(), "BPA dtype changed in filter!"); + Ok(prim.into_array()) } } diff --git a/encodings/fastlanes/src/bitpacking/compute/slice.rs b/encodings/fastlanes/src/bitpacking/compute/slice.rs index 26f8ef89e..249c6cb7e 100644 --- a/encodings/fastlanes/src/bitpacking/compute/slice.rs +++ b/encodings/fastlanes/src/bitpacking/compute/slice.rs @@ -2,12 +2,13 @@ use std::cmp::max; use vortex_array::compute::SliceFn; use vortex_array::variants::PrimitiveArrayTrait; -use vortex_array::{ArrayData, IntoArrayData}; +use vortex_array::{ArrayDType, ArrayData, IntoArrayData}; use vortex_error::VortexResult; use crate::{BitPackedArray, BitPackedEncoding}; impl SliceFn for BitPackedEncoding { + #[allow(clippy::panic_in_result_fn)] fn slice(&self, array: &BitPackedArray, start: usize, stop: usize) -> VortexResult { let offset_start = start + array.offset() as usize; let offset_stop = stop + array.offset() as usize; @@ -20,7 +21,7 @@ impl SliceFn for BitPackedEncoding { // slice the buffer using the encoded start/stop values // SAFETY: the invariants of the original BitPackedArray are preserved when slicing. - unsafe { + let sliced = unsafe { BitPackedArray::new_unchecked_with_offset( array.packed().slice(encoded_start..encoded_stop), array.ptype(), @@ -35,7 +36,11 @@ impl SliceFn for BitPackedEncoding { offset as u16, ) } - .map(|a| a.into_array()) + .map(|a| a.into_array())?; + + assert_eq!(sliced.dtype(), array.dtype(), "BPA slice changed type"); + + Ok(sliced) } } From 3da5540cb0ce5a03e39270a7fec637177974344b Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Thu, 19 Dec 2024 10:32:18 -0500 Subject: [PATCH 4/8] fix TakeFn --- .github/workflows/bench-pr.yml | 1 - .../src/bitpacking/compute/filter.rs | 7 +--- .../fastlanes/src/bitpacking/compute/slice.rs | 10 ++--- .../fastlanes/src/bitpacking/compute/take.rs | 34 ++++++++------- encodings/fastlanes/src/bitpacking/mod.rs | 31 +++++++++++++- vortex-array/src/array/primitive/patch.rs | 41 +++++++++++++------ 6 files changed, 84 insertions(+), 40 deletions(-) diff --git a/.github/workflows/bench-pr.yml b/.github/workflows/bench-pr.yml index 5bb42716d..48c1d2c01 100644 --- a/.github/workflows/bench-pr.yml +++ b/.github/workflows/bench-pr.yml @@ -181,7 +181,6 @@ jobs: BENCH_VORTEX_RATIOS: '.*' RUSTFLAGS: '-C target-cpu=native' HOME: /home/ci-runner - RUST_BACKTRACE: full run: | cargo run --bin clickbench --release -- -d gh-json | tee clickbench.json - name: Setup AWS CLI diff --git a/encodings/fastlanes/src/bitpacking/compute/filter.rs b/encodings/fastlanes/src/bitpacking/compute/filter.rs index 8325f76d2..91dfbee79 100644 --- a/encodings/fastlanes/src/bitpacking/compute/filter.rs +++ b/encodings/fastlanes/src/bitpacking/compute/filter.rs @@ -3,7 +3,7 @@ use fastlanes::BitPacking; use vortex_array::array::PrimitiveArray; use vortex_array::compute::{filter, FilterFn, FilterIter, FilterMask}; use vortex_array::variants::PrimitiveArrayTrait; -use vortex_array::{ArrayDType, ArrayData, IntoArrayData, IntoArrayVariant}; +use vortex_array::{ArrayData, IntoArrayData, IntoArrayVariant}; use vortex_dtype::{match_each_unsigned_integer_ptype, NativePType}; use vortex_error::VortexResult; @@ -12,14 +12,11 @@ use crate::bitpacking::compute::take::UNPACK_CHUNK_THRESHOLD; use crate::{BitPackedArray, BitPackedEncoding}; impl FilterFn for BitPackedEncoding { - #[allow(clippy::panic_in_result_fn)] fn filter(&self, array: &BitPackedArray, mask: FilterMask) -> VortexResult { let primitive = match_each_unsigned_integer_ptype!(array.ptype().to_unsigned(), |$I| { filter_primitive::<$I>(array, mask) }); - let prim = primitive?; - assert_eq!(prim.dtype(), array.dtype(), "BPA dtype changed in filter!"); - Ok(prim.into_array()) + Ok(primitive?.into_array()) } } diff --git a/encodings/fastlanes/src/bitpacking/compute/slice.rs b/encodings/fastlanes/src/bitpacking/compute/slice.rs index 249c6cb7e..80fdefb02 100644 --- a/encodings/fastlanes/src/bitpacking/compute/slice.rs +++ b/encodings/fastlanes/src/bitpacking/compute/slice.rs @@ -2,7 +2,7 @@ use std::cmp::max; use vortex_array::compute::SliceFn; use vortex_array::variants::PrimitiveArrayTrait; -use vortex_array::{ArrayDType, ArrayData, IntoArrayData}; +use vortex_array::{ArrayData, IntoArrayData}; use vortex_error::VortexResult; use crate::{BitPackedArray, BitPackedEncoding}; @@ -21,7 +21,7 @@ impl SliceFn for BitPackedEncoding { // slice the buffer using the encoded start/stop values // SAFETY: the invariants of the original BitPackedArray are preserved when slicing. - let sliced = unsafe { + unsafe { BitPackedArray::new_unchecked_with_offset( array.packed().slice(encoded_start..encoded_stop), array.ptype(), @@ -36,11 +36,7 @@ impl SliceFn for BitPackedEncoding { offset as u16, ) } - .map(|a| a.into_array())?; - - assert_eq!(sliced.dtype(), array.dtype(), "BPA slice changed type"); - - Ok(sliced) + .map(|a| a.into_array()) } } diff --git a/encodings/fastlanes/src/bitpacking/compute/take.rs b/encodings/fastlanes/src/bitpacking/compute/take.rs index f3ed90402..1939e491c 100644 --- a/encodings/fastlanes/src/bitpacking/compute/take.rs +++ b/encodings/fastlanes/src/bitpacking/compute/take.rs @@ -107,7 +107,11 @@ fn take_primitive( } }); - let unpatched_taken = PrimitiveArray::from_vec(output, taken_validity); + let mut unpatched_taken = PrimitiveArray::from_vec(output, taken_validity); + // Flip back to signed type before patching. + if array.ptype().is_signed_int() { + unpatched_taken = unpatched_taken.reinterpret_cast(array.ptype()); + } if let Some(patches) = array.patches() { if let Some(patches) = patches.take(&indices.to_array())? { return unpatched_taken.patch(patches); @@ -125,7 +129,8 @@ mod test { use rand::{thread_rng, Rng}; use vortex_array::array::PrimitiveArray; use vortex_array::compute::{scalar_at, slice, take}; - use vortex_array::{ArrayDType, IntoArrayData, IntoArrayVariant}; + use vortex_array::validity::Validity; + use vortex_array::{IntoArrayData, IntoArrayVariant}; use crate::BitPackedArray; @@ -213,21 +218,22 @@ mod test { #[test] #[cfg_attr(miri, ignore)] - fn take_signed() { + fn take_signed_with_patches() { let start = BitPackedArray::encode( - &PrimitiveArray::from(vec![1i32, 2i32, 3i32]).into_array(), - 2, + &PrimitiveArray::from(vec![1i32, 2i32, 3i32, 4i32]).into_array(), + 1, ) .unwrap(); - let taken = take(&start, PrimitiveArray::from(vec![0u64, 1, 2])).unwrap(); - assert_eq!(taken.dtype(), start.dtype()); - - let actual = taken - .into_primitive() - .unwrap() - .into_maybe_null_slice::(); - let expected = vec![1i32, 2, 3]; - assert_eq!(actual, expected); + let taken_primitive = super::take_primitive::( + &start, + &PrimitiveArray::from(vec![0u64, 1, 2, 3]), + Validity::NonNullable, + ) + .unwrap(); + assert_eq!( + taken_primitive.into_maybe_null_slice::(), + vec![1i32, 2, 3, 4] + ); } } diff --git a/encodings/fastlanes/src/bitpacking/mod.rs b/encodings/fastlanes/src/bitpacking/mod.rs index edd1ea95a..a79af7030 100644 --- a/encodings/fastlanes/src/bitpacking/mod.rs +++ b/encodings/fastlanes/src/bitpacking/mod.rs @@ -100,6 +100,16 @@ impl BitPackedArray { ); } + if let Some(ref patches) = patches { + if patches.dtype() != &DType::from(ptype) { + vortex_bail!( + "Patches DType {} does not match BitPackedArray dtype {}", + patches.dtype(), + ptype + ) + } + } + // expected packed size is in bytes let expected_packed_size = ((length + offset as usize + 1023) / 1024) * (128 * bit_width as usize); @@ -276,8 +286,9 @@ impl PrimitiveArrayTrait for BitPackedArray {} #[cfg(test)] mod test { + use itertools::Itertools; use vortex_array::array::PrimitiveArray; - use vortex_array::{IntoArrayData, IntoArrayVariant}; + use vortex_array::{IntoArrayData, IntoArrayVariant, IntoCanonical}; use crate::BitPackedArray; @@ -305,4 +316,22 @@ mod test { let _packed = BitPackedArray::encode(uncompressed.as_ref(), 9) .expect_err("Cannot pack value into larger width"); } + + #[test] + fn signed_with_patches() { + let values = (0i32..=512).collect_vec(); + let parray = PrimitiveArray::from(values.clone()).into_array(); + + let packed_with_patches = BitPackedArray::encode(&parray, 9).unwrap(); + assert!(packed_with_patches.patches().is_some()); + assert_eq!( + packed_with_patches + .into_canonical() + .unwrap() + .into_primitive() + .unwrap() + .into_maybe_null_slice::(), + values + ); + } } diff --git a/vortex-array/src/array/primitive/patch.rs b/vortex-array/src/array/primitive/patch.rs index c37d94a37..4b6dc0419 100644 --- a/vortex-array/src/array/primitive/patch.rs +++ b/vortex-array/src/array/primitive/patch.rs @@ -1,33 +1,50 @@ -use itertools::Itertools; -use vortex_dtype::{match_each_integer_ptype, match_each_native_ptype}; +use arrow_buffer::ArrowNativeType; +use vortex_dtype::{match_each_integer_ptype, match_each_native_ptype, NativePType}; use vortex_error::VortexResult; use crate::array::PrimitiveArray; use crate::patches::Patches; +use crate::validity::Validity; use crate::variants::PrimitiveArrayTrait; use crate::{ArrayLen, IntoArrayVariant}; impl PrimitiveArray { #[allow(clippy::cognitive_complexity)] pub fn patch(self, patches: Patches) -> VortexResult { - let (_, indices, values) = patches.into_parts(); - let indices = indices.into_primitive()?; - let values = values.into_primitive()?; + let (_, patch_indices, patch_values) = patches.into_parts(); + let patch_indices = patch_indices.into_primitive()?; + let patch_values = patch_values.into_primitive()?; let patched_validity = self.validity() - .patch(self.len(), indices.as_ref(), values.validity())?; + .patch(self.len(), patch_indices.as_ref(), patch_values.validity())?; - match_each_integer_ptype!(indices.ptype(), |$I| { + match_each_integer_ptype!(patch_indices.ptype(), |$I| { match_each_native_ptype!(self.ptype(), |$T| { - let mut own_values = self.into_maybe_null_slice::<$T>(); - for (idx, value) in indices.into_maybe_null_slice::<$I>().into_iter().zip_eq(values.into_maybe_null_slice::<$T>().into_iter()) { - own_values[idx as usize] = value; - } - Ok(Self::from_vec(own_values, patched_validity)) + self.patch_typed::<$T, $I>(patch_indices, patch_values, patched_validity) }) }) } + + fn patch_typed( + self, + patch_indices: PrimitiveArray, + patch_values: PrimitiveArray, + patched_validity: Validity, + ) -> VortexResult + where + T: NativePType + ArrowNativeType, + I: NativePType + ArrowNativeType, + { + let mut own_values = self.into_maybe_null_slice::(); + + let patch_indices = patch_indices.into_maybe_null_slice::(); + let patch_values = patch_values.into_maybe_null_slice::(); + for (idx, value) in itertools::zip_eq(patch_indices, patch_values) { + own_values[idx.as_usize()] = value; + } + Ok(Self::from_vec(own_values, patched_validity)) + } } #[cfg(test)] From d6edda8932dfbe8eeb4aa08e70f35ae2f2ae1d98 Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Thu, 19 Dec 2024 10:38:46 -0500 Subject: [PATCH 5/8] use as_nonnullable for comparing patches and array dtypes --- .../src/bitpacking/compute/filter.rs | 19 ++++++------------- .../fastlanes/src/bitpacking/compute/slice.rs | 1 - encodings/fastlanes/src/bitpacking/mod.rs | 5 +++-- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/encodings/fastlanes/src/bitpacking/compute/filter.rs b/encodings/fastlanes/src/bitpacking/compute/filter.rs index 91dfbee79..88a72562c 100644 --- a/encodings/fastlanes/src/bitpacking/compute/filter.rs +++ b/encodings/fastlanes/src/bitpacking/compute/filter.rs @@ -131,7 +131,7 @@ mod test { use itertools::Itertools; use vortex_array::array::PrimitiveArray; use vortex_array::compute::{filter, slice, FilterMask}; - use vortex_array::{ArrayDType, ArrayLen, IntoArrayVariant}; + use vortex_array::{ArrayLen, IntoArrayVariant}; use crate::BitPackedArray; @@ -178,9 +178,6 @@ mod test { #[test] fn filter_bitpacked_signed() { - // Elements 0..=499 are negative integers (patches) - // Element 500 = 0 (packed) - // Elements 501..999 are positive integers (packed) let values: Vec = (0..500).collect_vec(); let unpacked = PrimitiveArray::from(values.clone()); let bitpacked = BitPackedArray::encode(unpacked.as_ref(), 9).unwrap(); @@ -188,15 +185,11 @@ mod test { bitpacked.as_ref(), FilterMask::from_indices(values.len(), 0..500), ) - .unwrap(); + .unwrap() + .into_primitive() + .unwrap() + .into_maybe_null_slice::(); - assert_eq!(filtered.dtype(), bitpacked.dtype()); - - let filtered_values = filtered - .into_primitive() - .unwrap() - .into_maybe_null_slice::(); - - assert_eq!(filtered_values, values); + assert_eq!(filtered, values); } } diff --git a/encodings/fastlanes/src/bitpacking/compute/slice.rs b/encodings/fastlanes/src/bitpacking/compute/slice.rs index 80fdefb02..26f8ef89e 100644 --- a/encodings/fastlanes/src/bitpacking/compute/slice.rs +++ b/encodings/fastlanes/src/bitpacking/compute/slice.rs @@ -8,7 +8,6 @@ use vortex_error::VortexResult; use crate::{BitPackedArray, BitPackedEncoding}; impl SliceFn for BitPackedEncoding { - #[allow(clippy::panic_in_result_fn)] fn slice(&self, array: &BitPackedArray, start: usize, stop: usize) -> VortexResult { let offset_start = start + array.offset() as usize; let offset_stop = stop + array.offset() as usize; diff --git a/encodings/fastlanes/src/bitpacking/mod.rs b/encodings/fastlanes/src/bitpacking/mod.rs index a79af7030..26fb8ad0f 100644 --- a/encodings/fastlanes/src/bitpacking/mod.rs +++ b/encodings/fastlanes/src/bitpacking/mod.rs @@ -1,9 +1,9 @@ use std::fmt::{Debug, Display}; use std::sync::Arc; -use ::serde::{Deserialize, Serialize}; pub use compress::*; use fastlanes::BitPacking; +use ::serde::{Deserialize, Serialize}; use vortex_array::array::PrimitiveArray; use vortex_array::encoding::ids; use vortex_array::patches::{Patches, PatchesMetadata}; @@ -101,10 +101,11 @@ impl BitPackedArray { } if let Some(ref patches) = patches { + // Ensure that array and patches have same PType if patches.dtype() != &DType::from(ptype) { vortex_bail!( "Patches DType {} does not match BitPackedArray dtype {}", - patches.dtype(), + patches.dtype().as_nonnullable(), ptype ) } From 05e84613156c21149fb1d362ab2d1e98433c0c4e Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Thu, 19 Dec 2024 10:47:37 -0500 Subject: [PATCH 6/8] try harder --- encodings/fastlanes/src/bitpacking/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/encodings/fastlanes/src/bitpacking/mod.rs b/encodings/fastlanes/src/bitpacking/mod.rs index 26fb8ad0f..3c987d80f 100644 --- a/encodings/fastlanes/src/bitpacking/mod.rs +++ b/encodings/fastlanes/src/bitpacking/mod.rs @@ -102,7 +102,7 @@ impl BitPackedArray { if let Some(ref patches) = patches { // Ensure that array and patches have same PType - if patches.dtype() != &DType::from(ptype) { + if patches.dtype().as_nonnullable() != DType::from(ptype).as_nonnullable() { vortex_bail!( "Patches DType {} does not match BitPackedArray dtype {}", patches.dtype().as_nonnullable(), From 92d9baaead6b891d3a45a4d59d3e7f47881bdbcf Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Thu, 19 Dec 2024 10:47:48 -0500 Subject: [PATCH 7/8] lol --- encodings/fastlanes/src/bitpacking/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/encodings/fastlanes/src/bitpacking/mod.rs b/encodings/fastlanes/src/bitpacking/mod.rs index 3c987d80f..8be9a1d2f 100644 --- a/encodings/fastlanes/src/bitpacking/mod.rs +++ b/encodings/fastlanes/src/bitpacking/mod.rs @@ -1,9 +1,9 @@ use std::fmt::{Debug, Display}; use std::sync::Arc; +use ::serde::{Deserialize, Serialize}; pub use compress::*; use fastlanes::BitPacking; -use ::serde::{Deserialize, Serialize}; use vortex_array::array::PrimitiveArray; use vortex_array::encoding::ids; use vortex_array::patches::{Patches, PatchesMetadata}; From 2dda7b92e18aea13b0b4ed6027bfc6ed07e7458a Mon Sep 17 00:00:00 2001 From: Andrew Duffy Date: Thu, 19 Dec 2024 11:01:46 -0500 Subject: [PATCH 8/8] eq_ignore_nullability --- encodings/fastlanes/src/bitpacking/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/encodings/fastlanes/src/bitpacking/mod.rs b/encodings/fastlanes/src/bitpacking/mod.rs index 8be9a1d2f..1250ad223 100644 --- a/encodings/fastlanes/src/bitpacking/mod.rs +++ b/encodings/fastlanes/src/bitpacking/mod.rs @@ -102,7 +102,7 @@ impl BitPackedArray { if let Some(ref patches) = patches { // Ensure that array and patches have same PType - if patches.dtype().as_nonnullable() != DType::from(ptype).as_nonnullable() { + if !patches.dtype().eq_ignore_nullability(ptype.into()) { vortex_bail!( "Patches DType {} does not match BitPackedArray dtype {}", patches.dtype().as_nonnullable(),