diff --git a/vortex-array/src/arrays/primitive/compute/cast.rs b/vortex-array/src/arrays/primitive/compute/cast.rs index 68b3da04cca..76f1cc68546 100644 --- a/vortex-array/src/arrays/primitive/compute/cast.rs +++ b/vortex-array/src/arrays/primitive/compute/cast.rs @@ -19,6 +19,7 @@ use crate::dtype::DType; use crate::dtype::NativePType; use crate::dtype::Nullability; use crate::dtype::PType; +use crate::match_each_float_ptype; use crate::match_each_native_ptype; use crate::scalar_fn::fns::cast::CastKernel; use crate::scalar_fn::fns::cast::CastReduce; @@ -90,6 +91,12 @@ impl CastKernel for Primitive { ); } + // Reject float-to-integer casts that would silently truncate fractional parts. + // Invalid (null) slots are excluded from the check; their underlying bits are irrelevant. + if array.ptype().is_float() && new_ptype.is_int() { + reject_fractional_values(array, ctx)?; + } + // Same-width integers have identical bit representations due to 2's // complement. If all values fit in the target range, reinterpret with // no allocation. @@ -131,6 +138,47 @@ fn values_fit_in( .is_none_or(|mm| mm.min.cast(&target_dtype).is_ok() && mm.max.cast(&target_dtype).is_ok()) } +/// Returns an error if any valid element in `array` (a float source ptype) has a non-zero +/// fractional part. Invalid (null) positions are skipped. +/// +/// NaN and ±Inf are caught here too: `fract()` on those values returns NaN, and +/// `NaN != 0.0` evaluates to `true`, so they are also rejected — in practice they are +/// already rejected by `values_fit_in` unless masked as invalid. +fn reject_fractional_values( + array: ArrayView<'_, Primitive>, + ctx: &mut ExecutionCtx, +) -> VortexResult<()> { + let validity = array.validity()?.execute_mask(array.len(), ctx)?; + match_each_float_ptype!(array.ptype(), |F| { + reject_fractional_slice::(array.as_slice::(), &validity, array.ptype())?; + }); + Ok(()) +} + +/// Per-slice fractional check, generic over any `NativePType` that can be converted to `f64`. +fn reject_fractional_slice( + slice: &[F], + validity: &vortex_mask::Mask, + src_ptype: PType, +) -> VortexResult<()> +where + F: NativePType + Copy + std::fmt::Display + Into, +{ + for (i, &src) in slice.iter().enumerate() { + if !validity.value(i) { + continue; + } + let as_f64: f64 = src.into(); + if as_f64.fract() != 0.0 { + vortex_bail!( + Compute: "Cannot cast fractional value {} to integer — value has a fractional part", + src_ptype, + ); + } + } + Ok(()) +} + /// Caller must ensure all valid values are representable via `values_fit_in`. /// Out-of-range values at invalid positions are truncated/wrapped by `as`, /// which is fine because they are masked out by validity. @@ -139,7 +187,7 @@ fn cast, T: NativePType>(array: &[F]) -> Buffer< } #[cfg(test)] -mod test { +mod tests { use rstest::rstest; use vortex_buffer::BitBuffer; use vortex_buffer::buffer; @@ -365,6 +413,47 @@ mod test { Ok(()) } + /// ABA-20: casting a fractional f64 to i64 must return an error, not silently truncate. + /// + /// Repro: 1.5_f64 cast to i64 must error; 2.0_f64 cast to i64 must succeed with value 2. + /// Validity-masked invalid slots (None) must not cause false positives. + #[test] + fn issue_aba20_cast_f64_to_i64_rejects_fractional_values() -> vortex_error::VortexResult<()> { + // Fractional value must error. + let fractional = buffer![1.5_f64].into_array(); + #[expect(deprecated)] + assert!( + fractional + .cast(PType::I64.into()) + .and_then(|a| a.to_canonical().map(|c| c.into_array())) + .is_err(), + "cast(1.5_f64 -> i64) must return Err — value has a fractional part" + ); + + // Exact float must succeed with the correct integer value. + let exact = buffer![2.0_f64].into_array(); + #[expect(deprecated)] + let result = exact + .cast(PType::I64.into()) + .and_then(|a| a.to_canonical().map(|c| c.into_array()))?; + assert_arrays_eq!(result.to_primitive(), PrimitiveArray::from_iter([2i64])); + + // Invalid (null) slots must not cause false positives — the None slot's underlying + // bits are irrelevant and must be masked out, not checked for fractionality. + let with_nulls = PrimitiveArray::from_option_iter([Some(2.0_f64), None, Some(3.0)]); + #[expect(deprecated)] + let nullable_result = with_nulls + .into_array() + .cast(DType::Primitive(PType::I64, Nullability::Nullable)) + .and_then(|a| a.to_canonical().map(|c| c.into_array()))?; + assert_arrays_eq!( + nullable_result.to_primitive(), + PrimitiveArray::from_option_iter([Some(2i64), None, Some(3)]) + ); + + Ok(()) + } + #[rstest] #[case(buffer![0u8, 1, 2, 3, 255].into_array())] #[case(buffer![0u16, 100, 1000, 65535].into_array())] diff --git a/vortex-array/src/scalar/typed_view/primitive/pvalue.rs b/vortex-array/src/scalar/typed_view/primitive/pvalue.rs index 02640d2e416..c7361cdb961 100644 --- a/vortex-array/src/scalar/typed_view/primitive/pvalue.rs +++ b/vortex-array/src/scalar/typed_view/primitive/pvalue.rs @@ -263,9 +263,35 @@ impl PValue { /// Converts this value to a specific native primitive type. /// /// # Errors - /// Returns `VortexError` if the conversion is not supported or would overflow. + /// Returns `VortexError` if the conversion is not supported, would overflow, or would + /// silently discard a fractional part (float-to-integer cast with a non-zero fraction). #[inline] pub fn cast(&self) -> VortexResult { + // Reject float-to-integer casts that would silently truncate a fractional part. + // NaN and ±Inf are also rejected because their fract() is NaN (≠ 0.0). + if T::PTYPE.is_int() { + match *self { + PValue::F16(f) => { + // f16 has no fract(); promote to f32 via ToPrimitive then check. + let as_f32 = ToPrimitive::to_f32(&f).unwrap_or(f32::NAN); + if as_f32.fract() != 0.0f32 { + vortex_bail!("Cannot cast fractional value {f} to {}", T::PTYPE); + } + } + PValue::F32(f) => { + if f.fract() != 0.0f32 { + vortex_bail!("Cannot cast fractional value {f} to {}", T::PTYPE); + } + } + PValue::F64(f) => { + if f.fract() != 0.0f64 { + vortex_bail!("Cannot cast fractional value {f} to {}", T::PTYPE); + } + } + _ => {} + } + } + let res = match *self { PValue::U8(u) => T::from_u8(u), PValue::U16(u) => T::from_u16(u),