From 8cb9bd32be441dee92e9ffd3be50d4f8d399fde3 Mon Sep 17 00:00:00 2001 From: Vincent <1729065730@qq.com> Date: Sat, 24 Oct 2026 11:04:35 +0800 Subject: [PATCH 1/3] Return an error for decimal scalars outside the precision DecimalArray::scalar_at and the decimal min_max aggregate built their results with Scalar::decimal, which panics when a stored value does not fit the precision of the dtype. DecimalArray construction does not check its values and file reads trust the stored precision, so such a value could make a read panic. Add Scalar::try_decimal and use it in both places so they return an error instead. Signed-off-by: Vincent <1729065730@qq.com> --- .../src/aggregate_fn/fns/min_max/decimal.rs | 18 ++++++++------- .../src/aggregate_fn/fns/min_max/mod.rs | 17 ++++++++++++++ .../src/arrays/decimal/vtable/operations.rs | 23 ++++++++++++++++--- vortex-array/src/scalar/constructor.rs | 21 ++++++++++++++++- 4 files changed, 67 insertions(+), 12 deletions(-) diff --git a/vortex-array/src/aggregate_fn/fns/min_max/decimal.rs b/vortex-array/src/aggregate_fn/fns/min_max/decimal.rs index 1d6d0d81ca9..ea26b7e65a1 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/decimal.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/decimal.rs @@ -44,7 +44,7 @@ where .validity()? .execute_mask(array.as_ref().len(), ctx)? { - Mask::AllTrue(_) => compute_min_max(array.buffer::().iter(), array.decimal_dtype()), + Mask::AllTrue(_) => compute_min_max(array.buffer::().iter(), array.decimal_dtype())?, Mask::AllFalse(_) => None, Mask::Values(v) => compute_min_max( array @@ -53,7 +53,7 @@ where .zip(v.bit_buffer().iter()) .filter_map(|(v, m)| m.then_some(v)), array.decimal_dtype(), - ), + )?, }, ) } @@ -61,22 +61,24 @@ where fn compute_min_max<'a, T>( iter: impl Iterator, decimal_dtype: DecimalDType, -) -> Option +) -> VortexResult> where T: Into + NativeDecimalType + Ord + Copy + 'a, { - match iter.minmax_by(|a, b| a.cmp(b)) { + // The stored values are not guaranteed to fit the precision (for example, when read from a + // file), so return an error instead of panicking. + Ok(match iter.minmax_by(|a, b| a.cmp(b)) { itertools::MinMaxResult::NoElements => None, itertools::MinMaxResult::OneElement(&x) => { - let scalar = Scalar::decimal(x.into(), decimal_dtype, NonNullable); + let scalar = Scalar::try_decimal(x.into(), decimal_dtype, NonNullable)?; Some(MinMaxResult { min: scalar.clone(), max: scalar, }) } itertools::MinMaxResult::MinMax(&min, &max) => Some(MinMaxResult { - min: Scalar::decimal(min.into(), decimal_dtype, NonNullable), - max: Scalar::decimal(max.into(), decimal_dtype, NonNullable), + min: Scalar::try_decimal(min.into(), decimal_dtype, NonNullable)?, + max: Scalar::try_decimal(max.into(), decimal_dtype, NonNullable)?, }), - } + }) } diff --git a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs index 8aa0e559e84..af66cc00022 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs @@ -969,6 +969,23 @@ mod tests { Ok(()) } + #[test] + fn test_decimal_value_outside_precision_errors() { + // 100 does not fit in decimal(2, 0), and `DecimalArray::new` does not check the values. + for values in [buffer![100i8], buffer![1i8, 100i8]] { + let decimal = DecimalArray::new(values, DecimalDType::new(2, 0), Validity::NonNullable); + let mut ctx = SESSION.create_execution_ctx(); + assert!( + min_max( + &decimal.into_array(), + &mut ctx, + NumericalAggregateOpts::default(), + ) + .is_err() + ); + } + } + #[test] fn list_and_fixed_size_list_return_dtype() { let element_dtype = DType::Primitive(PType::I32, Nullability::Nullable); diff --git a/vortex-array/src/arrays/decimal/vtable/operations.rs b/vortex-array/src/arrays/decimal/vtable/operations.rs index 09d1d31a29c..1b6ffb0b340 100644 --- a/vortex-array/src/arrays/decimal/vtable/operations.rs +++ b/vortex-array/src/arrays/decimal/vtable/operations.rs @@ -19,13 +19,15 @@ impl OperationsVTable for Decimal { index: usize, _ctx: &mut ExecutionCtx, ) -> VortexResult { - Ok(match_each_decimal_value_type!(array.values_type(), |D| { - Scalar::decimal( + // The stored values are not guaranteed to fit the precision (for example, when read from a + // file), so return an error instead of panicking. + match_each_decimal_value_type!(array.values_type(), |D| { + Scalar::try_decimal( DecimalValue::from(array.buffer::()[index]), array.decimal_dtype(), array.dtype().nullability(), ) - })) + }) } } @@ -102,4 +104,19 @@ mod tests { ) ); } + + #[test] + fn test_scalar_at_value_outside_precision_errors() { + let array = DecimalArray::new( + buffer![100i8], + DecimalDType::new(2, 0), + Validity::NonNullable, + ); + + assert!( + array + .execute_scalar(0, &mut SESSION.create_execution_ctx()) + .is_err() + ); + } } diff --git a/vortex-array/src/scalar/constructor.rs b/vortex-array/src/scalar/constructor.rs index 81f72054696..c827b9f666e 100644 --- a/vortex-array/src/scalar/constructor.rs +++ b/vortex-array/src/scalar/constructor.rs @@ -65,16 +65,35 @@ impl Scalar { } /// Creates a new decimal scalar with the given value, precision, scale, and nullability. + /// + /// # Panics + /// + /// Panics if `value` does not fit in the precision of `decimal_type`. Use + /// [`Scalar::try_decimal`] when the value comes from data that has not been validated. pub fn decimal( value: DecimalValue, decimal_type: DecimalDType, nullability: Nullability, ) -> Self { + Self::try_decimal(value, decimal_type, nullability) + .vortex_expect("unable to construct a decimal `Scalar`") + } + + /// Attempts to create a new decimal scalar with the given value, precision, scale, and + /// nullability. + /// + /// # Errors + /// + /// Returns an error if `value` does not fit in the precision of `decimal_type`. + pub fn try_decimal( + value: DecimalValue, + decimal_type: DecimalDType, + nullability: Nullability, + ) -> VortexResult { Self::try_new( DType::Decimal(decimal_type, nullability), Some(ScalarValue::Decimal(value)), ) - .vortex_expect("unable to construct a decimal `Scalar`") } /// Creates a new UTF-8 scalar from a string-like value. From 0ee4df9e4a6ebd3b29d5d64157da147aedccf892 Mon Sep 17 00:00:00 2001 From: Vincent <1729065730@qq.com> Date: Mon, 26 Oct 2026 23:32:29 +0800 Subject: [PATCH 2/3] Address review: tests, contract docs, and other fallible decimal scalars - Match the precision error message in the out-of-range tests, return VortexResult, and split the cases with rstest. Add a negative min case and a nullable case where the out-of-range value sits in a null slot. - Document on DecimalArray::new_unchecked that values are not required to fit the precision and are not checked on construction or file reads. - Use Scalar::try_decimal in DecimalByteParts::scalar_at, the fuzz scalar_at reference, and the Python scalar factory (ValueError instead of a panic). Signed-off-by: Vincent <1729065730@qq.com> --- .../src/decimal_byte_parts/array.rs | 3 +- fuzz/src/array/scalar_at.rs | 4 +- .../src/aggregate_fn/fns/min_max/mod.rs | 61 ++++++++++++++----- vortex-array/src/arrays/decimal/array.rs | 14 +++-- .../src/arrays/decimal/vtable/operations.rs | 41 +++++++++++-- vortex-python/src/scalar/factory.rs | 7 +-- 6 files changed, 95 insertions(+), 35 deletions(-) diff --git a/encodings/decimal-byte-parts/src/decimal_byte_parts/array.rs b/encodings/decimal-byte-parts/src/decimal_byte_parts/array.rs index 7b60b349023..8536b1dd5af 100644 --- a/encodings/decimal-byte-parts/src/decimal_byte_parts/array.rs +++ b/encodings/decimal-byte-parts/src/decimal_byte_parts/array.rs @@ -26,7 +26,6 @@ use vortex_array::dtype::DecimalDType; use vortex_array::require_child; use vortex_array::scalar::DecimalValue; use vortex_array::scalar::Scalar; -use vortex_array::scalar::ScalarValue; use vortex_array::serde::ArrayChildren; use vortex_array::vtable::OperationsVTable; use vortex_array::vtable::VTable; @@ -397,7 +396,7 @@ impl OperationsVTable for DecimalByteParts { ), }; - Scalar::try_new(array.dtype().clone(), Some(ScalarValue::Decimal(value))) + Scalar::try_decimal(value, array.decimal_dtype(), array.dtype().nullability()) } } diff --git a/fuzz/src/array/scalar_at.rs b/fuzz/src/array/scalar_at.rs index c978aacbc0d..28b85fcf810 100644 --- a/fuzz/src/array/scalar_at.rs +++ b/fuzz/src/array/scalar_at.rs @@ -48,11 +48,11 @@ pub fn scalar_at_canonical_array( } Canonical::Decimal(array) => { match_each_decimal_value_type!(array.values_type(), |D| { - Scalar::decimal( + Scalar::try_decimal( DecimalValue::from(array.buffer::()[index]), array.decimal_dtype(), array.dtype().nullability(), - ) + )? }) } Canonical::VarBinView(array) => varbin_scalar(array.bytes_at(index), array.dtype()), diff --git a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs index af66cc00022..2785de40753 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs @@ -517,6 +517,7 @@ mod tests { use std::sync::Arc; use std::sync::LazyLock; + use rstest::rstest; use vortex_buffer::BitBuffer; use vortex_buffer::buffer; use vortex_error::VortexExpect; @@ -969,21 +970,53 @@ mod tests { Ok(()) } - #[test] - fn test_decimal_value_outside_precision_errors() { + #[rstest] + #[case::one_element(buffer![100i8])] + #[case::min_max(buffer![1i8, 100i8])] + #[case::negative_min(buffer![-100i8, 1i8])] + fn test_decimal_value_outside_precision_errors( + #[case] values: vortex_buffer::Buffer, + ) -> VortexResult<()> { // 100 does not fit in decimal(2, 0), and `DecimalArray::new` does not check the values. - for values in [buffer![100i8], buffer![1i8, 100i8]] { - let decimal = DecimalArray::new(values, DecimalDType::new(2, 0), Validity::NonNullable); - let mut ctx = SESSION.create_execution_ctx(); - assert!( - min_max( - &decimal.into_array(), - &mut ctx, - NumericalAggregateOpts::default(), - ) - .is_err() - ); - } + let decimal = DecimalArray::new(values, DecimalDType::new(2, 0), Validity::NonNullable); + let mut ctx = SESSION.create_execution_ctx(); + let err = min_max( + &decimal.into_array(), + &mut ctx, + NumericalAggregateOpts::default(), + ) + .unwrap_err(); + assert!( + err.to_string().contains("does not fit in precision"), + "unexpected error: {err}" + ); + Ok(()) + } + + #[test] + fn test_decimal_value_outside_precision_in_null_slot() -> VortexResult<()> { + // The out-of-range value sits in a null slot, so it must be ignored. + let decimal = DecimalArray::new( + buffer![5i8, 100i8], + DecimalDType::new(2, 0), + Validity::from_iter([true, false]), + ); + let mut ctx = SESSION.create_execution_ctx(); + let result = min_max( + &decimal.into_array(), + &mut ctx, + NumericalAggregateOpts::default(), + )? + .vortex_expect("should have result"); + + let expected = Scalar::decimal( + DecimalValue::I8(5), + DecimalDType::new(2, 0), + Nullability::NonNullable, + ); + assert_eq!(result.min, expected); + assert_eq!(result.max, expected); + Ok(()) } #[test] diff --git a/vortex-array/src/arrays/decimal/array.rs b/vortex-array/src/arrays/decimal/array.rs index 9269e7c83ea..2c752c8603b 100644 --- a/vortex-array/src/arrays/decimal/array.rs +++ b/vortex-array/src/arrays/decimal/array.rs @@ -257,9 +257,12 @@ impl DecimalData { /// /// The caller must ensure all of the following invariants are satisfied: /// - /// - All non-null values in `buffer` must be representable within the specified precision. - /// - For example, with precision=5 and scale=2, all values must be in range [-999.99, 999.99]. /// - If `validity` is [`Validity::Array`], its length must exactly equal `buffer.len()`. + /// + /// Values are not required to fit the precision of `decimal_dtype`. Neither this constructor + /// nor [`DecimalArray::try_new`] checks them, and arrays read from a file keep whatever values + /// were stored. Code that turns stored values into scalars must use a fallible path such as + /// [`Scalar::try_decimal`](crate::scalar::Scalar::try_decimal) rather than assume they fit. pub unsafe fn new_unchecked( buffer: Buffer, decimal_dtype: DecimalDType, @@ -280,9 +283,9 @@ impl DecimalData { /// /// The caller must ensure all of the following invariants are satisfied: /// - /// - All non-null values in `values` must be representable within the specified precision. - /// - For example, with precision=5 and scale=2, all values must be in range [-999.99, 999.99]. - /// - If `validity` is [`Validity::Array`], its length must exactly equal `buffer.len()`. + /// - If `validity` is [`Validity::Array`], its length must exactly equal `values.len()`. + /// + /// See [`DecimalArray::new_unchecked`] for why values are not required to fit the precision. pub unsafe fn new_unchecked_handle( values: BufferHandle, values_type: DecimalType, @@ -325,7 +328,6 @@ impl DecimalData { /// The caller must ensure: /// - The `byte_buffer` contains valid data for the specified `values_type` /// - The buffer length is compatible with the `values_type` (i.e., divisible by the type size) - /// - All non-null values are representable within the specified precision /// - If `validity` is [`Validity::Array`], its length must equal the number of elements pub unsafe fn new_unchecked_from_byte_buffer( byte_buffer: ByteBuffer, diff --git a/vortex-array/src/arrays/decimal/vtable/operations.rs b/vortex-array/src/arrays/decimal/vtable/operations.rs index 1b6ffb0b340..5b91d79de9b 100644 --- a/vortex-array/src/arrays/decimal/vtable/operations.rs +++ b/vortex-array/src/arrays/decimal/vtable/operations.rs @@ -35,7 +35,9 @@ impl OperationsVTable for Decimal { mod tests { use std::sync::LazyLock; + use rstest::rstest; use vortex_buffer::buffer; + use vortex_error::VortexResult; use vortex_session::VortexSession; use crate::IntoArray; @@ -105,18 +107,45 @@ mod tests { ); } - #[test] - fn test_scalar_at_value_outside_precision_errors() { + #[rstest] + #[case::positive(100i8)] + #[case::negative(-100i8)] + fn test_scalar_at_value_outside_precision_errors(#[case] value: i8) -> VortexResult<()> { + // `DecimalArray::new` does not check that the values fit the precision. let array = DecimalArray::new( - buffer![100i8], + buffer![value], DecimalDType::new(2, 0), Validity::NonNullable, ); + let err = array + .execute_scalar(0, &mut SESSION.create_execution_ctx()) + .unwrap_err(); assert!( - array - .execute_scalar(0, &mut SESSION.create_execution_ctx()) - .is_err() + err.to_string().contains("does not fit in precision"), + "unexpected error: {err}" + ); + Ok(()) + } + + #[test] + fn test_scalar_at_ignores_value_outside_precision_in_null_slot() -> VortexResult<()> { + let array = DecimalArray::new( + buffer![5i8, 100i8], + DecimalDType::new(2, 0), + Validity::from_iter([true, false]), + ); + let mut ctx = SESSION.create_execution_ctx(); + + assert_eq!( + array.execute_scalar(0, &mut ctx)?, + Scalar::decimal( + DecimalValue::I8(5), + DecimalDType::new(2, 0), + Nullability::Nullable + ) ); + assert!(array.execute_scalar(1, &mut ctx)?.is_null()); + Ok(()) } } diff --git a/vortex-python/src/scalar/factory.rs b/vortex-python/src/scalar/factory.rs index b46ecf934d8..fae227a7853 100644 --- a/vortex-python/src/scalar/factory.rs +++ b/vortex-python/src/scalar/factory.rs @@ -154,11 +154,8 @@ fn scalar_helper_inner(value: &Bound<'_, PyAny>, dtype: Option<&DType>) -> PyRes "Value can't be represented as decimal", )); }; - return Ok(Scalar::decimal( - value, - *decimal_dtype, - Nullability::NonNullable, - )); + return Scalar::try_decimal(value, *decimal_dtype, Nullability::NonNullable) + .map_err(|e| PyValueError::new_err(e.to_string())); } if let Ok(integer) = value.cast::() { From 938932296af80791a3cf9aff87b62c5a8d193ba4 Mon Sep 17 00:00:00 2001 From: Vincent <1729065730@qq.com> Date: Tue, 27 Oct 2026 03:28:19 +0800 Subject: [PATCH 3/3] Address review: safety docs, min/max guard, and comment cleanup Signed-off-by: Vincent <1729065730@qq.com> --- .../src/aggregate_fn/fns/min_max/decimal.rs | 58 ++++++++----------- .../src/aggregate_fn/fns/min_max/mod.rs | 8 +-- vortex-array/src/arrays/decimal/array.rs | 25 ++++---- .../src/arrays/decimal/vtable/operations.rs | 3 +- vortex-array/src/scalar/constructor.rs | 2 +- 5 files changed, 42 insertions(+), 54 deletions(-) diff --git a/vortex-array/src/aggregate_fn/fns/min_max/decimal.rs b/vortex-array/src/aggregate_fn/fns/min_max/decimal.rs index ea26b7e65a1..2c361dea778 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/decimal.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/decimal.rs @@ -38,24 +38,22 @@ fn compute_min_max_with_validity( where D: Into + NativeDecimalType, { - Ok( - match array - .as_ref() - .validity()? - .execute_mask(array.as_ref().len(), ctx)? - { - Mask::AllTrue(_) => compute_min_max(array.buffer::().iter(), array.decimal_dtype())?, - Mask::AllFalse(_) => None, - Mask::Values(v) => compute_min_max( - array - .buffer::() - .iter() - .zip(v.bit_buffer().iter()) - .filter_map(|(v, m)| m.then_some(v)), - array.decimal_dtype(), - )?, - }, - ) + let mask = array + .as_ref() + .validity()? + .execute_mask(array.as_ref().len(), ctx)?; + match mask { + Mask::AllTrue(_) => compute_min_max(array.buffer::().iter(), array.decimal_dtype()), + Mask::AllFalse(_) => Ok(None), + Mask::Values(v) => compute_min_max( + array + .buffer::() + .iter() + .zip(v.bit_buffer().iter()) + .filter_map(|(v, m)| m.then_some(v)), + array.decimal_dtype(), + ), + } } fn compute_min_max<'a, T>( @@ -65,20 +63,12 @@ fn compute_min_max<'a, T>( where T: Into + NativeDecimalType + Ord + Copy + 'a, { - // The stored values are not guaranteed to fit the precision (for example, when read from a - // file), so return an error instead of panicking. - Ok(match iter.minmax_by(|a, b| a.cmp(b)) { - itertools::MinMaxResult::NoElements => None, - itertools::MinMaxResult::OneElement(&x) => { - let scalar = Scalar::try_decimal(x.into(), decimal_dtype, NonNullable)?; - Some(MinMaxResult { - min: scalar.clone(), - max: scalar, - }) - } - itertools::MinMaxResult::MinMax(&min, &max) => Some(MinMaxResult { - min: Scalar::try_decimal(min.into(), decimal_dtype, NonNullable)?, - max: Scalar::try_decimal(max.into(), decimal_dtype, NonNullable)?, - }), - }) + let Some((&min, &max)) = iter.minmax().into_option() else { + return Ok(None); + }; + + Ok(Some(MinMaxResult { + min: Scalar::try_decimal(min.into(), decimal_dtype, NonNullable)?, + max: Scalar::try_decimal(max.into(), decimal_dtype, NonNullable)?, + })) } diff --git a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs index 2785de40753..748c72a310f 100644 --- a/vortex-array/src/aggregate_fn/fns/min_max/mod.rs +++ b/vortex-array/src/aggregate_fn/fns/min_max/mod.rs @@ -519,6 +519,7 @@ mod tests { use rstest::rstest; use vortex_buffer::BitBuffer; + use vortex_buffer::Buffer; use vortex_buffer::buffer; use vortex_error::VortexExpect; use vortex_error::VortexResult; @@ -974,10 +975,9 @@ mod tests { #[case::one_element(buffer![100i8])] #[case::min_max(buffer![1i8, 100i8])] #[case::negative_min(buffer![-100i8, 1i8])] - fn test_decimal_value_outside_precision_errors( - #[case] values: vortex_buffer::Buffer, - ) -> VortexResult<()> { - // 100 does not fit in decimal(2, 0), and `DecimalArray::new` does not check the values. + fn test_decimal_value_outside_precision_errors(#[case] values: Buffer) -> VortexResult<()> { + // 100 and -100 do not fit in decimal(2, 0), and `DecimalArray::new` does not check the + // values. let decimal = DecimalArray::new(values, DecimalDType::new(2, 0), Validity::NonNullable); let mut ctx = SESSION.create_execution_ctx(); let err = min_max( diff --git a/vortex-array/src/arrays/decimal/array.rs b/vortex-array/src/arrays/decimal/array.rs index 2c752c8603b..02ad51bb1df 100644 --- a/vortex-array/src/arrays/decimal/array.rs +++ b/vortex-array/src/arrays/decimal/array.rs @@ -251,18 +251,15 @@ impl DecimalData { /// /// * `buffer` is a typed buffer containing the decimal values. /// * `decimal_dtype` specifies the decimal precision and scale. - /// * `validity` holds the null values. /// - /// # Safety - /// - /// The caller must ensure all of the following invariants are satisfied: + /// Values are not checked against the precision of `decimal_dtype`, either here, in + /// [`DecimalArray::try_new`], or when an array is read from a file. Code that turns stored values + /// into scalars must use a fallible path such as + /// [`Scalar::try_decimal`](crate::scalar::Scalar::try_decimal). /// - /// - If `validity` is [`Validity::Array`], its length must exactly equal `buffer.len()`. + /// # Safety /// - /// Values are not required to fit the precision of `decimal_dtype`. Neither this constructor - /// nor [`DecimalArray::try_new`] checks them, and arrays read from a file keep whatever values - /// were stored. Code that turns stored values into scalars must use a fallible path such as - /// [`Scalar::try_decimal`](crate::scalar::Scalar::try_decimal) rather than assume they fit. + /// `buffer` is a typed [`Buffer`], so there are no further requirements on the caller. pub unsafe fn new_unchecked( buffer: Buffer, decimal_dtype: DecimalDType, @@ -279,13 +276,15 @@ impl DecimalData { /// Create a new array with decimal values backed by the given buffer handle. /// - /// # Safety + /// As with [`DecimalArray::new_unchecked`], values are not checked against the precision of + /// `decimal_dtype`. /// - /// The caller must ensure all of the following invariants are satisfied: + /// # Safety /// - /// - If `validity` is [`Validity::Array`], its length must exactly equal `values.len()`. + /// - The length of `values` must be a multiple of the byte width of `values_type`. + /// - `values` must be aligned to the native type of `values_type`. /// - /// See [`DecimalArray::new_unchecked`] for why values are not required to fit the precision. + /// Breaking either rule does not cause undefined behavior, but [`DecimalData::buffer`] panics. pub unsafe fn new_unchecked_handle( values: BufferHandle, values_type: DecimalType, diff --git a/vortex-array/src/arrays/decimal/vtable/operations.rs b/vortex-array/src/arrays/decimal/vtable/operations.rs index 5b91d79de9b..c43ff65c7a6 100644 --- a/vortex-array/src/arrays/decimal/vtable/operations.rs +++ b/vortex-array/src/arrays/decimal/vtable/operations.rs @@ -19,8 +19,7 @@ impl OperationsVTable for Decimal { index: usize, _ctx: &mut ExecutionCtx, ) -> VortexResult { - // The stored values are not guaranteed to fit the precision (for example, when read from a - // file), so return an error instead of panicking. + // Stored values can fall outside the precision, see `DecimalData::new_unchecked`. match_each_decimal_value_type!(array.values_type(), |D| { Scalar::try_decimal( DecimalValue::from(array.buffer::()[index]), diff --git a/vortex-array/src/scalar/constructor.rs b/vortex-array/src/scalar/constructor.rs index c827b9f666e..b4528608a9c 100644 --- a/vortex-array/src/scalar/constructor.rs +++ b/vortex-array/src/scalar/constructor.rs @@ -76,7 +76,7 @@ impl Scalar { nullability: Nullability, ) -> Self { Self::try_decimal(value, decimal_type, nullability) - .vortex_expect("unable to construct a decimal `Scalar`") + .vortex_expect("decimal value must fit the precision of `decimal_type`") } /// Attempts to create a new decimal scalar with the given value, precision, scale, and