From 25713a5c3f36cfb2247df6015d2620d71d34f9b0 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Tue, 23 Jun 2026 16:47:02 -0400 Subject: [PATCH 1/4] Fix decimal cast rescaling Signed-off-by: "Nicholas Gates" --- .../src/arrays/constant/compute/cast.rs | 25 +++ .../src/arrays/decimal/compute/cast.rs | 192 ++++++++++++++---- .../src/scalar/typed_view/decimal/dvalue.rs | 87 ++++++++ .../src/scalar/typed_view/decimal/scalar.rs | 15 +- .../src/scalar/typed_view/decimal/tests.rs | 87 +++++++- .../src/scalar/typed_view/primitive/scalar.rs | 39 +++- 6 files changed, 378 insertions(+), 67 deletions(-) diff --git a/vortex-array/src/arrays/constant/compute/cast.rs b/vortex-array/src/arrays/constant/compute/cast.rs index 2fd9b3ded32..db3a41829f0 100644 --- a/vortex-array/src/arrays/constant/compute/cast.rs +++ b/vortex-array/src/arrays/constant/compute/cast.rs @@ -25,8 +25,15 @@ mod tests { use rstest::rstest; use crate::IntoArray; + use crate::LEGACY_SESSION; + use crate::VortexSessionExecute; use crate::arrays::ConstantArray; + use crate::builtins::ArrayBuiltins; use crate::compute::conformance::cast::test_cast_conformance; + use crate::dtype::DType; + use crate::dtype::DecimalDType; + use crate::dtype::Nullability; + use crate::scalar::DecimalValue; use crate::scalar::Scalar; #[rstest] @@ -39,4 +46,22 @@ mod tests { fn test_cast_constant_conformance(#[case] array: crate::ArrayRef) { test_cast_conformance(&array); } + + #[test] + fn test_cast_constant_i64_to_decimal() { + let target_dtype = DType::Decimal(DecimalDType::new(21, 2), Nullability::NonNullable); + let casted = ConstantArray::new(Scalar::from(42i64), 5) + .into_array() + .cast(target_dtype.clone()) + .unwrap(); + + assert_eq!(casted.dtype(), &target_dtype); + let scalar = casted + .execute_scalar(0, &mut LEGACY_SESSION.create_execution_ctx()) + .unwrap(); + assert_eq!( + scalar.as_decimal().decimal_value(), + Some(DecimalValue::I128(4200)) + ); + } } diff --git a/vortex-array/src/arrays/decimal/compute/cast.rs b/vortex-array/src/arrays/decimal/compute/cast.rs index 432313d3cb6..6c1ea5a1e4d 100644 --- a/vortex-array/src/arrays/decimal/compute/cast.rs +++ b/vortex-array/src/arrays/decimal/compute/cast.rs @@ -2,10 +2,13 @@ // SPDX-FileCopyrightText: Copyright the Vortex contributors use vortex_buffer::Buffer; +use vortex_buffer::BufferMut; use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; +use vortex_error::vortex_err; use vortex_error::vortex_panic; +use vortex_mask::Mask; use crate::ArrayRef; use crate::ExecutionCtx; @@ -14,11 +17,14 @@ use crate::array::ArrayView; use crate::arrays::Decimal; use crate::arrays::DecimalArray; use crate::dtype::DType; +use crate::dtype::DecimalDType; use crate::dtype::DecimalType; use crate::dtype::NativeDecimalType; use crate::match_each_decimal_value_type; +use crate::scalar::DecimalValue; use crate::scalar_fn::fns::cast::CastKernel; use crate::scalar_fn::fns::cast::CastReduce; +use crate::validity::Validity; impl CastReduce for Decimal { fn cast(array: ArrayView<'_, Decimal>, dtype: &DType) -> VortexResult> { @@ -77,55 +83,112 @@ impl CastKernel for Decimal { ); }; - // Scale changes are not yet supported - if from_decimal_dtype.scale() != to_decimal_dtype.scale() { - vortex_bail!( - "Casting decimal with scale {} to scale {} not yet implemented", - from_decimal_dtype.scale(), - to_decimal_dtype.scale() - ); - } - - // Downcasting precision is not yet supported - if to_decimal_dtype.precision() < from_decimal_dtype.precision() { - vortex_bail!( - "Downcasting decimal from precision {} to {} not yet implemented", - from_decimal_dtype.precision(), - to_decimal_dtype.precision() - ); - } - // If the dtype is exactly the same, return self if array.dtype() == dtype { return Ok(Some(array.array().clone())); } + let validity = array.validity()?; + // Cast the validity to the new nullability - let new_validity = array - .validity()? + let new_validity = validity + .clone() .cast_nullability(*to_nullability, array.len(), ctx)?; - // If the target needs a wider physical type, upcast the values + if from_decimal_dtype == to_decimal_dtype { + // SAFETY: new_validity has the same length, only its nullability tag changes. + unsafe { + return Ok(Some( + DecimalArray::new_unchecked_handle( + array.buffer_handle().clone(), + array.values_type(), + *to_decimal_dtype, + new_validity, + ) + .into_array(), + )); + } + } + + let valid_values = validity.execute_mask(array.len(), ctx)?; let target_values_type = DecimalType::smallest_decimal_value_type(to_decimal_dtype); - let array = if target_values_type > array.values_type() { - upcast_decimal_values(array, target_values_type)? - } else { - array.array().as_::().into_owned() - }; - // SAFETY: new_validity same length as previous validity, just cast - unsafe { - Ok(Some( - DecimalArray::new_unchecked_handle( - array.buffer_handle().clone(), - array.values_type(), + match_each_decimal_value_type!(array.values_type(), |F| { + match_each_decimal_value_type!(target_values_type, |T| { + cast_decimal_values::( + array, + *from_decimal_dtype, *to_decimal_dtype, new_validity, + &valid_values, ) - .into_array(), - )) + .map(Some) + }) + }) + } +} + +fn cast_decimal_values( + array: ArrayView<'_, Decimal>, + from_decimal_dtype: DecimalDType, + to_decimal_dtype: DecimalDType, + validity: Validity, + valid_values: &Mask, +) -> VortexResult +where + F: NativeDecimalType, + T: NativeDecimalType, + DecimalValue: From, +{ + let values = array.buffer::(); + let mut buffer = BufferMut::::zeroed(values.len()); + + match valid_values { + Mask::AllTrue(_) => { + for (idx, value) in values.iter().copied().enumerate() { + buffer[idx] = + cast_decimal_value::(value, from_decimal_dtype, to_decimal_dtype)?; + } + } + Mask::AllFalse(_) => {} + Mask::Values(mask) => { + for (idx, (value, is_valid)) in values + .iter() + .copied() + .zip(mask.bit_buffer().iter()) + .enumerate() + { + if is_valid { + buffer[idx] = + cast_decimal_value::(value, from_decimal_dtype, to_decimal_dtype)?; + } + } } } + + Ok(DecimalArray::new(buffer.freeze(), to_decimal_dtype, validity).into_array()) +} + +fn cast_decimal_value( + value: F, + from_decimal_dtype: DecimalDType, + to_decimal_dtype: DecimalDType, +) -> VortexResult +where + F: NativeDecimalType, + T: NativeDecimalType, + DecimalValue: From, +{ + DecimalValue::from(value) + .cast_decimal(from_decimal_dtype, to_decimal_dtype)? + .cast::() + .ok_or_else(|| { + vortex_err!( + "decimal value cannot be represented as {} after casting to {}", + T::DECIMAL_TYPE, + to_decimal_dtype + ) + }) } /// Upcast a DecimalArray to a wider physical representation (e.g., i32 -> i64) while keeping @@ -262,19 +325,59 @@ mod tests { } #[test] - fn cast_different_scale_fails() { + fn cast_different_scale_rescales() { let array = DecimalArray::new( buffer![100i32], DecimalDType::new(10, 2), Validity::NonNullable, ); - // Try to cast to different scale - not supported + // Cast 1.00 to scale 3, where it is stored as 1000. let different_dtype = DType::Decimal(DecimalDType::new(15, 3), Nullability::NonNullable); #[expect(deprecated)] - let result = array + let casted = array .into_array() .cast(different_dtype) + .unwrap() + .to_decimal(); + + assert_eq!(casted.precision(), 15); + assert_eq!(casted.scale(), 3); + assert_eq!(casted.values_type(), DecimalType::I64); + assert_eq!(casted.buffer::().as_ref(), &[1000]); + } + + #[test] + fn cast_downcast_precision_succeeds_when_values_fit() { + let array = DecimalArray::new( + buffer![100i64], + DecimalDType::new(18, 2), + Validity::NonNullable, + ); + + // Downcasting precision is allowed when every value fits. + let smaller_dtype = DType::Decimal(DecimalDType::new(10, 2), Nullability::NonNullable); + #[expect(deprecated)] + let casted = array.into_array().cast(smaller_dtype).unwrap().to_decimal(); + + assert_eq!(casted.precision(), 10); + assert_eq!(casted.scale(), 2); + assert_eq!(casted.buffer::().as_ref(), &[100]); + } + + #[test] + fn cast_downcast_precision_checks_values() { + let array = DecimalArray::new( + buffer![1000i64], + DecimalDType::new(18, 0), + Validity::NonNullable, + ); + + let smaller_dtype = DType::Decimal(DecimalDType::new(3, 0), Nullability::NonNullable); + #[expect(deprecated)] + let result = array + .into_array() + .cast(smaller_dtype) .and_then(|a| a.to_canonical().map(|c| c.into_array())); assert!(result.is_err()); @@ -282,24 +385,23 @@ mod tests { result .unwrap_err() .to_string() - .contains("Casting decimal with scale 2 to scale 3 not yet implemented") + .contains("does not fit in precision") ); } #[test] - fn cast_downcast_precision_fails() { + fn cast_lower_scale_requires_exact_rescale() { let array = DecimalArray::new( - buffer![100i64], - DecimalDType::new(18, 2), + buffer![123456i64], + DecimalDType::new(10, 4), Validity::NonNullable, ); - // Try to downcast precision - not supported - let smaller_dtype = DType::Decimal(DecimalDType::new(10, 2), Nullability::NonNullable); + let lower_scale_dtype = DType::Decimal(DecimalDType::new(10, 2), Nullability::NonNullable); #[expect(deprecated)] let result = array .into_array() - .cast(smaller_dtype) + .cast(lower_scale_dtype) .and_then(|a| a.to_canonical().map(|c| c.into_array())); assert!(result.is_err()); @@ -307,7 +409,7 @@ mod tests { result .unwrap_err() .to_string() - .contains("Downcasting decimal from precision 18 to 10 not yet implemented") + .contains("would lose precision") ); } diff --git a/vortex-array/src/scalar/typed_view/decimal/dvalue.rs b/vortex-array/src/scalar/typed_view/decimal/dvalue.rs index f13571c8509..c1a3be7b71c 100644 --- a/vortex-array/src/scalar/typed_view/decimal/dvalue.rs +++ b/vortex-array/src/scalar/typed_view/decimal/dvalue.rs @@ -12,7 +12,11 @@ use num_traits::CheckedDiv; use num_traits::CheckedMul; use num_traits::CheckedSub; use vortex_error::VortexExpect; +use vortex_error::VortexResult; +use vortex_error::vortex_bail; +use vortex_error::vortex_err; +use crate::dtype::BigCast; use crate::dtype::DecimalDType; use crate::dtype::DecimalType; use crate::dtype::NativeDecimalType; @@ -116,6 +120,80 @@ impl DecimalValue { }) } + /// Rescales a stored decimal value from one scale to another. + /// + /// This preserves the represented numeric value exactly. Reducing scale fails if doing so + /// would discard non-zero fractional digits. + pub(crate) fn rescale_i256(value: i256, from_scale: i8, to_scale: i8) -> VortexResult { + if from_scale == to_scale || value == i256::ZERO { + return Ok(value); + } + + let scale_delta = to_scale as i16 - from_scale as i16; + if scale_delta > 0 { + let factor = decimal_scale_factor(scale_delta as u32)?; + value.checked_mul(&factor).ok_or_else(|| { + vortex_err!( + "Rescaling decimal from scale {} to {} overflows", + from_scale, + to_scale + ) + }) + } else { + let factor = decimal_scale_factor((-scale_delta) as u32)?; + let remainder = value % factor; + if remainder != i256::ZERO { + vortex_bail!( + "Rescaling decimal value {} from scale {} to {} would lose precision", + value, + from_scale, + to_scale + ); + } + Ok(value / factor) + } + } + + /// Rescales this value to `to_decimal_dtype`, checks precision, and stores it in the target + /// decimal value width. + pub(crate) fn cast_decimal( + &self, + from_decimal_dtype: DecimalDType, + to_decimal_dtype: DecimalDType, + ) -> VortexResult { + let rescaled = Self::rescale_i256( + self.as_i256(), + from_decimal_dtype.scale(), + to_decimal_dtype.scale(), + )?; + Self::try_from_i256(rescaled, to_decimal_dtype) + } + + /// Converts an untyped stored decimal integer into the physical value type selected by + /// `decimal_dtype`, after enforcing the dtype precision. + pub(crate) fn try_from_i256(value: i256, decimal_dtype: DecimalDType) -> VortexResult { + let decimal_value = Self::I256(value); + if !decimal_value.fits_in_precision(decimal_dtype) { + vortex_bail!( + "decimal value {} does not fit in precision of {}", + decimal_value, + decimal_dtype + ); + } + + let target_type = DecimalType::smallest_decimal_value_type(&decimal_dtype); + match_each_decimal_value_type!(target_type, |T| { + let value = ::from(value).ok_or_else(|| { + vortex_err!( + "decimal value {} cannot be represented as {}", + decimal_value, + target_type + ) + })?; + Ok(Self::from(value)) + }) + } + /// Returns the 0 value given the [`DecimalType`]. pub fn zero(decimal_type: &DecimalDType) -> Self { let smallest_type = DecimalType::smallest_decimal_value_type(decimal_type); @@ -172,6 +250,15 @@ impl DecimalValue { } } +fn decimal_scale_factor(exp: u32) -> VortexResult { + i256::from_i128(10).checked_pow(exp).ok_or_else(|| { + vortex_err!( + "decimal scale factor 10^{} cannot be represented in i256", + exp + ) + }) +} + // Additional trait implementations for decimal types to ensure consistency. // Comparisons between DecimalValue types should upcast to i256 and operate in the upcast space. diff --git a/vortex-array/src/scalar/typed_view/decimal/scalar.rs b/vortex-array/src/scalar/typed_view/decimal/scalar.rs index b0890177f21..e365a1be02c 100644 --- a/vortex-array/src/scalar/typed_view/decimal/scalar.rs +++ b/vortex-array/src/scalar/typed_view/decimal/scalar.rs @@ -69,20 +69,9 @@ impl<'a> DecimalScalar<'a> { pub(crate) fn cast(&self, dtype: &DType) -> VortexResult { match dtype { DType::Decimal(target_dtype, target_nullability) => { - // Cast between decimal types - if self.decimal_type == *target_dtype { - // Same decimal type, just change nullability if needed - return Scalar::try_new( - dtype.clone(), - self.decimal_value.map(ScalarValue::Decimal), - ); - } - - // TODO(connor): Implement proper decimal scaling logic - whatever that means??? - // Different precision/scale - need to implement scaling logic - // For now, we'll do a simple value preservation without scaling if let Some(value) = &self.decimal_value { - Ok(Scalar::decimal(*value, *target_dtype, *target_nullability)) + let value = value.cast_decimal(self.decimal_type, *target_dtype)?; + Ok(Scalar::decimal(value, *target_dtype, *target_nullability)) } else { Ok(Scalar::null(dtype.clone())) } diff --git a/vortex-array/src/scalar/typed_view/decimal/tests.rs b/vortex-array/src/scalar/typed_view/decimal/tests.rs index 540fcb526a0..63f924b2239 100644 --- a/vortex-array/src/scalar/typed_view/decimal/tests.rs +++ b/vortex-array/src/scalar/typed_view/decimal/tests.rs @@ -115,7 +115,7 @@ fn test_decimal_cast_between_decimal_types() { Nullability::NonNullable, ); - // Cast to different decimal type (currently just preserves value) + // Cast to different decimal type. let result = decimal_scalar .cast(&DType::Decimal( DecimalDType::new(20, 4), @@ -123,9 +123,9 @@ fn test_decimal_cast_between_decimal_types() { )) .unwrap(); - // Value should be preserved (TODO(connor): proper scaling logic - whatever that means???) + // 123.45 with scale 2 is represented as 123.4500 with scale 4. let decimal_value: Option = result.try_into().unwrap(); - assert_eq!(decimal_value, Some(DecimalValue::I32(12345))); + assert_eq!(decimal_value, Some(DecimalValue::I128(1234500))); } #[test] @@ -309,7 +309,6 @@ fn test_decimal_to_decimal_different_scale() { ); // Cast to decimal with scale=4 - // TODO: This should properly rescale, but for now it preserves the raw value let target_dtype = DType::Decimal(DecimalDType::new(10, 4), Nullability::NonNullable); let result = decimal.cast(&target_dtype); assert!(result.is_ok()); @@ -317,7 +316,85 @@ fn test_decimal_to_decimal_different_scale() { let casted = result.unwrap(); assert_eq!( casted.as_decimal().decimal_value(), - Some(DecimalValue::I32(10000)) + Some(DecimalValue::I64(1000000)) + ); +} + +#[test] +fn test_decimal_to_decimal_lower_scale_exact() { + let decimal = Scalar::decimal( + DecimalValue::I64(1234500), // 123.4500 + DecimalDType::new(10, 4), + Nullability::NonNullable, + ); + + let casted = decimal + .cast(&DType::Decimal( + DecimalDType::new(10, 2), + Nullability::NonNullable, + )) + .unwrap(); + + assert_eq!( + casted.as_decimal().decimal_value(), + Some(DecimalValue::I64(12345)) + ); +} + +#[test] +fn test_decimal_to_decimal_lower_scale_lossy_fails() { + let decimal = Scalar::decimal( + DecimalValue::I64(1234567), // 123.4567 + DecimalDType::new(10, 4), + Nullability::NonNullable, + ); + + let result = decimal.cast(&DType::Decimal( + DecimalDType::new(10, 2), + Nullability::NonNullable, + )); + + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("would lose precision") + ); +} + +#[test] +fn test_primitive_i64_to_decimal_rescales() { + let scalar = Scalar::primitive(42i64, Nullability::NonNullable); + + let casted = scalar + .cast(&DType::Decimal( + DecimalDType::new(21, 2), + Nullability::NonNullable, + )) + .unwrap(); + + assert_eq!( + casted.as_decimal().decimal_value(), + Some(DecimalValue::I128(4200)) + ); +} + +#[test] +fn test_primitive_to_decimal_precision_checked() { + let scalar = Scalar::primitive(1000i32, Nullability::NonNullable); + + let result = scalar.cast(&DType::Decimal( + DecimalDType::new(2, 0), + Nullability::NonNullable, + )); + + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("does not fit in precision") ); } diff --git a/vortex-array/src/scalar/typed_view/primitive/scalar.rs b/vortex-array/src/scalar/typed_view/primitive/scalar.rs index e3981de8dd3..3ca4d337a45 100644 --- a/vortex-array/src/scalar/typed_view/primitive/scalar.rs +++ b/vortex-array/src/scalar/typed_view/primitive/scalar.rs @@ -18,15 +18,19 @@ use num_traits::CheckedSub; use vortex_error::VortexError; use vortex_error::VortexExpect; use vortex_error::VortexResult; +use vortex_error::vortex_bail; use vortex_error::vortex_ensure; use vortex_error::vortex_panic; use super::pvalue::CoercePValue; use crate::dtype::DType; +use crate::dtype::DecimalDType; use crate::dtype::FromPrimitiveOrF16; use crate::dtype::NativePType; use crate::dtype::PType; +use crate::dtype::i256; use crate::match_each_native_ptype; +use crate::scalar::DecimalValue; use crate::scalar::NumericOperator; use crate::scalar::PValue; use crate::scalar::Scalar; @@ -164,13 +168,21 @@ impl<'a> PrimitiveScalar<'a> { /// Casts this scalar to the given `dtype`. pub(crate) fn cast(&self, dtype: &DType) -> VortexResult { - let ptype = PType::try_from(dtype)?; let pvalue = self .pvalue .vortex_expect("nullness handled in Scalar::cast"); - Ok(match_each_native_ptype!(ptype, |Q| { - Scalar::primitive(pvalue.cast::()?, dtype.nullability()) - })) + + match dtype { + DType::Primitive(ptype, nullability) => Ok(match_each_native_ptype!(*ptype, |Q| { + Scalar::primitive(pvalue.cast::()?, *nullability) + })), + DType::Decimal(decimal_dtype, nullability) => Ok(Scalar::decimal( + pvalue_to_decimal(pvalue, *decimal_dtype)?, + *decimal_dtype, + *nullability, + )), + _ => vortex_bail!("Cannot cast primitive scalar to {dtype}"), + } } /// Returns true if the scalar is nan. @@ -267,6 +279,25 @@ impl<'a> PrimitiveScalar<'a> { } } +fn pvalue_to_decimal(pvalue: PValue, decimal_dtype: DecimalDType) -> VortexResult { + let value = match pvalue { + PValue::U8(v) => i256::from_i128(i128::from(v)), + PValue::U16(v) => i256::from_i128(i128::from(v)), + PValue::U32(v) => i256::from_i128(i128::from(v)), + PValue::U64(v) => i256::from_i128(i128::from(v)), + PValue::I8(v) => i256::from_i128(i128::from(v)), + PValue::I16(v) => i256::from_i128(i128::from(v)), + PValue::I32(v) => i256::from_i128(i128::from(v)), + PValue::I64(v) => i256::from_i128(i128::from(v)), + PValue::F16(_) | PValue::F32(_) | PValue::F64(_) => { + vortex_bail!("Cannot cast floating primitive {pvalue} to decimal {decimal_dtype}") + } + }; + + let scaled = DecimalValue::rescale_i256(value, 0, decimal_dtype.scale())?; + DecimalValue::try_from_i256(scaled, decimal_dtype) +} + impl Sub for PrimitiveScalar<'_> { type Output = Self; From 8948181751c858a9927fec4389ba6da267966616 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Tue, 23 Jun 2026 17:03:42 -0400 Subject: [PATCH 2/4] Vectorize decimal array casts Signed-off-by: "Nicholas Gates" --- .../src/arrays/decimal/compute/cast.rs | 213 +++++++++++++++--- 1 file changed, 180 insertions(+), 33 deletions(-) diff --git a/vortex-array/src/arrays/decimal/compute/cast.rs b/vortex-array/src/arrays/decimal/compute/cast.rs index 6c1ea5a1e4d..fda2a8a55aa 100644 --- a/vortex-array/src/arrays/decimal/compute/cast.rs +++ b/vortex-array/src/arrays/decimal/compute/cast.rs @@ -1,8 +1,11 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +use num_traits::CheckedMul; use vortex_buffer::Buffer; use vortex_buffer::BufferMut; +use vortex_compute::lane_kernels::IndexedSourceExt; +use vortex_error::VortexError; use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; @@ -16,10 +19,12 @@ use crate::IntoArray; use crate::array::ArrayView; use crate::arrays::Decimal; use crate::arrays::DecimalArray; +use crate::dtype::BigCast; use crate::dtype::DType; use crate::dtype::DecimalDType; use crate::dtype::DecimalType; use crate::dtype::NativeDecimalType; +use crate::dtype::i256; use crate::match_each_decimal_value_type; use crate::scalar::DecimalValue; use crate::scalar_fn::fns::cast::CastKernel; @@ -137,58 +142,170 @@ fn cast_decimal_values( ) -> VortexResult where F: NativeDecimalType, - T: NativeDecimalType, + T: NativeDecimalType + CheckedMul, DecimalValue: From, { let values = array.buffer::(); - let mut buffer = BufferMut::::zeroed(values.len()); + let values = values.as_slice(); + let cast_plan = DecimalCastPlan::::new(from_decimal_dtype, to_decimal_dtype); - match valid_values { + let buffer = match valid_values { Mask::AllTrue(_) => { - for (idx, value) in values.iter().copied().enumerate() { - buffer[idx] = - cast_decimal_value::(value, from_decimal_dtype, to_decimal_dtype)?; - } + let mut buffer = BufferMut::::with_capacity(values.len()); + values + .try_map_into(&mut buffer.spare_capacity_mut()[..values.len()], |value| { + cast_plan.cast(value) + }) + .map_err(|idx| { + decimal_cast_error::(values[idx], from_decimal_dtype, to_decimal_dtype) + })?; + // SAFETY: try_map_into initializes every lane before returning Ok. + unsafe { buffer.set_len(values.len()) }; + buffer.freeze() } - Mask::AllFalse(_) => {} + Mask::AllFalse(_) => BufferMut::::zeroed(values.len()).freeze(), Mask::Values(mask) => { - for (idx, (value, is_valid)) in values - .iter() - .copied() - .zip(mask.bit_buffer().iter()) - .enumerate() - { - if is_valid { - buffer[idx] = - cast_decimal_value::(value, from_decimal_dtype, to_decimal_dtype)?; - } - } + let mut buffer = BufferMut::::with_capacity(values.len()); + values + .try_map_masked_into( + mask.bit_buffer(), + &mut buffer.spare_capacity_mut()[..values.len()], + |value| cast_plan.cast(value), + ) + .map_err(|idx| { + decimal_cast_error::(values[idx], from_decimal_dtype, to_decimal_dtype) + })?; + // SAFETY: try_map_masked_into initializes every lane before returning Ok. + unsafe { buffer.set_len(values.len()) }; + buffer.freeze() } - } + }; - Ok(DecimalArray::new(buffer.freeze(), to_decimal_dtype, validity).into_array()) + Ok(DecimalArray::new(buffer, to_decimal_dtype, validity).into_array()) } -fn cast_decimal_value( +#[cold] +fn decimal_cast_error( value: F, from_decimal_dtype: DecimalDType, to_decimal_dtype: DecimalDType, -) -> VortexResult +) -> VortexError where F: NativeDecimalType, T: NativeDecimalType, DecimalValue: From, { - DecimalValue::from(value) - .cast_decimal(from_decimal_dtype, to_decimal_dtype)? - .cast::() - .ok_or_else(|| { - vortex_err!( - "decimal value cannot be represented as {} after casting to {}", - T::DECIMAL_TYPE, - to_decimal_dtype - ) - }) + match DecimalValue::from(value) + .cast_decimal(from_decimal_dtype, to_decimal_dtype) + .and_then(|value| { + value.cast::().ok_or_else(|| { + vortex_err!( + "decimal value cannot be represented as {} after casting to {}", + T::DECIMAL_TYPE, + to_decimal_dtype + ) + }) + }) { + Ok(_) => vortex_err!( + "decimal value cannot be represented as {} after casting from {} to {}", + T::DECIMAL_TYPE, + from_decimal_dtype, + to_decimal_dtype + ), + Err(error) => error, + } +} + +#[derive(Debug, Clone, Copy)] +enum DecimalCastPlan { + SameScale { min: T, max: T }, + ScaleUp { factor: T, min: T, max: T }, + ScaleUpOverflow, + ScaleDown { factor: i256, min: i256, max: i256 }, + ScaleDownOverflow, +} + +impl DecimalCastPlan +where + T: NativeDecimalType + CheckedMul, +{ + fn new(from_decimal_dtype: DecimalDType, to_decimal_dtype: DecimalDType) -> Self { + let scale_delta = to_decimal_dtype.scale() as i16 - from_decimal_dtype.scale() as i16; + if scale_delta == 0 { + let (min, max) = decimal_precision_range::(to_decimal_dtype); + return Self::SameScale { min, max }; + } + + if scale_delta > 0 { + let Some(factor) = decimal_scale_factor::(scale_delta as u32) else { + return Self::ScaleUpOverflow; + }; + let (min, max) = decimal_precision_range::(to_decimal_dtype); + return Self::ScaleUp { factor, min, max }; + } + + let Some(factor) = decimal_scale_factor::((-scale_delta) as u32) else { + return Self::ScaleDownOverflow; + }; + let (min, max) = decimal_precision_range::(to_decimal_dtype); + Self::ScaleDown { factor, min, max } + } + + #[inline] + fn cast(&self, value: F) -> Option + where + F: NativeDecimalType, + { + match *self { + DecimalCastPlan::SameScale { min, max } => { + let value = ::from(value)?; + (value >= min && value <= max).then_some(value) + } + DecimalCastPlan::ScaleUp { factor, min, max } => { + let value = ::from(value)?; + let value = value.checked_mul(&factor)?; + (value >= min && value <= max).then_some(value) + } + DecimalCastPlan::ScaleUpOverflow | DecimalCastPlan::ScaleDownOverflow => { + (value == F::default()).then_some(T::default()) + } + DecimalCastPlan::ScaleDown { factor, min, max } => { + let value = ::from(value)?; + if value == i256::ZERO { + return Some(T::default()); + } + if value % factor != i256::ZERO { + return None; + } + + let value = value / factor; + if value < min || value > max { + return None; + } + ::from(value) + } + } + } +} + +fn decimal_precision_range(decimal_dtype: DecimalDType) -> (T, T) { + let precision = usize::from(decimal_dtype.precision()); + ( + T::MIN_BY_PRECISION[precision], + T::MAX_BY_PRECISION[precision], + ) +} + +fn decimal_scale_factor(exp: u32) -> Option +where + T: NativeDecimalType + CheckedMul, +{ + let ten = ::from(10_i8)?; + let mut factor = ::from(1_i8)?; + for _ in 0..exp { + factor = factor.checked_mul(&ten)?; + } + Some(factor) } /// Upcast a DecimalArray to a wider physical representation (e.g., i32 -> i64) while keeping @@ -413,6 +530,36 @@ mod tests { ); } + #[test] + fn cast_lower_scale_ignores_null_lane_failures() { + let array = DecimalArray::new( + buffer![100i64, 123456], + DecimalDType::new(10, 4), + Validity::from_iter([true, false]), + ); + + let lower_scale_dtype = DType::Decimal(DecimalDType::new(3, 2), Nullability::Nullable); + #[expect(deprecated)] + let casted = array + .into_array() + .cast(lower_scale_dtype) + .unwrap() + .to_decimal(); + + let mask = casted + .as_ref() + .validity() + .unwrap() + .execute_mask( + casted.as_ref().len(), + &mut LEGACY_SESSION.create_execution_ctx(), + ) + .unwrap(); + assert!(mask.value(0)); + assert!(!mask.value(1)); + assert_eq!(casted.buffer::().as_ref()[0], 1); + } + #[test] fn cast_upcast_precision_succeeds() { let array = DecimalArray::new( From edc92c0db5d649b52ebe10e0819c33a5fa5b3f67 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Tue, 23 Jun 2026 17:33:42 -0400 Subject: [PATCH 3/4] DCO Remediation Commit for Nicholas Gates Use array_session in decimal cast tests so the PR merge ref matches develop's test-session import pattern. I, Nicholas Gates , hereby add my Signed-off-by to this commit: 25713a5c3f36cfb2247df6015d2620d71d34f9b0 I, Nicholas Gates , hereby add my Signed-off-by to this commit: 8948181751c858a9927fec4389ba6da267966616 Signed-off-by: Nicholas Gates --- vortex-array/src/arrays/decimal/compute/cast.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vortex-array/src/arrays/decimal/compute/cast.rs b/vortex-array/src/arrays/decimal/compute/cast.rs index fda2a8a55aa..e2d4703af93 100644 --- a/vortex-array/src/arrays/decimal/compute/cast.rs +++ b/vortex-array/src/arrays/decimal/compute/cast.rs @@ -367,8 +367,8 @@ mod tests { use super::upcast_decimal_values; use crate::IntoArray; - use crate::LEGACY_SESSION; use crate::VortexSessionExecute; + use crate::array_session; use crate::arrays::DecimalArray; use crate::builtins::ArrayBuiltins; #[expect(deprecated)] @@ -552,7 +552,7 @@ mod tests { .unwrap() .execute_mask( casted.as_ref().len(), - &mut LEGACY_SESSION.create_execution_ctx(), + &mut array_session().create_execution_ctx(), ) .unwrap(); assert!(mask.value(0)); @@ -689,7 +689,7 @@ mod tests { .unwrap() .execute_mask( casted.as_ref().len(), - &mut LEGACY_SESSION.create_execution_ctx(), + &mut array_session().create_execution_ctx(), ) .unwrap(); assert!(mask.value(0)); From e2178ef2ecc359b8abc904370482d3cb60dc1d32 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Tue, 23 Jun 2026 17:48:01 -0400 Subject: [PATCH 4/4] Keep precision-widening decimal casts zero-copy Generalize the decimal CastKernel buffer-reuse fast path: when the scale is unchanged, the target precision only widens, and the current physical value type already fits the target precision, reuse the values buffer instead of allocating and re-scanning every lane. This restores the zero-copy behavior the vectorized cast regressed for the common widening case (e.g. casting a Decimal(10,2) i64 array to Decimal(18,2)). Also assert (debug-only) that the vectorized fast path and the scalar slow path agree: decimal_cast_error's success arm is unreachable unless they drift. Add a regression test asserting the widening cast shares the source buffer. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Nicholas Gates --- .../src/arrays/decimal/compute/cast.rs | 63 ++++++++++++++++--- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/vortex-array/src/arrays/decimal/compute/cast.rs b/vortex-array/src/arrays/decimal/compute/cast.rs index e2d4703af93..80fafde5059 100644 --- a/vortex-array/src/arrays/decimal/compute/cast.rs +++ b/vortex-array/src/arrays/decimal/compute/cast.rs @@ -100,8 +100,18 @@ impl CastKernel for Decimal { .clone() .cast_nullability(*to_nullability, array.len(), ctx)?; - if from_decimal_dtype == to_decimal_dtype { - // SAFETY: new_validity has the same length, only its nullability tag changes. + // Reuse the values buffer untouched when no rescale is required, the target precision + // only widens (so every value still fits), and the current physical type is already wide + // enough to hold the target precision. This keeps the common precision-widening cast + // (and pure nullability changes) zero-copy instead of allocating and re-scanning. + if from_decimal_dtype.scale() == to_decimal_dtype.scale() + && to_decimal_dtype.precision() >= from_decimal_dtype.precision() + && array + .values_type() + .is_compatible_decimal_value_type(*to_decimal_dtype) + { + // SAFETY: the source values are bit-identical and remain in range for the wider + // precision, and new_validity has the same length, only its nullability tag changes. unsafe { return Ok(Some( DecimalArray::new_unchecked_handle( @@ -206,12 +216,22 @@ where ) }) }) { - Ok(_) => vortex_err!( - "decimal value cannot be represented as {} after casting from {} to {}", - T::DECIMAL_TYPE, - from_decimal_dtype, - to_decimal_dtype - ), + Ok(_) => { + // The fast path only returns `None` for values the slow path also rejects, so this + // arm should be unreachable. If it is hit, the fast and slow paths have drifted and + // we are erroring on a value that is actually representable. + debug_assert!( + false, + "decimal fast-path cast rejected value {value} that the slow path accepts \ + (from {from_decimal_dtype} to {to_decimal_dtype})" + ); + vortex_err!( + "decimal value cannot be represented as {} after casting from {} to {}", + T::DECIMAL_TYPE, + from_decimal_dtype, + to_decimal_dtype + ) + } Err(error) => error, } } @@ -580,6 +600,33 @@ mod tests { assert_eq!(casted.values_type(), DecimalType::I128); } + #[test] + fn cast_widening_same_physical_type_is_zero_copy() { + // Decimal(10,2) and Decimal(18,2) are both physically i64 with the same scale, so widening + // the precision must reuse the values buffer rather than allocate and re-scan it. + let array = DecimalArray::new( + buffer![100i64, 200, 300], + DecimalDType::new(10, 2), + Validity::NonNullable, + ); + let src_ptr = array.buffer::().as_ptr(); + + let wider_dtype = DType::Decimal(DecimalDType::new(18, 2), Nullability::NonNullable); + #[expect(deprecated)] + let casted = array.into_array().cast(wider_dtype).unwrap().to_decimal(); + + assert_eq!(casted.precision(), 18); + assert_eq!(casted.scale(), 2); + assert_eq!(casted.values_type(), DecimalType::I64); + assert_eq!(casted.buffer::().as_ref(), &[100, 200, 300]); + // The values buffer must be shared with the source (zero-copy), not reallocated. + assert_eq!( + casted.buffer::().as_ptr(), + src_ptr, + "precision-widening cast must reuse the source values buffer" + ); + } + #[test] fn cast_to_non_decimal_returns_err() { let array = DecimalArray::new(