From 6c74406743e3a821439271934fc62666b3432f98 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 4 Nov 2026 14:47:14 -0500 Subject: [PATCH 1/4] Choose DecimalByteParts lower parts from valid values Signed-off-by: Connor Tsui --- .../decimal-byte-parts/benches/common/mod.rs | 11 + .../benches/dbp_assemble.rs | 4 +- .../decimal-byte-parts/benches/dbp_split.rs | 5 +- .../src/decimal_byte_parts/assemble/tests.rs | 13 +- .../src/decimal_byte_parts/mod.rs | 49 +-- .../src/decimal_byte_parts/plugin/tests.rs | 19 ++ .../src/decimal_byte_parts/prop_tests.rs | 5 +- .../{split.rs => split/mod.rs} | 302 +++++++++-------- .../src/decimal_byte_parts/split/tests.rs | 307 ++++++++++++++++++ .../src/decimal_byte_parts/testing.rs | 38 ++- .../encodings/decimal_byte_parts_v2.rs | 21 +- 11 files changed, 591 insertions(+), 183 deletions(-) rename encodings/decimal-byte-parts/src/decimal_byte_parts/{split.rs => split/mod.rs} (50%) create mode 100644 encodings/decimal-byte-parts/src/decimal_byte_parts/split/tests.rs diff --git a/encodings/decimal-byte-parts/benches/common/mod.rs b/encodings/decimal-byte-parts/benches/common/mod.rs index 197c518b722..bbd12f94e22 100644 --- a/encodings/decimal-byte-parts/benches/common/mod.rs +++ b/encodings/decimal-byte-parts/benches/common/mod.rs @@ -88,4 +88,15 @@ pub(super) mod arrays { _ => vortex_panic!("unsupported benchmark storage type: {values_type}"), } } + + /// The number of lower parts that holds every value of `values_type`, so the split and + /// assembly benchmarks keep the shape of each storage width. + pub(crate) fn full_width_lower_part_count(values_type: DecimalType) -> usize { + match values_type { + DecimalType::I64 => 0, + DecimalType::I128 => 1, + DecimalType::I256 => 3, + _ => vortex_panic!("unsupported benchmark storage type: {values_type}"), + } + } } diff --git a/encodings/decimal-byte-parts/benches/dbp_assemble.rs b/encodings/decimal-byte-parts/benches/dbp_assemble.rs index 9a263826e0e..ddbda30621e 100644 --- a/encodings/decimal-byte-parts/benches/dbp_assemble.rs +++ b/encodings/decimal-byte-parts/benches/dbp_assemble.rs @@ -126,6 +126,7 @@ mod arrays { use crate::common::arrays::cases; use crate::common::arrays::decimal_array; + use crate::common::arrays::full_width_lower_part_count; pub(super) static SESSION: LazyLock = LazyLock::new(|| VortexSession::empty().with::()); @@ -134,7 +135,8 @@ mod arrays { fn dbp_assemble(bencher: Bencher, (values_type, len): (DecimalType, usize)) { let decimal = decimal_array(values_type, len, Validity::NonNullable); let mut ctx = SESSION.create_execution_ctx(); - let parts = split_decimal(&decimal, &mut ctx).vortex_expect("split benchmark input"); + let parts = split_decimal(&decimal, full_width_lower_part_count(values_type), &mut ctx) + .vortex_expect("split benchmark input"); bench_parts( bencher, parts.msp, diff --git a/encodings/decimal-byte-parts/benches/dbp_split.rs b/encodings/decimal-byte-parts/benches/dbp_split.rs index d38371bbd8a..57b110d6c1e 100644 --- a/encodings/decimal-byte-parts/benches/dbp_split.rs +++ b/encodings/decimal-byte-parts/benches/dbp_split.rs @@ -94,6 +94,7 @@ mod arrays { use crate::common::arrays::cases; use crate::common::arrays::decimal_array; + use crate::common::arrays::full_width_lower_part_count; pub(super) static SESSION: LazyLock = LazyLock::new(|| VortexSession::empty().with::()); @@ -118,11 +119,13 @@ mod arrays { fn bench_split(bencher: Bencher, values_type: DecimalType, len: usize, validity: Validity) { let decimal = decimal_array(values_type, len, validity); mark_optimized(decimal.as_array(), &SESSION).vortex_expect("mark the decimal as optimized"); + let lower_part_count = full_width_lower_part_count(values_type); bencher .with_inputs(|| SESSION.create_execution_ctx()) .bench_refs(|ctx| { - split_decimal(black_box(&decimal), ctx).vortex_expect("split decimal array") + split_decimal(black_box(&decimal), lower_part_count, ctx) + .vortex_expect("split decimal array") }); } } diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/assemble/tests.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/assemble/tests.rs index 9597cb6559f..43a0ebb894a 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/assemble/tests.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/assemble/tests.rs @@ -41,6 +41,7 @@ use vortex_session::VortexSession; use super::assemble_decimal; use crate::decimal_byte_parts::split_decimal; +use crate::decimal_byte_parts::testing::full_width_lower_part_count; static SESSION: LazyLock = LazyLock::new(VortexSession::empty); @@ -129,7 +130,8 @@ fn assert_form(decoded: &ArrayRef, decimal_dtype: DecimalDType, child_precision: fn round_trip(decimal: DecimalArray) -> VortexResult { let mut ctx = SESSION.create_execution_ctx(); - let parts = split_decimal(&decimal, &mut ctx)?; + let lower_part_count = full_width_lower_part_count(decimal.values_type()); + let parts = split_decimal(&decimal, lower_part_count, &mut ctx)?; assemble_decimal( &parts.msp, &parts.lower_parts, @@ -166,7 +168,7 @@ fn test_split_without_valid_rows( validity, ) }); - let parts = split_decimal(&decimal, &mut ctx)?; + let parts = split_decimal(&decimal, full_width_lower_part_count(values_type), &mut ctx)?; assert!(parts.msp.is::()); assert!(parts.lower_parts.iter().all(|part| part.is::())); assert_eq!(parts.msp.len(), len); @@ -213,8 +215,9 @@ fn test_split_zeroes_null_words( .collect::>(), Validity::NonNullable, ); - let parts = split_decimal(&decimal, &mut ctx)?; - assert_eq!(parts.lower_parts.len(), if wide_256 { 3 } else { 1 }); + let lower_part_count = if wide_256 { 3 } else { 1 }; + let parts = split_decimal(&decimal, lower_part_count, &mut ctx)?; + assert_eq!(parts.lower_parts.len(), lower_part_count); assert_eq!( parts.msp.dtype(), &DType::Primitive(PType::I64, decimal.dtype().nullability()) @@ -265,7 +268,7 @@ fn test_split_narrow_decimal_reuses_values( let decimal = DecimalArray::new(buffer![1i32, 2, 3], DecimalDType::new(9, 0), validity); let values_ptr = decimal.buffer::().as_ptr(); - let parts = split_decimal(&decimal, &mut ctx)?; + let parts = split_decimal(&decimal, 0, &mut ctx)?; assert!(parts.lower_parts.is_empty()); assert_eq!(parts.msp.dtype().as_ptype(), PType::I32); let msp = parts.msp.clone().execute::(&mut ctx)?; diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs index 5df1b622c53..47b02230990 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/mod.rs @@ -21,23 +21,38 @@ use vortex_array::dtype::DType; use vortex_array::dtype::Nullability; use vortex_array::dtype::PType; +/// The maximum number of 64-bit lower parts an encoded `i128` decimal can carry. +const MAX_I128_LOWER_PARTS: usize = 1; + +/// The maximum number of 64-bit lower parts an encoded `i256` decimal can carry. +const MAX_I256_LOWER_PARTS: usize = 3; + +/// The maximum number of 64-bit lower parts an encoded decimal can carry. +const MAX_LOWER_PARTS: usize = MAX_I256_LOWER_PARTS; + +/// Number of bits stored in each lower part. +const LOWER_PART_BITS: usize = 64; + +/// The dtype of every lower part. +const LOWER_PART_DTYPE: DType = DType::Primitive(PType::U64, Nullability::NonNullable); + mod array; +pub use array::*; + mod assemble; pub(crate) mod compute; -mod plugin; -#[cfg(test)] -mod prop_tests; -mod rules; -mod split; -#[cfg(test)] -mod testing; -pub use array::*; +mod plugin; pub use plugin::DecimalBytePartsPlugin; pub use plugin::DecimalBytePartsV2Metadata; pub use plugin::decimal_byte_parts_v1_id; pub use plugin::decimal_byte_parts_v2_id; + +mod rules; + +mod split; pub use split::DecimalParts; +pub use split::lower_part_count; pub use split::split_decimal; #[doc(hidden)] @@ -49,17 +64,7 @@ pub mod _benchmarking { pub use super::split::split_wide; } -/// The maximum number of 64-bit lower parts an encoded `i128` decimal can carry. -const MAX_I128_LOWER_PARTS: usize = 1; - -/// The maximum number of 64-bit lower parts an encoded `i256` decimal can carry. -const MAX_I256_LOWER_PARTS: usize = 3; - -/// The maximum number of 64-bit lower parts an encoded decimal can carry. -const MAX_LOWER_PARTS: usize = MAX_I256_LOWER_PARTS; - -/// Number of bits stored in each lower part. -const LOWER_PART_BITS: usize = 64; - -/// The dtype of every lower part. -const LOWER_PART_DTYPE: DType = DType::Primitive(PType::U64, Nullability::NonNullable); +#[cfg(test)] +mod prop_tests; +#[cfg(test)] +mod testing; diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/plugin/tests.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/plugin/tests.rs index 69ac1bf6052..08954312adb 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/plugin/tests.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/plugin/tests.rs @@ -18,6 +18,7 @@ use vortex_array::dtype::DType; use vortex_array::dtype::DecimalDType; use vortex_array::dtype::Nullability; use vortex_array::dtype::PType; +use vortex_array::dtype::i256; use vortex_array::serde::SerializeOptions; use vortex_array::serde::SerializedArray; use vortex_array::session::ArraySessionExt; @@ -46,6 +47,24 @@ static SESSION: LazyLock = LazyLock::new(VortexSession::empty); &DecimalArray::new(buffer![-99i64, 0, 99], DecimalDType::new(2, 0), Validity::NonNullable), &mut SESSION.create_execution_ctx(), ))] +#[case::wider_i128_storage(DecimalByteParts::encode( + &DecimalArray::new(buffer![-99i128, 0, 99], DecimalDType::new(2, 0), Validity::NonNullable), + &mut SESSION.create_execution_ctx(), +))] +#[case::wider_i256_storage(DecimalByteParts::encode( + &DecimalArray::new( + buffer![i256::from_i128(-99), i256::ZERO, i256::from_i128(99)], + DecimalDType::new(2, 0), Validity::NonNullable, + ), + &mut SESSION.create_execution_ctx(), +))] +#[case::wider_i256_storage_all_null(DecimalByteParts::encode( + &DecimalArray::new( + buffer![i256::from_parts(1 << 127, 0); 3], + DecimalDType::new(2, 0), Validity::AllInvalid, + ), + &mut SESSION.create_execution_ctx(), +))] #[case::redundant_lower_parts(DecimalByteParts::try_new_with_lower_parts( buffer![0i64; 3].into_array(), vec![buffer![0u64; 3].into_array(), buffer![0u64; 3].into_array(), lower_part()], diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/prop_tests.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/prop_tests.rs index 1e742d7bda0..371a7800387 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/prop_tests.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/prop_tests.rs @@ -149,8 +149,9 @@ fn decoded_survives_encode_then_decode(tc: TestCase) { /// /// Starting from the encoded side reaches part counts `split_decimal` never produces, so this /// covers layouts the property above cannot generate. It compares decoded values rather than -/// the arrays themselves because re-encoding normalizes the part count: splitting an `i256` -/// always yields three lower parts, whatever the original array carried. +/// the arrays themselves because re-encoding normalizes the part count: +/// [`DecimalByteParts::encode`] uses the fewest lower parts that hold the valid values, whatever +/// the original array carried. #[hegel::test] fn encoded_survives_decode_then_encode(tc: TestCase) { let (array, _len) = draw_encoded(&tc); diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/split.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/split/mod.rs similarity index 50% rename from encodings/decimal-byte-parts/src/decimal_byte_parts/split.rs rename to encodings/decimal-byte-parts/src/decimal_byte_parts/split/mod.rs index d8044e55323..71362e26e97 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/split.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/split/mod.rs @@ -9,16 +9,22 @@ use vortex_array::IntoArray; use vortex_array::arrays::ConstantArray; use vortex_array::arrays::DecimalArray; use vortex_array::arrays::PrimitiveArray; +use vortex_array::arrays::decimal::converted_buffer; +use vortex_array::arrays::decimal::valid_range; +use vortex_array::dtype::BigCast; use vortex_array::dtype::DType; use vortex_array::dtype::DecimalType; +use vortex_array::dtype::NativeDecimalType; use vortex_array::dtype::NativePType; use vortex_array::dtype::PType; use vortex_array::dtype::i256; +use vortex_array::match_each_decimal_value_type; use vortex_array::scalar::Scalar; use vortex_array::validity::Validity; use vortex_buffer::Buffer; use vortex_buffer::BufferMut; use vortex_error::VortexResult; +use vortex_error::vortex_bail; use vortex_mask::Mask; use super::DecimalByteParts; @@ -28,18 +34,19 @@ use super::MAX_I128_LOWER_PARTS; use super::MAX_I256_LOWER_PARTS; impl DecimalByteParts { - /// Encode a [`DecimalArray`] as byte parts, splitting wide values into lower parts. + /// Encode a [`DecimalArray`] as byte parts, with as many lower parts as its valid values need + /// (see [`lower_part_count`]). /// /// # Errors /// - /// Returns an error if the decimal cannot be split, or if its storage is wider than the - /// canonical width of its dtype and splits into more lower parts than the dtype allows (see - /// [`super::DecimalBytePartsData::validate`]). + /// Returns an error if the validity cannot be executed, or if a valid value needs more lower + /// parts than the canonical width of the dtype allows (see + /// [`super::DecimalBytePartsData::validate`]). Only a value outside the precision needs them. pub fn encode( decimal: &DecimalArray, exec_ctx: &mut ExecutionCtx, ) -> VortexResult { - let parts = split_decimal(decimal, exec_ctx)?; + let parts = split_decimal(decimal, lower_part_count(decimal, exec_ctx)?, exec_ctx)?; Self::try_new_with_lower_parts(parts.msp, parts.lower_parts, decimal.decimal_dtype()) } } @@ -53,8 +60,8 @@ pub struct DecimalParts { } impl DecimalParts { - /// Construct decimal parts from the MSP buffer constituting a narrow decimal (`i64` or narrower). - /// Narrow decimals have an MSP at most as wide as `i64` and no lower parts. + /// Construct decimal parts with no lower parts from an MSP buffer of `i64` or narrower values: + /// either the decimal's own narrow storage, or a copy of wide values that all fit `i64`. fn from_narrow(values: Buffer, validity: Validity) -> Self { Self { msp: PrimitiveArray::new(values, validity).into_array(), @@ -79,12 +86,67 @@ impl DecimalParts { } } -/// Split a canonical decimal array into a signed most significant part (MSP) and unsigned 64-bit -/// lower parts. The MSP is at most 64 bits. +/// The number of lower parts that the valid values of `decimal` need: 0 when every valid value +/// fits `i64`, 1 when every valid value fits `i128`, and 3 otherwise. /// -/// Values narrower than 128 bits are already a single signed part, so they are returned -/// with no lower parts. `i128` values split into an `i64` MSP and one lower part. `i256` -/// values split into an `i64` MSP and three lower parts. +/// Null slots do not count, so they can hold any value. An array without valid values needs no +/// lower parts. A storage width up to `i64` needs no lower parts either, so it needs no scan. +/// +/// The count depends only on the stored values, not on the precision. A value outside the +/// precision can need more lower parts than the dtype allows, and the constructor then rejects +/// the parts. The scan uses stored integer values directly so selecting the parts does not +/// require decimal scalars that fit the declared precision. +/// +/// # Errors +/// +/// Returns an error if the validity cannot be executed. +pub fn lower_part_count(decimal: &DecimalArray, ctx: &mut ExecutionCtx) -> VortexResult { + let values_type = decimal.values_type(); + if values_type <= DecimalType::I64 { + return Ok(0); + } + + let mask = decimal.validity()?.execute_mask(decimal.len(), ctx)?; + + match_each_decimal_value_type!(values_type, |T| { + Ok(valid_values_lower_part_count(&decimal.buffer::(), &mask)) + }) +} + +/// The number of lower parts that the valid values in `values` need, as [`lower_part_count`] +/// describes. +fn valid_values_lower_part_count(values: &[T], validity: &Mask) -> usize { + let Some((min, max)) = valid_range(values, validity) else { + return 0; + }; + + if range_fits::(min, max) { + 0 + } else if range_fits::(min, max) { + MAX_I128_LOWER_PARTS + } else { + MAX_I256_LOWER_PARTS + } +} + +/// Whether `W` holds every value from `min` to `max`. +fn range_fits(min: T, max: T) -> bool { + ::from(min).is_some() && ::from(max).is_some() +} + +/// Split a canonical decimal array into a signed most significant part (MSP) and +/// `lower_part_count` unsigned 64-bit lower parts. The MSP is at most 64 bits. +/// +/// These shapes are supported: +/// +/// - A storage width up to `i64` with no lower parts. The MSP reuses the values buffer. +/// - `i128` or `i256` values with no lower parts. The values are copied into an `i64` MSP. +/// - `i128` values with one lower part, or `i256` values with three lower parts. The values split +/// on 64-bit boundaries into an `i64` MSP and the lower parts. +/// - `i256` values with one lower part. The values are copied into `i128`, and then split as +/// `i128` values. +/// +/// [`lower_part_count`] gives the smallest count that holds the valid values. /// /// The MSP retains the decimal's validity while lower parts are non-nullable. Lower parts /// are constructed with zeroes at null positions instead of invalid bytes. @@ -92,50 +154,124 @@ impl DecimalParts { /// /// # Errors /// -/// Returns an error if the array's validity cannot be derived or executed. -pub fn split_decimal(decimal: &DecimalArray, ctx: &mut ExecutionCtx) -> VortexResult { +/// Returns an error if the array's validity cannot be derived or executed, if the storage width +/// and `lower_part_count` are not one of the shapes above, or if a valid value does not fit +/// `lower_part_count` lower parts. +pub fn split_decimal( + decimal: &DecimalArray, + lower_part_count: usize, + ctx: &mut ExecutionCtx, +) -> VortexResult { + let shape = SplitShape::classify(decimal.values_type(), lower_part_count)?; + let validity = decimal.validity()?; let len = decimal.len(); let mask = validity.execute_mask(len, ctx)?; if mask.all_false() || decimal.is_empty() { - return Ok(split_no_valid_row(decimal, &validity)); + return Ok(split_no_valid_row(len, shape, &validity)); } - Ok(match decimal.values_type() { - DecimalType::I8 => DecimalParts::from_narrow(decimal.buffer::(), validity), - DecimalType::I16 => DecimalParts::from_narrow(decimal.buffer::(), validity), - DecimalType::I32 => DecimalParts::from_narrow(decimal.buffer::(), validity), - DecimalType::I64 => DecimalParts::from_narrow(decimal.buffer::(), validity), - DecimalType::I128 => { + Ok(match shape { + SplitShape::ReuseI8 => DecimalParts::from_narrow(decimal.buffer::(), validity), + SplitShape::ReuseI16 => DecimalParts::from_narrow(decimal.buffer::(), validity), + SplitShape::ReuseI32 => DecimalParts::from_narrow(decimal.buffer::(), validity), + SplitShape::ReuseI64 => DecimalParts::from_narrow(decimal.buffer::(), validity), + SplitShape::IntoI64 => { + DecimalParts::from_narrow(converted_buffer::(decimal, &mask)?, validity) + } + SplitShape::I128OneLowerPart => { let (msp, lower) = split_wide(&decimal.buffer::(), &mask, i128_to_parts); DecimalParts::from_wide(msp, lower, validity) } - DecimalType::I256 => { + SplitShape::I256OneLowerPart => { + let values = converted_buffer::(decimal, &mask)?; + let (msp, lower) = split_wide(&values, &mask, i128_to_parts); + DecimalParts::from_wide(msp, lower, validity) + } + SplitShape::I256ThreeLowerParts => { let (msp, lower) = split_wide(&decimal.buffer::(), &mask, i256_to_parts); DecimalParts::from_wide(msp, lower, validity) } }) } +/// One of the shapes that [`split_decimal`] lists, chosen from a storage width and a lower-part +/// count. +#[derive(Clone, Copy)] +enum SplitShape { + ReuseI8, + ReuseI16, + ReuseI32, + ReuseI64, + /// `i128` or `i256` values. + IntoI64, + I128OneLowerPart, + /// Copied into `i128` first. + I256OneLowerPart, + I256ThreeLowerParts, +} + +impl SplitShape { + /// The shape of splitting `values_type` values into `lower_part_count` lower parts, or an + /// error if [`split_decimal`] does not support it. + fn classify(values_type: DecimalType, lower_part_count: usize) -> VortexResult { + Ok(match (values_type, lower_part_count) { + (DecimalType::I8, 0) => Self::ReuseI8, + (DecimalType::I16, 0) => Self::ReuseI16, + (DecimalType::I32, 0) => Self::ReuseI32, + (DecimalType::I64, 0) => Self::ReuseI64, + (DecimalType::I128 | DecimalType::I256, 0) => Self::IntoI64, + (DecimalType::I128, MAX_I128_LOWER_PARTS) => Self::I128OneLowerPart, + (DecimalType::I256, MAX_I128_LOWER_PARTS) => Self::I256OneLowerPart, + (DecimalType::I256, MAX_I256_LOWER_PARTS) => Self::I256ThreeLowerParts, + _ => { + let supported = match values_type { + DecimalType::I128 => "0 or 1", + DecimalType::I256 => "0, 1 or 3", + _ => "0", + }; + vortex_bail!( + "{values_type} values split into {supported} lower parts, \ + got {lower_part_count}" + ) + } + }) + } + + /// The type of the MSP. + fn msp_ptype(self) -> PType { + match self { + Self::ReuseI8 => PType::I8, + Self::ReuseI16 => PType::I16, + Self::ReuseI32 => PType::I32, + Self::ReuseI64 + | Self::IntoI64 + | Self::I128OneLowerPart + | Self::I256OneLowerPart + | Self::I256ThreeLowerParts => PType::I64, + } + } + + /// The number of `u64` lower parts. + fn lower_part_count(self) -> usize { + match self { + Self::ReuseI8 | Self::ReuseI16 | Self::ReuseI32 | Self::ReuseI64 | Self::IntoI64 => 0, + Self::I128OneLowerPart | Self::I256OneLowerPart => MAX_I128_LOWER_PARTS, + Self::I256ThreeLowerParts => MAX_I256_LOWER_PARTS, + } + } +} + /// Splits decimals with no valid rows (all null or empty) into constant decimal parts with the /// corresponding nullability. -fn split_no_valid_row(decimal: &DecimalArray, validity: &Validity) -> DecimalParts { - let (msp_ptype, lower_part_count) = match decimal.values_type() { - DecimalType::I8 => (PType::I8, 0), - DecimalType::I16 => (PType::I16, 0), - DecimalType::I32 => (PType::I32, 0), - DecimalType::I64 => (PType::I64, 0), - DecimalType::I128 => (PType::I64, MAX_I128_LOWER_PARTS), - DecimalType::I256 => (PType::I64, MAX_I256_LOWER_PARTS), - }; +fn split_no_valid_row(len: usize, shape: SplitShape, validity: &Validity) -> DecimalParts { // Empty masks are also all-false. The default scalar is null for nullable inputs // and zero for non-nullable empty inputs, preserving the MSP's nullability. - let msp = Scalar::default_value(&DType::Primitive(msp_ptype, validity.nullability())); - let len = decimal.len(); + let msp = Scalar::default_value(&DType::Primitive(shape.msp_ptype(), validity.nullability())); DecimalParts { msp: ConstantArray::new(msp, len).into_array(), - lower_parts: vec![ConstantArray::new(0u64, len).into_array(); lower_part_count], + lower_parts: vec![ConstantArray::new(0u64, len).into_array(); shape.lower_part_count()], } } @@ -205,6 +341,7 @@ pub fn split_wide( part.set_len(len); } } + (msp.freeze(), lower.map(BufferMut::freeze)) } @@ -235,99 +372,4 @@ pub const fn i256_to_parts(value: i256) -> (i64, [u64; MAX_I256_LOWER_PARTS]) { } #[cfg(test)] -mod tests { - use std::sync::LazyLock; - - use rstest::rstest; - use vortex_array::VortexSessionExecute; - use vortex_array::arrays::DecimalArray; - use vortex_array::arrays::PrimitiveArray; - use vortex_array::assert_arrays_eq; - use vortex_array::dtype::DecimalDType; - use vortex_array::dtype::PType; - use vortex_array::dtype::i256; - use vortex_array::validity::Validity; - use vortex_buffer::Buffer; - use vortex_buffer::buffer; - use vortex_error::VortexResult; - use vortex_session::VortexSession; - - use super::DecimalByteParts; - use super::split_decimal; - use crate::decimal_byte_parts::LOWER_PART_DTYPE; - use crate::decimal_byte_parts::MAX_LOWER_PARTS; - - static SESSION: LazyLock = LazyLock::new(VortexSession::empty); - - #[test] - fn test_split_i256_part_count_and_types() -> VortexResult<()> { - let mut ctx = SESSION.create_execution_ctx(); - let decimal = DecimalArray::new( - Buffer::from(vec![i256::from_i128(i128::MAX), i256::MIN]), - DecimalDType::new(76, 0), - Validity::NonNullable, - ); - let parts = split_decimal(&decimal, &mut ctx)?; - assert_eq!(parts.lower_parts.len(), MAX_LOWER_PARTS); - assert_eq!(parts.msp.dtype().as_ptype(), PType::I64); - for part in &parts.lower_parts { - assert_eq!(part.dtype(), &LOWER_PART_DTYPE); - } - Ok(()) - } - - #[rstest] - fn test_split_i256_part_order( - #[values(Validity::NonNullable, Validity::from_iter([true, false, true]))] - validity: Validity, - ) -> VortexResult<()> { - let mut ctx = SESSION.create_execution_ctx(); - let decimal = DecimalArray::new( - buffer![ - i256::from_parts((2u128 << 64) | 3, (1i128 << 64) | 4), - i256::ZERO, - i256::from_parts((6u128 << 64) | 7, (-2i128 << 64) | 5), - ], - DecimalDType::new(76, 0), - validity.clone(), - ); - let parts = split_decimal(&decimal, &mut ctx)?; - assert_arrays_eq!( - parts.msp, - PrimitiveArray::new(buffer![1i64, 0, -2], validity), - &mut ctx - ); - assert_eq!(parts.lower_parts.len(), 3); - for (part, expected) in parts.lower_parts.into_iter().zip([ - buffer![4u64, 0, 5], - buffer![2u64, 0, 6], - buffer![3u64, 0, 7], - ]) { - assert_arrays_eq!( - part, - PrimitiveArray::new(expected, Validity::NonNullable), - &mut ctx - ); - } - Ok(()) - } - - /// Storage wider than the canonical width splits into more lower parts than the dtype allows. - #[rstest] - #[case::wider_i128_storage(DecimalArray::new( - buffer![-99i128, 0, 99], DecimalDType::new(2, 0), Validity::NonNullable, - ))] - #[case::wider_i256_storage(DecimalArray::new( - buffer![i256::from_i128(-99), i256::ZERO, i256::from_i128(99)], - DecimalDType::new(2, 0), Validity::NonNullable, - ))] - fn test_encode_rejects_lower_parts_past_canonical_width(#[case] decimal: DecimalArray) { - let result = DecimalByteParts::encode(&decimal, &mut SESSION.create_execution_ctx()); - assert!( - result - .as_ref() - .is_err_and(|err| err.to_string().contains("allows at most 0 lower parts")), - "expected the lower-part bound to reject the parts, got {result:?}" - ); - } -} +mod tests; diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/split/tests.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/split/tests.rs new file mode 100644 index 00000000000..cb3da13cab4 --- /dev/null +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/split/tests.rs @@ -0,0 +1,307 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +//! Lower-part selection and checked splitting of valid decimal values. + +use std::sync::LazyLock; + +use rstest::rstest; +use vortex_array::VortexSessionExecute; +use vortex_array::arrays::Constant; +use vortex_array::arrays::DecimalArray; +use vortex_array::arrays::PrimitiveArray; +use vortex_array::assert_arrays_eq; +use vortex_array::dtype::DecimalDType; +use vortex_array::dtype::NativeDecimalType; +use vortex_array::dtype::PType; +use vortex_array::dtype::i256; +use vortex_array::validity::Validity; +use vortex_buffer::Buffer; +use vortex_buffer::buffer; +use vortex_error::VortexResult; +use vortex_session::VortexSession; + +use super::lower_part_count; +use super::split_decimal; +use crate::decimal_byte_parts::DecimalByteParts; +use crate::decimal_byte_parts::MAX_LOWER_PARTS; + +static SESSION: LazyLock = LazyLock::new(VortexSession::empty); + +/// 2^127, the smallest positive value that does not fit `i128`. +const I256_ABOVE_I128: i256 = i256::from_parts(1 << 127, 0); + +/// A decimal of `values` at the largest precision of their storage width. +fn decimal(values: Buffer, validity: Validity) -> DecimalArray { + DecimalArray::new(values, DecimalDType::new(T::MAX_PRECISION, 0), validity) +} + +/// Runs splitting through validation and checks the reported requirement. +#[track_caller] +fn assert_split_error(decimal: &DecimalArray, lower_part_count: usize, expected: &str) { + let error = split_decimal( + decimal, + lower_part_count, + &mut SESSION.create_execution_ctx(), + ) + .expect_err("the fixture violates a split requirement"); + assert!(error.to_string().contains(expected), "{error}"); +} + +#[rstest] +#[case::i8(decimal(buffer![i8::MIN, i8::MAX], Validity::NonNullable), 0)] +#[case::i16(decimal(buffer![i16::MIN, i16::MAX], Validity::NonNullable), 0)] +#[case::i32(decimal(buffer![i32::MIN, i32::MAX], Validity::NonNullable), 0)] +#[case::i64(decimal(buffer![i64::MIN, i64::MAX], Validity::NonNullable), 0)] +#[case::i128_fits_i64( + decimal(buffer![i128::from(i64::MIN), i128::from(i64::MAX)], Validity::NonNullable), + 0 +)] +#[case::i128_below_i64_min( + decimal(buffer![i128::from(i64::MIN) - 1, 0], Validity::NonNullable), + 1 +)] +#[case::i128_above_i64_max( + decimal(buffer![0, i128::from(i64::MAX) + 1], Validity::NonNullable), + 1 +)] +#[case::i128_full_range(decimal(buffer![i128::MIN, i128::MAX], Validity::NonNullable), 1)] +#[case::i256_fits_i64( + decimal(buffer![i256::from_i128(i128::from(i64::MIN))], Validity::NonNullable), + 0 +)] +#[case::i256_below_i64_min( + decimal(buffer![i256::from_i128(i128::from(i64::MIN) - 1)], Validity::NonNullable), + 1 +)] +#[case::i256_minus_2_pow_127( + decimal(buffer![i256::from_i128(i128::MIN)], Validity::NonNullable), + 1 +)] +#[case::i256_below_minus_2_pow_127( + decimal(buffer![i256::from_i128(i128::MIN) - i256::ONE], Validity::NonNullable), + 3 +)] +#[case::i256_plus_2_pow_127(decimal(buffer![I256_ABOVE_I128], Validity::NonNullable), 3)] +#[case::null_slot_holds_2_pow_100( + decimal(buffer![1i128, 1 << 100], Validity::from_iter([true, false])), + 0 +)] +#[case::all_null(decimal(buffer![1i128 << 100], Validity::AllInvalid), 0)] +#[case::empty(decimal(Buffer::::empty(), Validity::NonNullable), 0)] +fn test_lower_part_count( + #[case] decimal: DecimalArray, + #[case] expected: usize, +) -> VortexResult<()> { + let count = lower_part_count(&decimal, &mut SESSION.create_execution_ctx())?; + assert_eq!(count, expected); + + Ok(()) +} + +/// The copy into the narrower type checks every valid value, so a count below the need of a +/// valid value fails. +#[rstest] +#[case::i128_into_msp(decimal(buffer![0i128, 1 << 70], Validity::NonNullable), 0)] +#[case::i256_into_msp(decimal(buffer![i256::ZERO, I256_ABOVE_I128], Validity::NonNullable), 0)] +#[case::i256_into_one_lower_part( + decimal(buffer![i256::ZERO, I256_ABOVE_I128], Validity::NonNullable), + 1 +)] +fn test_split_decimal_rejects_too_small_count( + #[case] decimal: DecimalArray, + #[case] lower_part_count: usize, +) -> VortexResult<()> { + assert_split_error(&decimal, lower_part_count, "does not fit"); + + Ok(()) +} + +/// Only 0 lower parts, or the count of `i128` or `i256` up to the storage width, are supported. +/// The count is checked before an array without valid values takes its shortcut. +#[rstest] +#[case::i8_one(decimal(buffer![0i8], Validity::NonNullable), 1)] +#[case::i64_one(decimal(buffer![0i64], Validity::NonNullable), 1)] +#[case::i128_two(decimal(buffer![0i128], Validity::NonNullable), 2)] +#[case::i128_three(decimal(buffer![0i128], Validity::NonNullable), 3)] +#[case::i256_two(decimal(buffer![i256::ZERO], Validity::NonNullable), 2)] +#[case::i256_four(decimal(buffer![i256::ZERO], Validity::NonNullable), 4)] +#[case::all_null(decimal(buffer![0i128], Validity::AllInvalid), 3)] +#[case::empty(decimal(Buffer::::empty(), Validity::NonNullable), 1)] +fn test_split_decimal_rejects_invalid_count( + #[case] decimal: DecimalArray, + #[case] lower_part_count: usize, +) -> VortexResult<()> { + assert_split_error( + &decimal, + lower_part_count, + &format!("lower parts, got {lower_part_count}"), + ); + + Ok(()) +} + +/// An array without valid values splits into constant parts with the requested count, not the +/// count of its storage width. +#[rstest] +#[case::i128_all_null_msp_only(decimal(buffer![1i128 << 100; 3], Validity::AllInvalid), 0)] +#[case::i128_empty_msp_only(decimal(Buffer::::empty(), Validity::NonNullable), 0)] +#[case::i256_all_null_msp_only(decimal(buffer![I256_ABOVE_I128; 3], Validity::AllInvalid), 0)] +#[case::i256_empty_msp_only(decimal(Buffer::::empty(), Validity::NonNullable), 0)] +#[case::i256_all_null_one_lower_part( + decimal(buffer![I256_ABOVE_I128; 3], Validity::AllInvalid), + 1 +)] +#[case::i256_empty_one_lower_part(decimal(Buffer::::empty(), Validity::NonNullable), 1)] +fn test_split_without_valid_rows_uses_requested_count( + #[case] decimal: DecimalArray, + #[case] lower_part_count: usize, +) -> VortexResult<()> { + let parts = split_decimal( + &decimal, + lower_part_count, + &mut SESSION.create_execution_ctx(), + )?; + assert!(parts.msp.is::()); + assert_eq!(parts.msp.dtype().as_ptype(), PType::I64); + assert_eq!( + parts.msp.dtype().nullability(), + decimal.dtype().nullability() + ); + assert_eq!(parts.lower_parts.len(), lower_part_count); + assert!(parts.lower_parts.iter().all(|part| part.is::())); + + Ok(()) +} + +/// `i256` values that fit `i128` split into an `i64` MSP and one lower part. The null slot +/// holds a value outside `i128`, which the narrowing copy ignores. +#[test] +fn test_split_i256_into_one_lower_part_round_trips() -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let decimal = DecimalArray::new( + buffer![ + i256::from_i128(i128::MIN), + I256_ABOVE_I128, + i256::from_i128(-1), + i256::from_i128((1 << 64) | 5), + i256::from_i128(i128::MAX), + ], + DecimalDType::new(76, 0), + Validity::from_iter([true, false, true, true, true]), + ); + let parts = split_decimal(&decimal, 1, &mut ctx)?; + assert_arrays_eq!( + parts.msp, + PrimitiveArray::new( + buffer![i64::MIN, 0, -1, 1, i64::MAX], + Validity::from_iter([true, false, true, true, true]), + ), + &mut ctx + ); + assert_eq!(parts.lower_parts.len(), 1); + assert_arrays_eq!( + parts.lower_parts[0], + PrimitiveArray::new( + buffer![0u64, 0, u64::MAX, 5, u64::MAX], + Validity::NonNullable + ), + &mut ctx + ); + + let encoded = DecimalByteParts::try_new_with_lower_parts( + parts.msp, + parts.lower_parts, + decimal.decimal_dtype(), + )?; + assert_arrays_eq!(encoded, decimal, &mut ctx); + + Ok(()) +} + +/// Wide values that fit `i64` split into an `i64` MSP without lower parts. The null slot holds +/// a value outside `i64`, which the narrowing copy ignores. +#[rstest] +#[case::i128(decimal( + buffer![i128::from(i64::MIN), 1 << 100, 0, i128::from(i64::MAX)], + Validity::from_iter([true, false, true, true]), +))] +#[case::i256(decimal( + buffer![ + i256::from_i128(i128::from(i64::MIN)), + I256_ABOVE_I128, + i256::ZERO, + i256::from_i128(i128::from(i64::MAX)), + ], + Validity::from_iter([true, false, true, true]), +))] +fn test_split_wide_into_msp_only(#[case] decimal: DecimalArray) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let parts = split_decimal(&decimal, 0, &mut ctx)?; + assert!(parts.lower_parts.is_empty()); + assert_arrays_eq!( + parts.msp, + PrimitiveArray::new( + buffer![i64::MIN, 0, 0, i64::MAX], + Validity::from_iter([true, false, true, true]), + ), + &mut ctx + ); + + Ok(()) +} + +#[rstest] +fn test_split_i256_part_order( + #[values(Validity::NonNullable, Validity::from_iter([true, false, true]))] validity: Validity, +) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let decimal = DecimalArray::new( + buffer![ + i256::from_parts((2u128 << 64) | 3, (1i128 << 64) | 4), + i256::ZERO, + i256::from_parts((6u128 << 64) | 7, (-2i128 << 64) | 5), + ], + DecimalDType::new(76, 0), + validity.clone(), + ); + let parts = split_decimal(&decimal, MAX_LOWER_PARTS, &mut ctx)?; + assert_arrays_eq!( + parts.msp, + PrimitiveArray::new(buffer![1i64, 0, -2], validity), + &mut ctx + ); + assert_eq!(parts.lower_parts.len(), 3); + for (part, expected) in parts.lower_parts.into_iter().zip([ + buffer![4u64, 0, 5], + buffer![2u64, 0, 6], + buffer![3u64, 0, 7], + ]) { + assert_arrays_eq!( + part, + PrimitiveArray::new(expected, Validity::NonNullable), + &mut ctx + ); + } + + Ok(()) +} + +/// A valid value outside the precision can need more lower parts than the dtype allows. +#[rstest] +#[case::wider_i128_storage(DecimalArray::new( + buffer![-99i128, 0, 1 << 70], DecimalDType::new(2, 0), Validity::NonNullable, +))] +#[case::wider_i256_storage(DecimalArray::new( + buffer![i256::from_i128(-99), i256::ZERO, i256::from_i128(1 << 70)], + DecimalDType::new(2, 0), Validity::NonNullable, +))] +fn test_encode_rejects_lower_parts_past_canonical_width(#[case] decimal: DecimalArray) { + let result = DecimalByteParts::encode(&decimal, &mut SESSION.create_execution_ctx()); + assert!( + result + .as_ref() + .is_err_and(|err| err.to_string().contains("allows at most 0 lower parts")), + "expected the lower-part bound to reject the parts, got {result:?}" + ); +} diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/testing.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/testing.rs index 74d9dc456ab..664b6c09987 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/testing.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/testing.rs @@ -8,6 +8,7 @@ use std::sync::LazyLock; use vortex_array::VortexSessionExecute; use vortex_array::arrays::DecimalArray; use vortex_array::dtype::DecimalDType; +use vortex_array::dtype::DecimalType; use vortex_array::dtype::i256; use vortex_array::validity::Validity; use vortex_buffer::Buffer; @@ -16,23 +17,52 @@ use vortex_session::VortexSession; use super::DecimalByteParts; use super::DecimalBytePartsArray; +use super::MAX_I128_LOWER_PARTS; +use super::MAX_I256_LOWER_PARTS; +use super::split_decimal; static SESSION: LazyLock = LazyLock::new(VortexSession::empty); /// An `i128`-backed decimal array, encoded as byte parts with one lower part. pub(crate) fn i128_parts(values: Vec, validity: Validity) -> DecimalBytePartsArray { - DecimalByteParts::encode( + encode_with_lower_parts( &DecimalArray::new(Buffer::from(values), DecimalDType::new(38, 2), validity), - &mut SESSION.create_execution_ctx(), + MAX_I128_LOWER_PARTS, ) - .vortex_expect("valid decimal byte parts") } /// An `i256`-backed decimal array, encoded as byte parts with three lower parts. pub(crate) fn i256_parts(values: Vec, validity: Validity) -> DecimalBytePartsArray { - DecimalByteParts::encode( + encode_with_lower_parts( &DecimalArray::new(Buffer::from(values), DecimalDType::new(76, 2), validity), + MAX_I256_LOWER_PARTS, + ) +} + +/// Encodes `decimal` as byte parts with `lower_part_count` lower parts, whatever its values need. +fn encode_with_lower_parts( + decimal: &DecimalArray, + lower_part_count: usize, +) -> DecimalBytePartsArray { + let parts = split_decimal( + decimal, + lower_part_count, &mut SESSION.create_execution_ctx(), ) + .vortex_expect("the fixture splits into a supported shape"); + DecimalByteParts::try_new_with_lower_parts( + parts.msp, + parts.lower_parts, + decimal.decimal_dtype(), + ) .vortex_expect("valid decimal byte parts") } + +/// The number of lower parts that holds every value of `values_type`. +pub(crate) fn full_width_lower_part_count(values_type: DecimalType) -> usize { + match values_type { + DecimalType::I128 => MAX_I128_LOWER_PARTS, + DecimalType::I256 => MAX_I256_LOWER_PARTS, + _ => 0, + } +} diff --git a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/decimal_byte_parts_v2.rs b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/decimal_byte_parts_v2.rs index 166d84f5ac6..da1a185d595 100644 --- a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/decimal_byte_parts_v2.rs +++ b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/decimal_byte_parts_v2.rs @@ -15,27 +15,12 @@ use vortex::array::dtype::i256; use vortex::array::validity::Validity; use vortex::buffer::Buffer; use vortex::encodings::decimal_byte_parts::DecimalByteParts; -use vortex::encodings::decimal_byte_parts::DecimalBytePartsArray; -use vortex::encodings::decimal_byte_parts::split_decimal; use vortex::error::VortexResult; use vortex_array::ExecutionCtx; use super::N; use crate::fixtures::FlatLayoutFixture; -/// Encode a canonical decimal as byte parts, splitting wide values into lower parts. -fn encode_byte_parts( - decimal: &DecimalArray, - ctx: &mut ExecutionCtx, -) -> VortexResult { - let parts = split_decimal(decimal, ctx)?; - DecimalByteParts::try_new_with_lower_parts( - parts.msp, - parts.lower_parts, - decimal.decimal_dtype(), - ) -} - pub struct DecimalBytePartsV2Fixture; impl FlatLayoutFixture for DecimalBytePartsV2Fixture { @@ -61,7 +46,7 @@ impl FlatLayoutFixture for DecimalBytePartsV2Fixture { wide_128_dtype, Validity::NonNullable, ); - let wide_128_arr = encode_byte_parts(&wide_128, ctx)?; + let wide_128_arr = DecimalByteParts::encode(&wide_128, ctx)?; // Negative values, so the sign extension above the MSP is exercised on read back. let wide_128_negative = DecimalArray::new( @@ -71,7 +56,7 @@ impl FlatLayoutFixture for DecimalBytePartsV2Fixture { wide_128_dtype, Validity::NonNullable, ); - let wide_128_negative_arr = encode_byte_parts(&wide_128_negative, ctx)?; + let wide_128_negative_arr = DecimalByteParts::encode(&wide_128_negative, ctx)?; // An `i256` magnitude beyond 128 bits, so all three lower parts are populated, with // nulls to pin that validity is carried by the MSP alone. @@ -84,7 +69,7 @@ impl FlatLayoutFixture for DecimalBytePartsV2Fixture { wide_256_dtype, Validity::from_iter((0..N).map(|i| i % 7 != 0)), ); - let wide_256_arr = encode_byte_parts(&wide_256, ctx)?; + let wide_256_arr = DecimalByteParts::encode(&wide_256, ctx)?; let arr = StructArray::try_new( FieldNames::from([ From a00c6f2f7b490c0f031b3087ce8559e39fb17e7b Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 4 Nov 2026 14:47:14 -0500 Subject: [PATCH 2/4] Build DecimalByteParts from the canonical array Signed-off-by: Connor Tsui --- vortex-array/src/arrays/decimal/utils.rs | 57 ---------- vortex-btrblocks/src/schemes/decimal.rs | 53 ++++++--- vortex-btrblocks/tests/decimal_config.rs | 133 ++++++++++++++++++++--- 3 files changed, 158 insertions(+), 85 deletions(-) diff --git a/vortex-array/src/arrays/decimal/utils.rs b/vortex-array/src/arrays/decimal/utils.rs index 17a868999b2..cff2e2f743f 100644 --- a/vortex-array/src/arrays/decimal/utils.rs +++ b/vortex-array/src/arrays/decimal/utils.rs @@ -1,8 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors -use itertools::Itertools; -use itertools::MinMaxResult; use vortex_buffer::Buffer; use vortex_buffer::BufferMut; use vortex_compute::lane_kernels::IndexedSourceExt; @@ -19,9 +17,7 @@ use crate::arrays::decimal::DecimalArrayExt; use crate::buffer::BufferHandle; use crate::dtype::BigCast; use crate::dtype::DecimalDType; -use crate::dtype::DecimalType; use crate::dtype::NativeDecimalType; -use crate::dtype::i256; use crate::match_each_decimal_value_type; /// Return the array's unscaled values widened to `W`, which must be at least as wide as the @@ -265,59 +261,6 @@ pub fn check_precision( Ok(()) } -macro_rules! try_downcast { - ($array:expr, from: $src:ty, to: $($dst:ty),*) => {{ - // Collect the min/max of the values - let minmax = $array.buffer::<$src>().iter().copied().minmax(); - match minmax { - MinMaxResult::NoElements => return $array, - MinMaxResult::OneElement(_) => return $array, - MinMaxResult::MinMax(min, max) => { - $( - if <$dst as BigCast>::from(min).is_some() && <$dst as BigCast>::from(max).is_some() { - return DecimalArray::new::<$dst>( - $array - .buffer::<$src>() - .into_iter() - .map(|v| <$dst as BigCast>::from(v).vortex_expect("decimal conversion failure")) - .collect(), - $array.decimal_dtype(), - $array - .validity() - .vortex_expect("decimal validity should be derivable"), - ); - } - )* - - return $array; - } - } - }}; -} - -/// Attempt to narrow the decimal array to any smaller supported type. -pub fn narrowed_decimal(decimal_array: DecimalArray) -> DecimalArray { - match decimal_array.values_type() { - // Cannot narrow any more - DecimalType::I8 => decimal_array, - DecimalType::I16 => { - try_downcast!(decimal_array, from: i16, to: i8) - } - DecimalType::I32 => { - try_downcast!(decimal_array, from: i32, to: i8, i16) - } - DecimalType::I64 => { - try_downcast!(decimal_array, from: i64, to: i8, i16, i32) - } - DecimalType::I128 => { - try_downcast!(decimal_array, from: i128, to: i8, i16, i32, i64) - } - DecimalType::I256 => { - try_downcast!(decimal_array, from: i256, to: i8, i16, i32, i64, i128) - } - } -} - #[cfg(test)] mod tests { use rstest::rstest; diff --git a/vortex-btrblocks/src/schemes/decimal.rs b/vortex-btrblocks/src/schemes/decimal.rs index e59f2a3a535..febd6185317 100644 --- a/vortex-btrblocks/src/schemes/decimal.rs +++ b/vortex-btrblocks/src/schemes/decimal.rs @@ -9,14 +9,15 @@ use vortex_array::Canonical; use vortex_array::ExecutionCtx; use vortex_array::IntoArray; use vortex_array::arrays::DecimalArray; -use vortex_array::arrays::decimal::narrowed_decimal; -use vortex_array::dtype::DecimalType; +use vortex_array::arrays::PrimitiveArray; +use vortex_array::arrays::WidenArray; use vortex_compressor::scheme::CompressionEstimate; use vortex_compressor::scheme::EstimateVerdict; use vortex_decimal_byte_parts::DecimalByteParts; use vortex_decimal_byte_parts::DecimalBytePartsSlots; use vortex_decimal_byte_parts::decimal_byte_parts_v1_id; use vortex_decimal_byte_parts::decimal_byte_parts_v2_id; +use vortex_decimal_byte_parts::lower_part_count; use vortex_decimal_byte_parts::split_decimal; use vortex_error::VortexResult; @@ -42,10 +43,15 @@ pub(crate) static DECIMAL_V2: DecimalScheme = DecimalScheme::v2(); /// Compression scheme for decimal arrays via byte-part decomposition. /// -/// Narrows the decimal to the smallest integer type and compresses its byte parts independently. -/// The v1 mode leaves values wider than `i64` canonical; v2 splits them into a signed most -/// significant part and up to three unsigned lower parts. Single-part arrays serialize as v1 -/// in either mode, while arrays with lower parts serialize as v2. +/// Splits the canonical decimal into a signed most significant part (MSP) and as many unsigned +/// lower parts as its valid values need (see [`lower_part_count`]), then compresses each part +/// independently. Only the MSP of an array without lower parts narrows: it takes the smallest +/// signed integer type that holds its valid values. The decimal itself is never narrowed, and the +/// lower parts stay `u64`, which the constructor requires. +/// +/// The v1 mode leaves an array canonical when a valid value is wider than `i64`; v2 splits it into +/// an `i64` MSP and one or three lower parts. Single-part arrays serialize as v1 in either mode, +/// while arrays with lower parts serialize as v2. /// /// The default uses v1. [`refine`](Scheme::refine) picks v2 when the v2 ID is allowed and v1 /// otherwise. @@ -57,7 +63,7 @@ pub struct DecimalScheme { impl DecimalScheme { /// Creates a decimal scheme configured for v1, disallowing splitting of wide decimals. /// - /// Values that remain wider than `i64` after narrowing stay canonical. + /// Arrays with a valid value wider than `i64` stay canonical. pub const fn v1() -> Self { Self { mode: DecimalSchemeMode::V1, @@ -118,7 +124,7 @@ impl Scheme for DecimalScheme { _compress_ctx: CompressorContext, _exec_ctx: &mut ExecutionCtx, ) -> CompressionEstimate { - // Decimal compression is almost always beneficial (narrowing + primitive compression). + // Decimal compression is almost always beneficial (MSP narrowing + primitive compression). CompressionEstimate::Verdict(EstimateVerdict::AlwaysUse) } @@ -130,16 +136,33 @@ impl Scheme for DecimalScheme { exec_ctx: &mut ExecutionCtx, ) -> VortexResult { let decimal = data.array().clone().execute::(exec_ctx)?; - let decimal = narrowed_decimal(decimal); - if self.mode == DecimalSchemeMode::V1 - && matches!(decimal.values_type(), DecimalType::I128 | DecimalType::I256) - { - return Ok(decimal.into_array()); + let lower_part_count = lower_part_count(&decimal, exec_ctx)?; + + // v1 cannot carry lower parts. Returning the input leaves it canonical, because the cascade + // keeps its input when the output is not smaller. + if self.mode == DecimalSchemeMode::V1 && lower_part_count > 0 { + return Ok(data.array().clone()); } - let parts = split_decimal(&decimal, exec_ctx)?; + let parts = split_decimal(&decimal, lower_part_count, exec_ctx)?; + + let msp = if lower_part_count == 0 { + #[expect( + deprecated, + reason = "TODO(connor)[Widen]: decide whether the MSP narrows its dtype or becomes a Widen (#10733)" + )] + let msp = WidenArray::encode_values( + parts.msp.execute::(exec_ctx)?, + exec_ctx, + )?; + msp.into_array() + } else { + // NB: narrowing an MSP next to lower parts would change the v2 output. + parts.msp + }; + let msp = compressor.compress_child( - &parts.msp, + &msp, &compress_ctx, self.id(), DecimalBytePartsSlots::MSP, diff --git a/vortex-btrblocks/tests/decimal_config.rs b/vortex-btrblocks/tests/decimal_config.rs index 257d1b1aa60..2f02b756695 100644 --- a/vortex-btrblocks/tests/decimal_config.rs +++ b/vortex-btrblocks/tests/decimal_config.rs @@ -1,7 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors -//! Decimal scheme refinement by allowed serialized IDs, and compression of wide decimal parts. +//! Decimal scheme refinement by allowed serialized IDs, the shapes of its output, and compression +//! of wide decimal parts. #![cfg(test)] @@ -17,6 +18,8 @@ use vortex_array::arrays::Decimal; use vortex_array::arrays::DecimalArray; use vortex_array::assert_arrays_eq; use vortex_array::dtype::DecimalDType; +use vortex_array::dtype::NativeDecimalType; +use vortex_array::dtype::PType; use vortex_array::dtype::i256; use vortex_array::serde::SerializeOptions; use vortex_array::serde::SerializedArray; @@ -98,6 +101,7 @@ fn decimal_array(wide: bool) -> ArrayRef { .into_array() } +#[track_caller] fn assert_decimal_output( builder: BtrBlocksCompressorBuilder, wide: bool, @@ -115,10 +119,36 @@ fn assert_decimal_output( } else { assert!(compressed.is::()); } - assert_arrays_eq!(array, compressed, &mut ctx); + assert_arrays_eq!(compressed, array, &mut ctx); + Ok(()) } +/// A decimal array of `values` at precision `precision` and scale 2. +fn decimal_at_precision( + precision: u8, + values: Buffer, + validity: Validity, +) -> ArrayRef { + DecimalArray::new(values, DecimalDType::new(precision, 2), validity).into_array() +} + +/// Serializes `array` and decodes it again. +fn serde_round_trip(array: &ArrayRef) -> VortexResult { + let array_ctx = ArrayContext::empty(); + let mut bytes = ByteBufferMut::empty(); + for buffer in array.serialize(&array_ctx, &SESSION, &SerializeOptions::default())? { + bytes.extend_from_slice(buffer.as_ref()); + } + + SerializedArray::try_from(bytes.freeze())?.decode( + array.dtype(), + array.len(), + &ReadContext::new(array_ctx.to_ids()), + &SESSION, + ) +} + #[rstest] #[case::no_editions(|| BtrBlocksCompressorBuilder::from_session(&SESSION), Expected::Canonical)] #[case::core_edition(|| BtrBlocksCompressorBuilder::from_session(&CORE_SESSION), Expected::V1)] @@ -215,17 +245,94 @@ fn wide_decimal_parts_roundtrip( assert!(parts.lower_parts().iter().all(|part| !part.is_canonical())); } - let array_ctx = ArrayContext::empty(); - let mut bytes = ByteBufferMut::empty(); - for buffer in compressed.serialize(&array_ctx, &SESSION, &SerializeOptions::default())? { - bytes.extend_from_slice(buffer.as_ref()); + assert_arrays_eq!(serde_round_trip(&compressed)?, array, &mut ctx); + + Ok(()) +} + +/// The shape of the scheme output for each storage width and value range. v1 leaves an array with +/// lower parts canonical. Only the valid values choose the lower-part count and the MSP type. +#[rstest] +#[case::positive_values_keep_signed_msp( + decimal_at_precision(12, (0..2048i64).collect::>(), Validity::NonNullable), + 0, + PType::I16 +)] +#[case::p38_fits_i64( + decimal_at_precision(38, (0..2048i128).collect::>(), Validity::NonNullable), + 0, + PType::I16 +)] +#[case::p38_above_64_bits( + decimal_at_precision( + 38, + (0..2048i128).map(|i| (1 << 70) + i).collect::>(), + Validity::NonNullable, + ), + 1, + PType::I64 +)] +#[case::p40_fits_i64( + decimal_at_precision( + 40, + (0..2048i128).map(i256::from_i128).collect::>(), + Validity::NonNullable, + ), + 0, + PType::I16 +)] +#[case::p40_above_64_bits( + decimal_at_precision( + 40, + (0..2048i128).map(|i| i256::from_i128((1 << 70) + i)).collect::>(), + Validity::NonNullable, + ), + 1, + PType::I64 +)] +#[case::p76_above_128_bits( + decimal_at_precision( + 76, + (0..2048u128).map(|i| i256::from_parts(i, 1 << 70)).collect::>(), + Validity::NonNullable, + ), + 3, + PType::I64 +)] +#[case::p38_large_null_slots( + decimal_at_precision( + 38, + (0..2048i128).map(|i| if i % 7 == 0 { 1 << 100 } else { i }).collect::>(), + Validity::from_iter((0..2048).map(|i| i % 7 != 0)), + ), + 0, + PType::I16 +)] +fn decimal_scheme_part_shapes( + #[case] array: ArrayRef, + #[case] lower_part_count: usize, + #[case] msp_ptype: PType, + #[values(false, true)] v2: bool, +) -> VortexResult<()> { + let builder = if v2 { + BtrBlocksCompressorBuilder::from_session(&SESSION).disable_editions() + } else { + BtrBlocksCompressorBuilder::from_session(&CORE_SESSION) + }; + let mut ctx = SESSION.create_execution_ctx(); + let compressed = builder.build().compress(&array, &mut ctx)?; + + if !v2 && lower_part_count > 0 { + assert!(compressed.is::()); + } else { + let parts = compressed + .as_opt::() + .ok_or_else(|| vortex_err!("expected decimal byte parts"))?; + assert_eq!(parts.lower_parts().len(), lower_part_count); + assert_eq!(parts.msp().dtype().as_ptype(), msp_ptype); } - let decoded = SerializedArray::try_from(bytes.freeze())?.decode( - array.dtype(), - array.len(), - &ReadContext::new(array_ctx.to_ids()), - &SESSION, - )?; - assert_arrays_eq!(array, decoded, &mut ctx); + + assert_arrays_eq!(serde_round_trip(&compressed)?, array, &mut ctx); + Ok(()) } From 26c7cfb1e1d4f506f10f99409c7e12970a24bda4 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 4 Nov 2026 14:47:14 -0500 Subject: [PATCH 3/4] Cover canonical decimal compression in the golden corpus Signed-off-by: Connor Tsui --- vortex-btrblocks/tests/golden.rs | 67 +++++++++++++++++++ .../golden__compact__decimal_negative.snap | 11 +++ ...en__compact__decimal_null_slot_values.snap | 11 +++ .../golden__compact__decimal_nullable.snap | 11 +++ ...pact__decimal_wide_dtype_small_values.snap | 9 +++ .../golden__compact__decimal_wide_p40.snap | 7 ++ .../golden__regular__decimal_negative.snap | 11 +++ ...en__regular__decimal_null_slot_values.snap | 11 +++ .../golden__regular__decimal_nullable.snap | 11 +++ ...ular__decimal_wide_dtype_small_values.snap | 9 +++ .../golden__regular__decimal_wide_p40.snap | 7 ++ 11 files changed, 165 insertions(+) create mode 100644 vortex-btrblocks/tests/snapshots/golden__compact__decimal_negative.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__compact__decimal_null_slot_values.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__compact__decimal_nullable.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__compact__decimal_wide_dtype_small_values.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__compact__decimal_wide_p40.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__regular__decimal_negative.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__regular__decimal_null_slot_values.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__regular__decimal_nullable.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__regular__decimal_wide_dtype_small_values.snap create mode 100644 vortex-btrblocks/tests/snapshots/golden__regular__decimal_wide_p40.snap diff --git a/vortex-btrblocks/tests/golden.rs b/vortex-btrblocks/tests/golden.rs index bd7fc3fd78b..19f81c92951 100644 --- a/vortex-btrblocks/tests/golden.rs +++ b/vortex-btrblocks/tests/golden.rs @@ -23,6 +23,7 @@ #![allow(clippy::cast_possible_truncation, clippy::tests_outside_test_module)] use std::fmt; +use std::ops::Range; use std::sync::Arc; use std::sync::LazyLock; @@ -47,6 +48,7 @@ use vortex_array::display::TreeExtractor; use vortex_array::dtype::DType; use vortex_array::dtype::DecimalDType; use vortex_array::dtype::Nullability; +use vortex_array::dtype::i256; use vortex_array::extension::datetime::TimeUnit; #[cfg(feature = "pco")] use vortex_array::session::ArraySessionExt; @@ -161,6 +163,17 @@ fn corpus() -> VortexResult> { ("string_low_cardinality", string_low_cardinality()), ("binary_low_cardinality", binary_low_cardinality()), ("decimal_prices", decimal_prices()), + ( + "decimal_wide_dtype_small_values", + decimal_wide_dtype_small_values(), + ), + ("decimal_nullable", decimal_nullable(120, None)), + ("decimal_negative", decimal_negative()), + ("decimal_wide_p40", decimal_wide_p40()), + ( + "decimal_null_slot_values", + decimal_nullable(123, Some(1i64 << 40..1i64 << 48)), + ), ("temporal_timestamp_micros", temporal_timestamp_micros()), ("bool_random", bool_random()), ("struct_mixed", struct_mixed()?), @@ -373,6 +386,60 @@ fn decimal_prices() -> ArrayRef { DecimalArray::new(values, DecimalDType::new(12, 2), Validity::NonNullable).into_array() } +/// [`decimal_prices`] at a precision whose canonical width is `i128`: the values fit `i64`, so the +/// byte parts need no lower part. +fn decimal_wide_dtype_small_values() -> ArrayRef { + let mut rng = StdRng::seed_from_u64(119); + let values: Buffer = (0..N) + .map(|_| rng.random_range(0..10_000_000i128)) + .collect(); + DecimalArray::new(values, DecimalDType::new(38, 2), Validity::NonNullable).into_array() +} + +/// Seeded prices with a tenth of the rows null. Null slots hold zeros unless a range is supplied. +fn decimal_nullable(seed: u64, null_slot_range: Option>) -> ArrayRef { + let mut rng = StdRng::seed_from_u64(seed); + let mut validity: Vec = Vec::with_capacity(N); + let values: Buffer = (0..N) + .map(|_| { + let valid = rng.random_range(0..10) != 0; + validity.push(valid); + if valid { + rng.random_range(0..10_000_000i64) + } else if let Some(range) = &null_slot_range { + rng.random_range(range.clone()) + } else { + 0 + } + }) + .collect(); + DecimalArray::new( + values, + DecimalDType::new(12, 2), + Validity::Array(BoolArray::from_iter(validity).into_array()), + ) + .into_array() +} + +/// Decimals of mixed sign: the most significant part stays signed. +fn decimal_negative() -> ArrayRef { + let mut rng = StdRng::seed_from_u64(121); + let values: Buffer = (0..N) + .map(|_| rng.random_range(-100_000..=100_000i64)) + .collect(); + DecimalArray::new(values, DecimalDType::new(10, 2), Validity::NonNullable).into_array() +} + +/// Decimals at a precision whose canonical width is `i256`, with values from 2^64 to 2^100: they +/// need more than 64 bits but fit `i128`. +fn decimal_wide_p40() -> ArrayRef { + let mut rng = StdRng::seed_from_u64(122); + let values: Buffer = (0..N) + .map(|_| i256::from_i128(rng.random_range(1i128 << 64..1i128 << 100))) + .collect(); + DecimalArray::new(values, DecimalDType::new(40, 2), Validity::NonNullable).into_array() +} + /// Near-monotone microsecond timestamps: TemporalScheme (datetime-parts) habitat. fn temporal_timestamp_micros() -> ArrayRef { let mut rng = StdRng::seed_from_u64(116); diff --git a/vortex-btrblocks/tests/snapshots/golden__compact__decimal_negative.snap b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_negative.snap new file mode 100644 index 00000000000..609b57d9d8c --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_negative.snap @@ -0,0 +1,11 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(10,2), len=16384, nbytes=131072 +root: vortex.decimal_byte_parts.v2(decimal(10,2), len=16384) nbytes=36105 + metadata: + msp: vortex.zigzag(i32, len=16384) nbytes=36105 + metadata: + encoded: vortex.pco(u32, len=16384) nbytes=36105 + metadata: ptype: u32, nrows: 16384, slice: 0..16384 diff --git a/vortex-btrblocks/tests/snapshots/golden__compact__decimal_null_slot_values.snap b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_null_slot_values.snap new file mode 100644 index 00000000000..70c05254f0a --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_null_slot_values.snap @@ -0,0 +1,11 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(12,2)?, len=16384, nbytes=133120 +root: vortex.decimal_byte_parts.v2(decimal(12,2)?, len=16384) nbytes=45028 + metadata: + msp: vortex.pco(i32?, len=16384) nbytes=45028 + metadata: ptype: i32, nrows: 16384, slice: 0..16384 + validity: vortex.bool(bool, len=16384) nbytes=2048 + metadata: offset: 0 diff --git a/vortex-btrblocks/tests/snapshots/golden__compact__decimal_nullable.snap b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_nullable.snap new file mode 100644 index 00000000000..d4ed8eaa90c --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_nullable.snap @@ -0,0 +1,11 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(12,2)?, len=16384, nbytes=133120 +root: vortex.decimal_byte_parts.v2(decimal(12,2)?, len=16384) nbytes=44918 + metadata: + msp: vortex.pco(i32?, len=16384) nbytes=44918 + metadata: ptype: i32, nrows: 16384, slice: 0..16384 + validity: vortex.bool(bool, len=16384) nbytes=2048 + metadata: offset: 0 diff --git a/vortex-btrblocks/tests/snapshots/golden__compact__decimal_wide_dtype_small_values.snap b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_wide_dtype_small_values.snap new file mode 100644 index 00000000000..b8f1cc4c3ef --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_wide_dtype_small_values.snap @@ -0,0 +1,9 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(38,2), len=16384, nbytes=262144 +root: vortex.decimal_byte_parts.v2(decimal(38,2), len=16384) nbytes=47682 + metadata: + msp: vortex.pco(i32, len=16384) nbytes=47682 + metadata: ptype: i32, nrows: 16384, slice: 0..16384 diff --git a/vortex-btrblocks/tests/snapshots/golden__compact__decimal_wide_p40.snap b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_wide_p40.snap new file mode 100644 index 00000000000..e61c0429119 --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__compact__decimal_wide_p40.snap @@ -0,0 +1,7 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(40,2), len=16384, nbytes=524288 +root: vortex.decimal(decimal(40,2), len=16384) nbytes=524288 + metadata: decimal_dtype: decimal(40,2), values_type: i256 diff --git a/vortex-btrblocks/tests/snapshots/golden__regular__decimal_negative.snap b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_negative.snap new file mode 100644 index 00000000000..c04be9ffd4a --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_negative.snap @@ -0,0 +1,11 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(10,2), len=16384, nbytes=131072 +root: vortex.decimal_byte_parts.v2(decimal(10,2), len=16384) nbytes=36864 + metadata: + msp: fastlanes.for.v2(i32, len=16384) nbytes=36864 + metadata: global_reference: -99991i32, offset: 0 + encoded: fastlanes.bitpacked.v2(i32, len=16384) nbytes=36864 + metadata: bit_width: 18, offset: 0 diff --git a/vortex-btrblocks/tests/snapshots/golden__regular__decimal_null_slot_values.snap b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_null_slot_values.snap new file mode 100644 index 00000000000..0f1028d68d6 --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_null_slot_values.snap @@ -0,0 +1,11 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(12,2)?, len=16384, nbytes=133120 +root: vortex.decimal_byte_parts.v2(decimal(12,2)?, len=16384) nbytes=51200 + metadata: + msp: fastlanes.bitpacked.v2(i32?, len=16384) nbytes=51200 + metadata: bit_width: 24, offset: 0 + validity_child: vortex.bool(bool, len=16384) nbytes=2048 + metadata: offset: 0 diff --git a/vortex-btrblocks/tests/snapshots/golden__regular__decimal_nullable.snap b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_nullable.snap new file mode 100644 index 00000000000..0f1028d68d6 --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_nullable.snap @@ -0,0 +1,11 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(12,2)?, len=16384, nbytes=133120 +root: vortex.decimal_byte_parts.v2(decimal(12,2)?, len=16384) nbytes=51200 + metadata: + msp: fastlanes.bitpacked.v2(i32?, len=16384) nbytes=51200 + metadata: bit_width: 24, offset: 0 + validity_child: vortex.bool(bool, len=16384) nbytes=2048 + metadata: offset: 0 diff --git a/vortex-btrblocks/tests/snapshots/golden__regular__decimal_wide_dtype_small_values.snap b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_wide_dtype_small_values.snap new file mode 100644 index 00000000000..2b1e1d28aa3 --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_wide_dtype_small_values.snap @@ -0,0 +1,9 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(38,2), len=16384, nbytes=262144 +root: vortex.decimal_byte_parts.v2(decimal(38,2), len=16384) nbytes=49152 + metadata: + msp: fastlanes.bitpacked.v2(i32, len=16384) nbytes=49152 + metadata: bit_width: 24, offset: 0 diff --git a/vortex-btrblocks/tests/snapshots/golden__regular__decimal_wide_p40.snap b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_wide_p40.snap new file mode 100644 index 00000000000..e61c0429119 --- /dev/null +++ b/vortex-btrblocks/tests/snapshots/golden__regular__decimal_wide_p40.snap @@ -0,0 +1,7 @@ +--- +source: vortex-btrblocks/tests/golden.rs +expression: rendered +--- +input: decimal(40,2), len=16384, nbytes=524288 +root: vortex.decimal(decimal(40,2), len=16384) nbytes=524288 + metadata: decimal_dtype: decimal(40,2), values_type: i256 From 3ce38dba16442f9a4165eec792f7b688e55847b6 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 4 Nov 2026 14:47:57 -0500 Subject: [PATCH 4/4] Check split errors without requiring Debug on DecimalParts Signed-off-by: Connor Tsui --- .../src/decimal_byte_parts/split/tests.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/split/tests.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/split/tests.rs index cb3da13cab4..dfa4323b9f3 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/split/tests.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/split/tests.rs @@ -39,12 +39,14 @@ fn decimal(values: Buffer, validity: Validity) -> Decim /// Runs splitting through validation and checks the reported requirement. #[track_caller] fn assert_split_error(decimal: &DecimalArray, lower_part_count: usize, expected: &str) { - let error = split_decimal( + let error = match split_decimal( decimal, lower_part_count, &mut SESSION.create_execution_ctx(), - ) - .expect_err("the fixture violates a split requirement"); + ) { + Ok(_) => panic!("the fixture must violate a split requirement"), + Err(error) => error, + }; assert!(error.to_string().contains(expected), "{error}"); }