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), + ); +} 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.