sdf-jkl commented on code in PR #10157:
URL: https://github.com/apache/arrow-rs/pull/10157#discussion_r3615771205
##########
parquet-variant-compute/src/type_conversion.rs:
##########
@@ -287,6 +444,267 @@ where
}
}
+/// Return the unscaled integer representation for Arrow decimal type `O` from
a `Variant`.
+///
+/// This function is unlike `variant_to_unscaled_decim`, it would never
rescale the decimal value,
+/// and only return the unscaled integer representation for the specific
decimal variants.
+pub(crate) fn shred_variant_to_unscaled_decimal<O>(
+ variant: &Variant<'_, '_>,
+ precision: u8,
+ scale: i8,
+) -> Option<O::Native>
+where
+ O: ShredDecimalVariant,
+ O::Native: DecimalCast,
+{
+ match variant {
+ Variant::Int8(_)
+ | Variant::Int16(_)
+ | Variant::Int32(_)
+ | Variant::Int64(_)
+ | Variant::Decimal4(_)
+ | Variant::Decimal8(_)
+ | Variant::Decimal16(_) => O::shred_variant(variant, precision, scale),
+ _ => None,
+ }
+}
+pub(crate) trait ShredDecimalVariant: DecimalType {
+ fn shred_variant(value: &Variant<'_, '_>, precision: u8, scale: i8) ->
Option<Self::Native>;
+}
+
+fn convert_to_unscaled_decimal<I, O>(
+ input: I::Native,
+ input_precision: u8,
+ input_scale: i8,
+ target_precision: u8,
+ target_scale: i8,
+) -> Option<O::Native>
+where
+ I: DecimalType,
+ O: DecimalType,
+ I::Native: DecimalCast,
+ O::Native: DecimalCast,
+{
+ if let Some(converted) = rescale_decimal::<I, O>(
+ input,
+ input_precision,
+ input_scale,
+ target_precision,
+ target_scale,
+ ) && let Some(convert_back) = rescale_decimal::<O, I>(
Review Comment:
The MSRV issue is the `if let .. && let` chain locked to 1.88.
##########
parquet-variant/src/variant.rs:
##########
@@ -837,190 +785,185 @@ impl<'m, 'v> Variant<'m, 'v> {
}
}
- fn cast_decimal_to_num<D, T, F>(raw: D::Native, scale: u8, as_float: F) ->
Option<T>
- where
- D: DecimalType,
- D::Native: NumCast + ArrowNativeTypeOp,
- T: DecimalCastTarget,
- F: Fn(D::Native) -> f64,
- {
- let base: D::Native = NumCast::from(10)?;
-
- let div = base.pow_checked(<u32 as From<u8>>::from(scale)).ok()?;
- match T::KIND {
- NumericKind::Integer => raw
- .div_checked(div)
- .ok()
- .and_then(<T as NumCast>::from::<D::Native>),
- NumericKind::Float => T::from(single_decimal_to_float_lossy::<D,
_>(
- &as_float,
- raw,
- <i32 as From<u8>>::from(scale),
- )),
- }
- }
-
- /// Converts a boolean or numeric variant(integers, floating-point, and
decimals)
- /// to the specified numeric type `T`.
- ///
- /// Uses Arrow's casting logic to perform the conversion. Returns
`Some(T)` if
- /// the conversion succeeds, `None` if the variant can't be casted to type
`T`.
- fn as_num<T>(&self) -> Option<T>
- where
- T: DecimalCastTarget,
- {
- match *self {
- Variant::BooleanFalse => single_bool_to_numeric(false),
- Variant::BooleanTrue => single_bool_to_numeric(true),
- Variant::Int8(i) => num_cast(i),
- Variant::Int16(i) => num_cast(i),
- Variant::Int32(i) => num_cast(i),
- Variant::Int64(i) => num_cast(i),
- Variant::Float(f) => num_cast(f),
- Variant::Double(d) => num_cast(d),
- Variant::Decimal4(d) => {
- Self::cast_decimal_to_num::<Decimal32Type, T, _>(d.integer(),
d.scale(), |x| {
- x as f64
- })
- }
- Variant::Decimal8(d) => {
- Self::cast_decimal_to_num::<Decimal64Type, T, _>(d.integer(),
d.scale(), |x| {
- x as f64
- })
- }
- Variant::Decimal16(d) => {
- Self::cast_decimal_to_num::<Decimal128Type, T, _>(d.integer(),
d.scale(), |x| {
- x as f64
- })
- }
- _ => None,
- }
- }
-
- /// Converts this variant to an `i8` if possible.
+ /// Converts this variant to an `i8` for int variants if possible.
///
- /// Returns `Some(i8)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i8` range,
+ /// Returns `Some(i8)` for int variant, decimal variant(scale=0) that fit
in `i8` range,
/// `None` for other variants or values that would overflow.
///
/// # Examples
///
/// ```
- /// use parquet_variant::Variant;
+ /// use parquet_variant::{Variant, VariantDecimal4};
///
/// // you can read an int64 variant into an i8 if it fits
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_int8(), Some(123i8));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int8(), Some(0));
+ /// // or from a decimal variant that fit in i8 range
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_int8(), Some(123i8));
///
/// // but not if it would overflow
/// let v3 = Variant::from(1234i64);
/// assert_eq!(v3.as_int8(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
+ /// let v4 = Variant::from("hello");
/// assert_eq!(v4.as_int8(), None);
/// ```
pub fn as_int8(&self) -> Option<i8> {
- self.as_num()
+ match *self {
+ Variant::Int8(i) => Some(i),
+ Variant::Int16(i) => i.try_into().ok(),
+ Variant::Int32(i) => i.try_into().ok(),
+ Variant::Int64(i) => i.try_into().ok(),
+ Variant::Decimal4(d) if d.scale() == 0 =>
d.integer().try_into().ok(),
Review Comment:
This misses the decimals where the fraction side is 0, e.g. 123.00
We could add an API for `VariantDecimalType` in parquet-variant:
```rust
pub fn as_integer(&self) -> Option<$native> {
if self.scale == 0 {
return Some(self.integer);
}
let divisor = <$native>::pow(10, self.scale as u32);
(self.integer % divisor == 0).then(|| self.integer / divisor)
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
