klion26 commented on code in PR #10157:
URL: https://github.com/apache/arrow-rs/pull/10157#discussion_r3621955626


##########
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:
   Fixed in the new commit e1d5fe3f6dd4af48d3d49cedb5517e400e6b7f49



-- 
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]

Reply via email to