From c66cf7c2c0925d99111b446616f3548dd0a71664 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 4 Nov 2026 11:38:17 -0500 Subject: [PATCH 1/2] Keep same-scale decimal widening casts lazy Return a Widen when a same-scale decimal cast increases the canonical width, so callers can defer the copy until execution. Preserve the source dtype, value buffer, and statistics on the child, and pass subsequent decimal Widen casts through to that child. Cover width changes, noncanonical storage, nullability, statistics, repeated casts, and the default writer round trip. Consolidate the older precision-growth and buffer-reuse tests into the parameterized coverage. Signed-off-by: Connor Tsui --- .../src/arrays/decimal/compute/cast.rs | 106 ++---- vortex-array/src/arrays/widen/compute/cast.rs | 79 +++- vortex-array/src/arrays/widen/mod.rs | 7 +- .../src/arrays/widen/tests/decimal_cast.rs | 350 ++++++++++++++++++ .../src/arrays/widen/tests/decimal_compare.rs | 2 + vortex-array/src/arrays/widen/tests/mod.rs | 1 + vortex-file/src/tests.rs | 30 ++ 7 files changed, 489 insertions(+), 86 deletions(-) create mode 100644 vortex-array/src/arrays/widen/tests/decimal_cast.rs diff --git a/vortex-array/src/arrays/decimal/compute/cast.rs b/vortex-array/src/arrays/decimal/compute/cast.rs index c8f561af0b7..fd55b107a6f 100644 --- a/vortex-array/src/arrays/decimal/compute/cast.rs +++ b/vortex-array/src/arrays/decimal/compute/cast.rs @@ -1,6 +1,11 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +//! Decimal casts with checked precision, scale, and nullability. +//! +//! Same-scale casts that increase the canonical width defer the copy through a [`WidenArray`]. +//! Scale changes and precision reductions run the conversion kernel to check or rescale valid values. + use num_traits::CheckedMul; use vortex_buffer::Buffer; use vortex_buffer::BufferMut; @@ -21,6 +26,7 @@ use crate::arrays::ConstantArray; use crate::arrays::Decimal; use crate::arrays::DecimalArray; use crate::arrays::PrimitiveArray; +use crate::arrays::WidenArray; use crate::arrays::decimal::DecimalArrayExt; use crate::dtype::BigCast; use crate::dtype::DType; @@ -44,8 +50,6 @@ use crate::validity::Validity; impl CastReduce for Decimal { fn cast(array: ArrayView<'_, Decimal>, dtype: &DType) -> VortexResult> { - // Only nullability changes within the same decimal dtype are reducible without execution. - // Precision/scale changes need the kernel. let DType::Decimal(to_decimal_dtype, to_nullability) = dtype else { return Ok(None); }; @@ -56,7 +60,8 @@ impl CastReduce for Decimal { ); }; - if from_decimal_dtype != to_decimal_dtype { + let grows_width = grows_canonical_width(*from_decimal_dtype, *to_decimal_dtype); + if from_decimal_dtype != to_decimal_dtype && !grows_width { return Ok(None); } @@ -67,6 +72,10 @@ impl CastReduce for Decimal { return Ok(None); }; + if grows_width { + return widen_decimal_values(array, dtype, new_validity).map(Some); + } + // SAFETY: validity has the same length, only its nullability tag changes. unsafe { Ok(Some( @@ -121,6 +130,10 @@ impl CastKernel for Decimal { .clone() .cast_nullability(*to_nullability, array.len(), ctx)?; + if grows_canonical_width(*from_decimal_dtype, *to_decimal_dtype) { + return widen_decimal_values(array, dtype, new_validity).map(Some); + } + // 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 @@ -169,6 +182,31 @@ impl CastKernel for Decimal { } } +/// Returns whether a same-scale cast increases the canonical width. +fn grows_canonical_width(from: DecimalDType, to: DecimalDType) -> bool { + from.scale() == to.scale() && from.canonical_values_type() < to.canonical_values_type() +} + +/// Wraps the source values in a [`WidenArray`] with the target dtype. +/// +/// The caller must cast `validity` to the target nullability without changing its length or valid +/// rows. The child keeps the source decimal dtype and statistics because its values are unchanged. +fn widen_decimal_values( + array: ArrayView<'_, Decimal>, + dtype: &DType, + validity: Validity, +) -> VortexResult { + let values = DecimalArray::new_handle( + array.buffer_handle().clone(), + array.values_type(), + array.decimal_dtype(), + validity, + ); + values.statistics().inherit_from(array.statistics()); + + Ok(WidenArray::try_new(values.into_array(), dtype.clone())?.into_array()) +} + fn cast_to_integer( array: ArrayView<'_, Decimal>, nullability: Nullability, @@ -966,68 +1004,6 @@ mod tests { assert_eq!(casted.buffer::().as_ref()[0], 1); } - #[test] - fn cast_upcast_precision_succeeds() { - let mut ctx = SESSION.create_execution_ctx(); - let array = DecimalArray::new( - buffer![100i32, 200, 300], - DecimalDType::new(10, 2), - Validity::NonNullable, - ); - - // Cast to higher precision with same scale - should succeed - let wider_dtype = DType::Decimal(DecimalDType::new(38, 2), Nullability::NonNullable); - let casted = array - .into_array() - .cast(wider_dtype) - .unwrap() - .execute::(&mut ctx) - .unwrap(); - - assert_eq!(casted.precision(), 38); - assert_eq!(casted.scale(), 2); - assert_eq!(casted.len(), 3); - // Should be stored in i128 now (precision 38 requires i128) - assert_eq!(casted.values_type(), DecimalType::I128); - assert_arrays_eq!( - casted, - DecimalArray::from_iter([100i128, 200, 300], DecimalDType::new(38, 2)), - &mut ctx - ); - } - - #[test] - fn cast_widening_same_physical_type_is_zero_copy() { - let mut ctx = SESSION.create_execution_ctx(); - // 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); - let casted = array - .into_array() - .cast(wider_dtype) - .unwrap() - .execute::(&mut ctx) - .unwrap(); - - 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 mut ctx = SESSION.create_execution_ctx(); diff --git a/vortex-array/src/arrays/widen/compute/cast.rs b/vortex-array/src/arrays/widen/compute/cast.rs index 6f7131b2f90..940c57c2bf5 100644 --- a/vortex-array/src/arrays/widen/compute/cast.rs +++ b/vortex-array/src/arrays/widen/compute/cast.rs @@ -1,6 +1,12 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors +//! Cast reductions for decimal and primitive [`Widen`] arrays. +//! +//! Same-scale decimal casts retarget the `Widen` when the child already fits the target. Primitive +//! casts can retain a `Widen` for wider integer targets, after matching the child's signedness to the +//! target. Both paths leave required conversions and nullability checks to the child's cast. + use vortex_error::VortexResult; use crate::ArrayRef; @@ -10,33 +16,70 @@ use crate::arrays::widen::WidenArraySlotsExt; use crate::arrays::widen::compute::wrap_values; use crate::builtins::ArrayBuiltins; use crate::dtype::DType; +use crate::dtype::DecimalDType; use crate::dtype::PType; use crate::scalar_fn::fns::cast::CastReduce; impl CastReduce for Widen { fn cast(array: ArrayView<'_, Self>, dtype: &DType) -> VortexResult> { let values = array.values(); - let (DType::Primitive(target_ptype, nullability), DType::Primitive(values_ptype, _)) = - (dtype, values.dtype()) - else { - return Ok(None); - }; - - let cast_ptype = match values_ptype_for_cast(*values_ptype, *target_ptype) { - Some(cast_ptype) if target_ptype.byte_width() > cast_ptype.byte_width() => cast_ptype, - // A float target, or an integer target no wider than `cast_ptype`, goes to the values, - // whose cast checks every value and every null. - _ => return values.cast(dtype.clone()).map(Some), - }; - - // A wider integer target holds every value of `cast_ptype`, so the values stay narrow. - // They take the target nullability, so that a cast to a non-nullable target still fails on - // a null. - let values = values.cast(DType::Primitive(cast_ptype, *nullability))?; - wrap_values(values, dtype.clone()).map(Some) + + match values.dtype() { + DType::Decimal(values_dtype, _) => cast_decimal(values, *values_dtype, dtype), + DType::Primitive(values_ptype, _) => cast_primitive(values, *values_ptype, dtype), + _ => Ok(None), + } + } +} + +fn cast_decimal( + values: &ArrayRef, + values_dtype: DecimalDType, + dtype: &DType, +) -> VortexResult> { + if let DType::Decimal(target_dtype, _) = dtype + && values_dtype.scale() == target_dtype.scale() + && values_dtype.canonical_values_type() < target_dtype.canonical_values_type() + { + // The child's declared precision proves that every value fits the target. Only nullability + // may still need a check, so leave the width conversion to Widen execution. + let values = values.cast(values.dtype().with_nullability(dtype.nullability()))?; + + return wrap_values(values, dtype.clone()).map(Some); + } + + // Rescaling, narrower targets, and primitive conversions still need the child's cast. The child + // has the parent's values, scale, and nulls, so these conversions preserve the logical result. + match dtype { + DType::Decimal(..) | DType::Primitive(..) => values.cast(dtype.clone()).map(Some), + _ => Ok(None), } } +fn cast_primitive( + values: &ArrayRef, + values_ptype: PType, + dtype: &DType, +) -> VortexResult> { + let DType::Primitive(target_ptype, nullability) = dtype else { + return Ok(None); + }; + + let cast_ptype = match values_ptype_for_cast(values_ptype, *target_ptype) { + Some(cast_ptype) if target_ptype.byte_width() > cast_ptype.byte_width() => cast_ptype, + // A float needs value conversion, and an integer target no wider than the converted child + // cannot retain a Widen. Casting directly also checks overflow and nullability. + _ => return values.cast(dtype.clone()).map(Some), + }; + + // Match the child's signedness and nullability to the target. If both already match, the cast + // returns the original child. Otherwise, it converts the child and checks negative values and + // nulls as needed. Widen handles the remaining width increase because every child value fits. + let values = values.cast(DType::Primitive(cast_ptype, *nullability))?; + + wrap_values(values, dtype.clone()).map(Some) +} + /// Returns the type that the values take when the `Widen` is cast to the integer type `target`, or /// `None` for a float target. /// diff --git a/vortex-array/src/arrays/widen/mod.rs b/vortex-array/src/arrays/widen/mod.rs index 72ba52b208a..7d8284d2c26 100644 --- a/vortex-array/src/arrays/widen/mod.rs +++ b/vortex-array/src/arrays/widen/mod.rs @@ -11,10 +11,11 @@ //! [`WidenArray::encode`] picks the narrowest such type for a [`PrimitiveArray`]. //! //! A decimal child has the logical scale and a lower precision, and **must** be valid for its dtype. -//! Comparisons, bounds checks and fills use the child dtype to bound its valid values. +//! Comparisons, bounds checks, and fills use the child dtype to bound its valid values. //! [`WidenArray::encode_decimal`] and [`WidenArray::from_stored_decimal`] choose child precision -//! from valid values or supplied bounds, at the logical scale. Decimal execution copies the child -//! values into the logical canonical width. +//! from valid values or supplied bounds, at the logical scale. A widening decimal cast keeps the +//! source dtype on its child. Decimal execution copies child values directly into the logical +//! canonical width, since yielding a cast would produce another `Widen` instead of canonical values. //! //! Narrowing is a physical choice, so it **must not** change a result. Execution casts the values //! to the logical dtype for integers, and an operation runs on the values only where it gives the diff --git a/vortex-array/src/arrays/widen/tests/decimal_cast.rs b/vortex-array/src/arrays/widen/tests/decimal_cast.rs new file mode 100644 index 00000000000..e3dac36abbd --- /dev/null +++ b/vortex-array/src/arrays/widen/tests/decimal_cast.rs @@ -0,0 +1,350 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +//! Decimal casts preserve lazy widening, checked precision, and nullability. +//! +//! These cases also cover decimal buffers whose stored width differs from their canonical width. + +use rstest::rstest; +use vortex_buffer::Buffer; +use vortex_buffer::buffer; +use vortex_error::VortexExpect; +use vortex_error::VortexResult; + +use super::SESSION; +use super::canonical; +use crate::ArrayRef; +use crate::IntoArray; +use crate::VortexSessionExecute; +use crate::arrays::BoolArray; +use crate::arrays::Decimal; +use crate::arrays::DecimalArray; +use crate::arrays::DictArray; +use crate::arrays::Widen; +use crate::arrays::WidenArray; +use crate::arrays::decimal::DecimalArrayExt; +use crate::arrays::widen::WidenArraySlotsExt; +use crate::assert_arrays_eq; +use crate::builtins::ArrayBuiltins; +use crate::dtype::DType; +use crate::dtype::DecimalDType; +use crate::dtype::Nullability; +use crate::dtype::PType; +use crate::dtype::i256; +use crate::expr::stats::Precision; +use crate::expr::stats::Stat; +use crate::expr::stats::StatsProvider; +use crate::scalar::DecimalValue; +use crate::scalar::Scalar; +use crate::scalar_fn::fns::cast::CastKernel; +use crate::scalar_fn::fns::cast::CastReduce; +use crate::validity::Validity; + +#[rstest] +#[case::i8(DecimalArray::from_iter([12i8, -12], DecimalDType::new(2, 0)), 4, vec![12, -12])] +#[case::i16(DecimalArray::from_iter([12i16, -12], DecimalDType::new(4, 0)), 9, vec![12, -12])] +#[case::i32(DecimalArray::from_iter([12i32, -12], DecimalDType::new(9, 0)), 18, vec![12, -12])] +#[case::i64(DecimalArray::from_iter([12i64, -12], DecimalDType::new(18, 2)), 38, vec![12, -12])] +#[case::i128(DecimalArray::from_iter([12i128, -12], DecimalDType::new(38, 0)), 76, vec![12, -12])] +#[case::stored_target_width( + DecimalArray::from_iter([12i16, -12], DecimalDType::new(2, 0)), + 4, + vec![12, -12], +)] +#[case::stored_narrower_than_source( + DecimalArray::from_iter([12i32, -12], DecimalDType::new(10, 2)), + 38, + vec![12, -12], +)] +#[case::stored_i256( + DecimalArray::from_iter( + [i256::from_i128(12), i256::from_i128(-12)], + DecimalDType::new(2, 0), + ), + 4, + vec![12, -12], +)] +#[case::empty(DecimalArray::from_iter(Vec::::new(), DecimalDType::new(2, 0)), 4, vec![])] +#[case::all_null( + DecimalArray::new(buffer![12i8, -12], DecimalDType::new(2, 0), Validity::AllInvalid), + 4, + vec![0, 0], +)] +fn test_precision_growth_is_lazy( + #[case] array: DecimalArray, + #[case] precision: u8, + #[case] expected: Vec, +) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let source_dtype = array.dtype().clone(); + let source_values_type = array.values_type(); + let source_ptr = array.buffer_handle().as_host().as_ptr(); + let validity = array.validity()?; + let decimal_dtype = DecimalDType::new(precision, array.decimal_dtype().scale()); + let target = DType::Decimal(decimal_dtype, validity.nullability()); + + let casted = array.into_array().cast(target)?; + + assert!(casted.is::()); + let widen = casted.as_::(); + let values = widen.values(); + assert_eq!(values.dtype(), &source_dtype); + assert_eq!(values.as_::().values_type(), source_values_type); + assert_eq!( + values.as_::().buffer_handle().as_host().as_ptr(), + source_ptr + ); + + let executed = casted.execute::(&mut ctx)?; + assert_eq!( + executed.values_type(), + decimal_dtype.canonical_values_type() + ); + let expected = DecimalArray::new(Buffer::from_iter(expected), decimal_dtype, validity); + assert_arrays_eq!(executed, expected, &mut ctx); + + Ok(()) +} + +#[rstest] +#[case::to_nullable(Validity::NonNullable, Nullability::Nullable)] +#[case::to_non_nullable(Validity::AllValid, Nullability::NonNullable)] +fn test_precision_growth_sets_child_nullability( + #[case] validity: Validity, + #[case] nullability: Nullability, +) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let array = DecimalArray::new(buffer![12i8, -12], DecimalDType::new(2, 0), validity); + let target_decimal = DecimalDType::new(4, 0); + let target = DType::Decimal(target_decimal, nullability); + + let casted = array.into_array().cast(target)?; + + assert!(casted.is::()); + assert_eq!( + casted.as_::().values().dtype().nullability(), + nullability + ); + let expected_validity = match nullability { + Nullability::Nullable => Validity::AllValid, + Nullability::NonNullable => Validity::NonNullable, + }; + let expected = DecimalArray::new(buffer![12i16, -12], target_decimal, expected_validity); + assert_arrays_eq!(casted, expected, &mut ctx); + + Ok(()) +} + +/// The child holds the same values as the source, so both cast paths keep its exact extrema, even +/// when the cast changes the nullability. +#[rstest] +#[case::reduce(false, Nullability::NonNullable)] +#[case::kernel_to_nullable(true, Nullability::Nullable)] +fn test_precision_growth_keeps_values_statistics( + #[case] through_kernel: bool, + #[case] nullability: Nullability, +) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let source_decimal = DecimalDType::new(4, 0); + let extremum = |value: i16, nullability| { + Scalar::decimal(DecimalValue::I16(value), source_decimal, nullability) + }; + let array = DecimalArray::from_iter([12i16, -12], source_decimal); + + for (stat, value) in [(Stat::Min, -12), (Stat::Max, 12)] { + let value = extremum(value, Nullability::NonNullable).into_value(); + let value = value.vortex_expect("the planted extremum is not null"); + array.statistics().set(stat, Precision::Exact(value)); + } + + let target = DType::Decimal(DecimalDType::new(18, 0), nullability); + let casted = if through_kernel { + ::cast(array.as_view(), &target, &mut ctx)? + } else { + ::cast(array.as_view(), &target)? + }; + let casted = casted.vortex_expect("same-scale precision growth casts"); + + assert!(casted.is::()); + let values = casted.as_::().values().clone(); + let statistics = values.statistics(); + let min = statistics.get(Stat::Min); + let max = statistics.get(Stat::Max); + assert_eq!(min, Precision::Exact(extremum(-12, nullability))); + assert_eq!(max, Precision::Exact(extremum(12, nullability))); + + Ok(()) +} + +#[rstest] +#[case::all_valid(true)] +#[case::null_row(false)] +fn test_encoded_validity_falls_back_to_kernel(#[case] second_valid: bool) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let validity = DictArray::try_new( + buffer![0u8, 1].into_array(), + BoolArray::from_iter([true, second_valid]).into_array(), + )?; + let array = DecimalArray::new( + buffer![12i8, -12], + DecimalDType::new(2, 0), + Validity::Array(validity.into_array()), + ); + let target_decimal = DecimalDType::new(4, 0); + let target = DType::Decimal(target_decimal, Nullability::NonNullable); + + assert!(::cast(array.as_view(), &target)?.is_none()); + let result = ::cast(array.as_view(), &target, &mut ctx); + if !second_valid { + let error = result.expect_err("the encoded validity contains a null row"); + assert!(error.to_string().contains("invalid values to non-nullable")); + + return Ok(()); + } + + let casted = result?.vortex_expect("same-scale precision growth has a cast kernel"); + assert!(casted.is::()); + assert_eq!( + casted.as_::().values().dtype().nullability(), + Nullability::NonNullable + ); + let expected = DecimalArray::from_iter([12i16, -12], target_decimal); + assert_arrays_eq!(casted, expected, &mut ctx); + + Ok(()) +} + +#[rstest] +#[case::narrower_than_parent(9)] +#[case::wider_than_parent(38)] +fn test_decimal_widen_retargets_child(#[case] precision: u8) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let values = DictArray::try_new( + buffer![0u8, 1, 0].into_array(), + DecimalArray::from_option_iter([Some(125i16), None], DecimalDType::new(4, 2)).into_array(), + )? + .into_array(); + let array = WidenArray::try_new( + values.clone(), + DType::Decimal(DecimalDType::new(18, 2), Nullability::Nullable), + )? + .into_array(); + let target_decimal = DecimalDType::new(precision, 2); + let target = DType::Decimal(target_decimal, Nullability::Nullable); + + let casted = array.cast(target)?; + + assert!(casted.is::()); + assert!(ArrayRef::ptr_eq(casted.as_::().values(), &values)); + let expected = DecimalArray::from_option_iter([Some(125i128), None, Some(125)], target_decimal); + assert_arrays_eq!(casted, expected, &mut ctx); + + Ok(()) +} + +#[rstest] +#[case::smaller_decimal(DType::Decimal(DecimalDType::new(3, 2), Nullability::Nullable))] +#[case::rescale(DType::Decimal(DecimalDType::new(18, 3), Nullability::Nullable))] +#[case::integer(DType::Primitive(PType::I64, Nullability::Nullable))] +#[case::float(DType::Primitive(PType::F64, Nullability::Nullable))] +fn test_decimal_widen_cast_matches_canonical(#[case] target: DType) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let values = + DecimalArray::from_option_iter([Some(125i16), None, Some(-250)], DecimalDType::new(4, 2)); + let array = WidenArray::try_new( + values.into_array(), + DType::Decimal(DecimalDType::new(18, 2), Nullability::Nullable), + )? + .into_array(); + let expected = canonical(&array, &mut ctx)?.cast(target.clone())?; + + let casted = array.cast(target)?; + + assert_arrays_eq!(casted, expected, &mut ctx); + + Ok(()) +} + +#[test] +fn test_precision_narrowing_checks_values() -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let values = DecimalArray::from_iter([1000i16], DecimalDType::new(4, 0)); + let array = WidenArray::try_new( + values.into_array(), + DType::Decimal(DecimalDType::new(18, 0), Nullability::NonNullable), + )? + .into_array(); + let target = DType::Decimal(DecimalDType::new(3, 0), Nullability::NonNullable); + + let error = array + .cast(target)? + .execute::(&mut ctx) + .expect_err("1000 does not fit a precision of three digits"); + + assert!(error.to_string().contains("does not fit in precision")); + + Ok(()) +} + +#[test] +fn test_repeated_casts_terminate_without_nested_widen() -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let source = DecimalArray::from_iter([12i8, -12], DecimalDType::new(2, 0)).into_array(); + let mut array = source.clone(); + + for precision in [4, 9, 18, 38, 76, 4, 2] { + let decimal_dtype = DecimalDType::new(precision, 0); + let target = DType::Decimal(decimal_dtype, Nullability::NonNullable); + array = array.cast(target)?; + if let Some(widen) = array.as_opt::() { + assert!(!widen.values().is::()); + assert_eq!(widen.values().dtype(), source.dtype()); + } + + assert_arrays_eq!( + array, + DecimalArray::from_iter([12i128, -12], decimal_dtype), + &mut ctx + ); + } + + Ok(()) +} + +#[rstest] +#[case::reuses_canonical_storage( + DecimalArray::from_iter([12i64, -12], DecimalDType::new(10, 2)), + DecimalArray::from_iter([12i64, -12], DecimalDType::new(18, 2)), + true, +)] +#[case::reuses_wider_storage( + DecimalArray::from_iter([12i128, -12], DecimalDType::new(10, 0)), + DecimalArray::from_iter([12i128, -12], DecimalDType::new(18, 0)), + true, +)] +#[case::copies_narrow_storage( + DecimalArray::from_iter([125i32, -250], DecimalDType::new(10, 2)), + DecimalArray::from_iter([125i64, -250], DecimalDType::new(18, 2)), + false, +)] +fn test_same_canonical_width( + #[case] array: DecimalArray, + #[case] expected: DecimalArray, + #[case] reuses_buffer: bool, +) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let source_ptr = array.buffer_handle().as_host().as_ptr(); + let casted = array.into_array().cast(expected.dtype().clone())?; + assert!(!casted.is::()); + + let executed = casted.execute::(&mut ctx)?; + + assert_eq!(executed.values_type(), expected.values_type()); + assert_eq!( + executed.buffer_handle().as_host().as_ptr() == source_ptr, + reuses_buffer + ); + assert_arrays_eq!(executed, expected, &mut ctx); + + Ok(()) +} diff --git a/vortex-array/src/arrays/widen/tests/decimal_compare.rs b/vortex-array/src/arrays/widen/tests/decimal_compare.rs index cd71ebc1851..1b0625bb530 100644 --- a/vortex-array/src/arrays/widen/tests/decimal_compare.rs +++ b/vortex-array/src/arrays/widen/tests/decimal_compare.rs @@ -137,6 +137,8 @@ fn test_compare_constant( assert_compare_matches_canonical(&array, &constant, operator, &mut ctx) } +/// In `different_widths`, the cast of the `i8` child to the common precision returns a `Widen` of +/// its own, so the compare runs through the fallback. #[rstest] #[case::same_width( DecimalArray::from_iter( diff --git a/vortex-array/src/arrays/widen/tests/mod.rs b/vortex-array/src/arrays/widen/tests/mod.rs index 57c8b91508d..94299d64f52 100644 --- a/vortex-array/src/arrays/widen/tests/mod.rs +++ b/vortex-array/src/arrays/widen/tests/mod.rs @@ -11,6 +11,7 @@ mod aggregates; mod compute; mod decimal; mod decimal_aggregates; +mod decimal_cast; mod decimal_compare; mod decimal_fallback; mod decimal_stored; diff --git a/vortex-file/src/tests.rs b/vortex-file/src/tests.rs index c015c706b7d..1dcf89a916e 100644 --- a/vortex-file/src/tests.rs +++ b/vortex-file/src/tests.rs @@ -35,6 +35,7 @@ use vortex_array::arrays::StructArray; use vortex_array::arrays::TemporalArray; use vortex_array::arrays::VarBinArray; use vortex_array::arrays::VarBinViewArray; +use vortex_array::arrays::WidenArray; use vortex_array::arrays::dict::DictArraySlotsExt; use vortex_array::arrays::struct_::StructArrayExt; use vortex_array::assert_arrays_eq; @@ -259,6 +260,35 @@ async fn test_read_simple() { assert_arrays_eq!(result, st, &mut SESSION.create_execution_ctx()); } +#[rstest] +#[case::non_nullable(Validity::NonNullable)] +#[case::partially_valid(Validity::from_iter([true, false, true, true, false]))] +#[tokio::test] +#[cfg_attr(miri, ignore)] +async fn test_decimal_widen_default_writer_roundtrip( + #[case] validity: Validity, +) -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let decimal_dtype = DecimalDType::new(18, 2); + let values = DecimalArray::new( + buffer![-125i16, 50, 0, 250, 900], + DecimalDType::new(4, 2), + validity.clone(), + ); + let dtype = DType::Decimal(decimal_dtype, validity.nullability()); + let widened = WidenArray::try_new(values.into_array(), dtype)?.into_array(); + let array = StructArray::from_fields(&[("decimal", widened)])?.into_array(); + let expected_values = + DecimalArray::new(buffer![-125i64, 50, 0, 250, 900], decimal_dtype, validity); + let expected = StructArray::from_fields(&[("decimal", expected_values.into_array())])?; + + let actual = write_read_roundtrip(array).await?; + + assert_arrays_eq!(actual, expected, &mut ctx); + + Ok(()) +} + /// Wide decimals split into multi-part arrays only when the writer allows the v2 format. The /// default writer allows every registered encoding once editions are disabled; a strategy built /// from the session keeps the enabled editions' restrictions regardless. From 9b68f5ab1c106fbbd638479c197d1fb552ead5c2 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Wed, 4 Nov 2026 11:38:26 -0500 Subject: [PATCH 2/2] Measure lazy decimal casts separately from execution Keep canonical and Widen inputs for lazy construction, and one end-to-end canonical cast for execution into i256 values. Avoid a second end-to-end case that overlaps the construction and execution coverage. Update the CodSpeed target count to match the nine cases. Signed-off-by: Connor Tsui --- scripts/codspeed_bench_counts.json | 2 +- vortex-array/benches/widen_decimal.rs | 44 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/scripts/codspeed_bench_counts.json b/scripts/codspeed_bench_counts.json index 87ccb9e575b..2590e974a5d 100644 --- a/scripts/codspeed_bench_counts.json +++ b/scripts/codspeed_bench_counts.json @@ -75,7 +75,7 @@ "varbinview_try_new": 6, "varbinview_zip": 2, "widen": 17, - "widen_decimal": 6 + "widen_decimal": 9 }, "vortex-buffer": { "allocation": 35, diff --git a/vortex-array/benches/widen_decimal.rs b/vortex-array/benches/widen_decimal.rs index 8ccb87094da..036664926f4 100644 --- a/vortex-array/benches/widen_decimal.rs +++ b/vortex-array/benches/widen_decimal.rs @@ -8,6 +8,9 @@ //! Canonical execution is omitted because it only returns the existing array. Each iteration shares //! value buffers and builds fresh arrays so cached statistics cannot remove the operation's work. //! [`WidenArray::encode_decimal`] selects the child buffers outside the timed region. +//! +//! Casts from precision 38 to 76 measure lazy construction for canonical and `Widen` inputs. +//! A canonical-input cast also measures execution through the copy into `i256` values. use std::fmt; use std::sync::LazyLock; @@ -28,6 +31,7 @@ use vortex_array::arrays::DecimalArray; use vortex_array::arrays::Widen; use vortex_array::arrays::WidenArray; use vortex_array::arrays::widen::WidenArraySlotsExt; +use vortex_array::builtins::ArrayBuiltins; use vortex_array::dtype::DType; use vortex_array::dtype::DecimalDType; use vortex_array::dtype::DecimalType; @@ -124,6 +128,13 @@ fn widen_cases() -> Vec { .collect() } +fn cast_cases() -> Vec { + cases() + .into_iter() + .filter(|case| case.precision == 38) + .collect() +} + impl fmt::Display for Case { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( @@ -260,3 +271,36 @@ fn execute(bencher: Bencher, case: Case) { |array, ctx| execute_canonical(Ok(array), ctx), ); } + +#[divan::bench(args = cast_cases())] +fn cast_lazy(bencher: Bencher, case: Case) { + let values = case.values(); + let target = DType::Decimal(DecimalDType::new(76, SCALE), Nullability::NonNullable); + + bench_case( + bencher, + || values.array(case), + |array, _ctx| { + array + .cast(target.clone()) + .vortex_expect("a same-scale precision increase is valid") + }, + ); +} + +#[divan::bench] +fn cast_execute(bencher: Bencher) { + let case = Case { + precision: 38, + child_precision: 18, + input: Input::Canonical, + }; + let values = case.values(); + let target = DType::Decimal(DecimalDType::new(76, SCALE), Nullability::NonNullable); + + bench_case( + bencher, + || values.array(case), + |array, ctx| execute_canonical(array.cast(target.clone()), ctx), + ); +}