This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11302-953813bbd2c3f579cdfdec93ed31d5f36e2a6b9d in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 5c5d24abf25290634a423d498629fdf65953b4c7 Author: Kosta Tarasov <[email protected]> AuthorDate: Thu Oct 15 15:55:14 2026 +0000 fix: move float-to-decimal precision validation inside the cast kernel (#11302) # Which issue does this PR close? Closes #10795. # Rationale for this change `variant_get` converts floats and doubles to decimals without checking the target precision. For example, converting `12345.678` to `Decimal32(5, 2)` produces `12345.68`, even though the largest allowed value is `999.99`. Arrow's array cast already rejects this value. Centralize the precision check in a shared scalar conversion, following the approach used for string-to-decimal parsing in #10850, so callers do not need to remember a separate validation step. # What changes are included in this PR? - Add `float_to_decimal(input, precision, scale)`, which checks the target parameters and the scaled, rounded value. - Share the checked conversion between the Arrow array cast and `variant_get`. Safe array casts discard failures without allocating error messages; strict casts retain their existing diagnostics. - Deprecate `single_float_to_decimal` while preserving its existing conversion behavior. # Are these changes tested? Extended the existing `variant_get` safe/strict overflow tests with float and double inputs, including overflow caused by rounding. Added scalar API tests across all four decimal widths, invalid parameters, and non-finite values. A compatibility regression test verifies that the deprecated helper still accepts values that fit the native integer but exceed decimal precision, while the new API rejects them. Array benchmark coverage is in #11343; Variant benchmarks are proposed in #11416. # Are there any significant user-facing changes? `variant_get` now returns null for float-to-decimal precision overflow in safe mode and an error in strict mode. The new scalar API is public. The deprecated helper keeps its signature and existing behavior. Float-to-decimal array casts now validate precision and scale before processing values. # AI usage Developed with assistance from OpenAI Codex for implementation, tests, and the PR description. --------- Co-authored-by: Jeffrey Vo <[email protected]> --- arrow-cast/src/cast/decimal.rs | 195 +++++++++++++++++++++++-- arrow-cast/src/cast/mod.rs | 4 +- parquet-variant-compute/src/type_conversion.rs | 16 +- parquet-variant-compute/src/variant_get.rs | 62 +++++--- 4 files changed, 228 insertions(+), 49 deletions(-) diff --git a/arrow-cast/src/cast/decimal.rs b/arrow-cast/src/cast/decimal.rs index 2a11a142a6..367f3207df 100644 --- a/arrow-cast/src/cast/decimal.rs +++ b/arrow-cast/src/cast/decimal.rs @@ -674,37 +674,116 @@ where D: DecimalType + ArrowPrimitiveType, <D as ArrowPrimitiveType>::Native: DecimalCast, { + validate_decimal_precision_and_scale::<D>(precision, scale)?; let mul = 10_f64.powi(scale as i32); if cast_options.safe { array - .unary_opt::<_, D>(|v| { - single_float_to_decimal::<D>(v.as_(), mul) - .filter(|v| D::is_valid_decimal_precision(*v, precision)) - }) + .unary_opt::<_, D>(|v| float_to_decimal_checked::<D>(v.as_(), mul, precision).ok()) .with_precision_and_scale(precision, scale) .map(|a| Arc::new(a) as ArrayRef) } else { array .try_unary::<_, D, _>(|v| { - let v = single_float_to_decimal::<D>(v.as_(), mul).ok_or_else(|| { - ArrowError::CastError(format!( - "Cannot cast to {}({}, {}). Overflowing on {:?}", - D::PREFIX, - precision, - scale, - v - )) - })?; - D::validate_decimal_precision(v, precision, scale).map(|()| v) + float_to_decimal_checked::<D>(v.as_(), mul, precision) + .map_err(|e| e.into_arrow_error(v, precision, scale)) })? .with_precision_and_scale(precision, scale) .map(|a| Arc::new(a) as ArrayRef) } } +/// Converts a floating point value to the unscaled integer representation of a decimal. +/// +/// Scales the input by `10^scale` and rounds half away from zero. Returns an error +/// if the precision or scale is invalid, or the result does not fit the native +/// type or the requested precision. Non-finite inputs are rejected. +/// Use [`float_to_decimal_opt`] when the reason for failure is not needed. +/// +/// ``` +/// use arrow_array::types::Decimal32Type; +/// use arrow_cast::cast::float_to_decimal; +/// +/// assert_eq!(float_to_decimal::<Decimal32Type>(12.345, 5, 2).unwrap(), 1235); +/// assert!(float_to_decimal::<Decimal32Type>(12345.678, 5, 2).is_err()); +/// ``` +#[inline] +pub fn float_to_decimal<D>(input: f64, precision: u8, scale: i8) -> Result<D::Native, ArrowError> +where + D: DecimalType, + D::Native: DecimalCast, +{ + validate_decimal_precision_and_scale::<D>(precision, scale)?; + float_to_decimal_checked::<D>(input, 10_f64.powi(scale as i32), precision) + .map_err(|e| e.into_arrow_error(input, precision, scale)) +} + +/// Like [`float_to_decimal`], but returns `None` on failure. +/// With valid precision and scale, conversion failures do not allocate an error message. +/// +/// ``` +/// use arrow_array::types::Decimal32Type; +/// use arrow_cast::cast::float_to_decimal_opt; +/// +/// assert_eq!(float_to_decimal_opt::<Decimal32Type>(12.345, 5, 2), Some(1235)); +/// assert_eq!(float_to_decimal_opt::<Decimal32Type>(12345.678, 5, 2), None); +/// ``` +pub fn float_to_decimal_opt<D>(input: f64, precision: u8, scale: i8) -> Option<D::Native> +where + D: DecimalType, + D::Native: DecimalCast, +{ + validate_decimal_precision_and_scale::<D>(precision, scale).ok()?; + float_to_decimal_checked::<D>(input, 10_f64.powi(scale as i32), precision).ok() +} + +// Keep failure reporting allocation-free for scalar Option and safe array casts. +enum FloatToDecimalError<D: DecimalType> { + NativeOverflow, + PrecisionOverflow(D::Native), +} + +impl<D: DecimalType> FloatToDecimalError<D> { + fn into_arrow_error(self, input: impl std::fmt::Debug, precision: u8, scale: i8) -> ArrowError { + match self { + Self::NativeOverflow => ArrowError::CastError(format!( + "Cannot cast to {}({}, {}). Overflowing on {:?}", + D::PREFIX, + precision, + scale, + input + )), + Self::PrecisionOverflow(value) => { + D::validate_decimal_precision(value, precision, scale).unwrap_err() + } + } + } +} + +// `mul` is precomputed once for array casts. Precision must already be validated. +#[inline] +fn float_to_decimal_checked<D>( + input: f64, + mul: f64, + precision: u8, +) -> Result<D::Native, FloatToDecimalError<D>> +where + D: DecimalType, + D::Native: DecimalCast, +{ + let value = + D::Native::from_f64((mul * input).round()).ok_or(FloatToDecimalError::NativeOverflow)?; + if D::is_valid_decimal_precision(value, precision) { + Ok(value) + } else { + Err(FloatToDecimalError::PrecisionOverflow(value)) + } +} + /// Cast a single floating point value to a decimal native with the given multiple. -/// Returns `None` if the value cannot be represented with the requested precision. +/// Returns `None` if the scaled and rounded value does not fit the native integer +/// type. The caller must separately validate the decimal precision. +#[deprecated(since = "60.0.0", note = "Use `float_to_decimal` instead")] #[inline(always)] pub fn single_float_to_decimal<D>(input: f64, mul: f64) -> Option<D::Native> where @@ -836,6 +915,92 @@ where mod tests { use super::*; + #[test] + fn test_float_to_decimal() { + fn check<D: DecimalType>() + where + D::Native: DecimalCast, + { + for (input, scale, expected) in [ + (12.345, 2, 1235), + (-12.345, 2, -1235), + (999.99, 2, 99999), + (-999.99, 2, -99999), + (99999.25, 0, 99999), + (999992.5, -1, 99999), + (0.0, 2, 0), + ] { + assert_eq!( + float_to_decimal_opt::<D>(input, 5, scale), + float_to_decimal::<D>(input, 5, scale).ok(), + ); + assert_eq!( + float_to_decimal::<D>(input, 5, scale) + .unwrap() + .to_i128() + .unwrap(), + expected, + ); + } + for (input, scale) in [ + (12345.678, 2), + (-12345.678, 2), + (999.9975, 2), + (-999.9975, 2), + (99999.75, 0), + (-99999.75, 0), + (999997.5, -1), + (-999997.5, -1), + (f64::NAN, 0), + (f64::INFINITY, 0), + (f64::NEG_INFINITY, 0), + (f64::MAX, 0), + ] { + assert!(float_to_decimal::<D>(input, 5, scale).is_err()); + assert!(float_to_decimal_opt::<D>(input, 5, scale).is_none()); + } + for (precision, scale) in [(0, 0), (D::MAX_PRECISION + 1, 0), (5, 6)] { + assert!(float_to_decimal::<D>(0.0, precision, scale).is_err()); + assert!(float_to_decimal_opt::<D>(0.0, precision, scale).is_none()); + } + } + check::<Decimal32Type>(); + check::<Decimal64Type>(); + check::<Decimal128Type>(); + check::<Decimal256Type>(); + } + + #[test] + #[expect(deprecated)] + fn test_single_float_to_decimal_preserves_native_range() { + // Deprecation preserves acceptance of values that fit i32 but exceed + // Decimal32's maximum precision. The new API checks that precision. + for (input, expected) in [ + (1_000_000_000.0, 1_000_000_000), + (-1_000_000_000.0, -1_000_000_000), + (999_999_999.75, 1_000_000_000), + (-999_999_999.75, -1_000_000_000), + ] { + assert_eq!( + single_float_to_decimal::<Decimal32Type>(input, 1.0), + Some(expected) + ); + assert!(float_to_decimal::<Decimal32Type>(input, 9, 0).is_err()); + } + assert_eq!( + single_float_to_decimal::<Decimal32Type>(999_999_999.0, 1.0), + Some(999_999_999), + ); + assert_eq!( + single_float_to_decimal::<Decimal32Type>(12.345, 100.0), + Some(1235) + ); + assert_eq!( + single_float_to_decimal::<Decimal32Type>(f64::MAX, 1.0), + None + ); + } + #[test] #[expect(deprecated)] fn test_parse_string_to_decimal_native() { diff --git a/arrow-cast/src/cast/mod.rs b/arrow-cast/src/cast/mod.rs index 862ea50730..65bf6bbc8d 100644 --- a/arrow-cast/src/cast/mod.rs +++ b/arrow-cast/src/cast/mod.rs @@ -76,9 +76,9 @@ use arrow_schema::*; use arrow_select::take::take; use num_traits::{NumCast, ToPrimitive, cast::AsPrimitive}; +pub use decimal::{DecimalCast, float_to_decimal, float_to_decimal_opt, rescale_decimal}; #[expect(deprecated)] -pub use decimal::parse_string_to_decimal_native; -pub use decimal::{DecimalCast, rescale_decimal, single_float_to_decimal}; +pub use decimal::{parse_string_to_decimal_native, single_float_to_decimal}; pub use string::cast_single_string_to_boolean_default; /// Integers below 2^53 convert to an `f64` exactly. diff --git a/parquet-variant-compute/src/type_conversion.rs b/parquet-variant-compute/src/type_conversion.rs index 2882b70bbe..72282b6c0d 100644 --- a/parquet-variant-compute/src/type_conversion.rs +++ b/parquet-variant-compute/src/type_conversion.rs @@ -20,9 +20,9 @@ use arrow::array::ArrowNativeTypeOp; use arrow::compute::kernels::cast_utils::parse_decimal; use arrow::compute::{ - CastOptions, DecimalCast, cast_num_to_bool, cast_single_string_to_boolean_default, num_cast, - rescale_decimal, single_bool_to_numeric, single_decimal_to_float_lossy, - single_float_to_decimal, + CastOptions, DecimalCast, cast_num_to_bool, cast_single_string_to_boolean_default, + float_to_decimal_opt, num_cast, rescale_decimal, single_bool_to_numeric, + single_decimal_to_float_lossy, }; use arrow::datatypes::{ self, ArrowPrimitiveType, ArrowTimestampType, Decimal32Type, Decimal64Type, Decimal128Type, @@ -382,7 +382,7 @@ impl_timestamp_from_variant!( /// - Decimal variants (`Decimal4/8/16`) use their embedded precision and scale /// /// The value is rescaled to (`precision`, `scale`) using `rescale_decimal` for integers, -/// `single_float_to_decimal` for floats, and `parse_decimal` for strings. +/// `float_to_decimal_opt` for floats, and `parse_decimal` for strings. /// returns `None` if it cannot fit the requested precision. pub(crate) fn variant_to_unscaled_decimal<O>( variant: &Variant<'_, '_>, @@ -393,8 +393,6 @@ where O: DecimalType, O::Native: DecimalCast, { - let mul = 10_f64.powi(scale as i32); - match variant { Variant::Int8(i) => rescale_decimal::<Decimal32Type, O>( *i as i32, @@ -424,8 +422,10 @@ where precision, scale, ), - Variant::Float(f) => single_float_to_decimal::<O>(<f64 as From<f32>>::from(*f), mul), - Variant::Double(f) => single_float_to_decimal::<O>(*f, mul), + Variant::Float(f) => { + float_to_decimal_opt::<O>(<f64 as From<f32>>::from(*f), precision, scale) + } + Variant::Double(f) => float_to_decimal_opt::<O>(*f, precision, scale), Variant::String(v) => parse_decimal::<O>(v, precision, scale).ok(), Variant::ShortString(v) => parse_decimal::<O>(v, precision, scale).ok(), Variant::Decimal4(d) => rescale_decimal::<Decimal32Type, O>( diff --git a/parquet-variant-compute/src/variant_get.rs b/parquet-variant-compute/src/variant_get.rs index 9ec38f0aa1..5cedb008a1 100644 --- a/parquet-variant-compute/src/variant_get.rs +++ b/parquet-variant-compute/src/variant_get.rs @@ -3994,10 +3994,19 @@ mod test { #[test] fn get_decimal32_precision_overflow_safe() { // Exceed Decimal32 after scaling and rounding - let variant_array: ArrayRef = ArrayRef::from(VariantArray::from_iter([ - Some(VariantDecimal4::try_new(VariantDecimal4::MAX_UNSCALED_VALUE, 0).unwrap()), + let variant_array = ArrayRef::from(VariantArray::from_iter([ + Some(Variant::from( + VariantDecimal4::try_new(VariantDecimal4::MAX_UNSCALED_VALUE, 0).unwrap(), + )), // integer value round up overflows - Some(VariantDecimal4::try_new(VariantDecimal4::MAX_UNSCALED_VALUE, 9).unwrap()), + Some(Variant::from( + VariantDecimal4::try_new(VariantDecimal4::MAX_UNSCALED_VALUE, 9).unwrap(), + )), + // Float inputs that exceed precision directly or after rounding to 1.00. + Some(Variant::Float(1.1)), + Some(Variant::Double(1.1)), + Some(Variant::Float(0.999)), + Some(Variant::Double(0.999)), ])); let field = Field::new("result", DataType::Decimal32(2, 2), true); @@ -4005,31 +4014,36 @@ mod test { let result = variant_get(&variant_array, options).unwrap(); let result = result.as_any().downcast_ref::<Decimal32Array>().unwrap(); - assert!(result.is_null(0)); - assert!(result.is_null(1)); // should overflow because 1.00 does not fit into precision (2) + assert_eq!(result.null_count(), 6); // Even 1.00 exceeds precision 2. } #[test] fn get_decimal32_precision_overflow_unsafe_errors() { - let variant_array: ArrayRef = ArrayRef::from(VariantArray::from_iter([Some( - VariantDecimal4::try_new(VariantDecimal4::MAX_UNSCALED_VALUE, 0).unwrap(), - )])); - - let field = Field::new("result", DataType::Decimal32(9, 2), true); - let cast_options = CastOptions { - safe: false, - ..Default::default() - }; - let options = GetOptions::new() - .with_as_type(Some(FieldRef::from(field))) - .with_cast_options(cast_options); - let err = variant_get(&variant_array, options).unwrap_err(); - - assert!( - err.to_string().contains( - "Failed to cast to Decimal32(precision=9, scale=2) from variant Decimal4" - ) - ); + for (value, kind) in [ + ( + VariantDecimal4::try_new(VariantDecimal4::MAX_UNSCALED_VALUE, 0) + .unwrap() + .into(), + "Decimal4", + ), + (Variant::Float(1.1), "Float"), + (Variant::Double(1.1), "Double"), + (Variant::Float(0.999), "Float"), + (Variant::Double(0.999), "Double"), + ] { + let variant_array = ArrayRef::from(VariantArray::from_iter([Some(value)])); + let field = Field::new("result", DataType::Decimal32(2, 2), true); + let options = GetOptions::new() + .with_as_type(Some(FieldRef::from(field))) + .with_cast_options(CastOptions { + safe: false, + ..Default::default() + }); + let err = variant_get(&variant_array, options).unwrap_err(); + assert!(err.to_string().contains(&format!( + "Failed to cast to Decimal32(precision=2, scale=2) from variant {kind}" + ))); + } } #[test]
