sdf-jkl commented on code in PR #10157:
URL: https://github.com/apache/arrow-rs/pull/10157#discussion_r3623401981
##########
parquet-variant/src/variant.rs:
##########
@@ -837,190 +785,205 @@ 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`.
+ /// Converts this variant to an `i8` for int variants if possible.
///
- /// 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.
- ///
- /// 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));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(1234i64);
- /// assert_eq!(v3.as_int8(), None);
+ /// let v4 = Variant::from(1234i64);
+ /// assert_eq!(v4.as_int8(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int8(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.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) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i16` if possible.
///
- /// Returns `Some(i16)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i16` range
+ /// Returns `Some(i16)` for int variants, decimal variants(scale=0) that
fit in `i16` 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 i16 if it fits
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_int16(), Some(123i16));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int16(), Some(0));
+ /// // or from a decimal variant
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_int16(), Some(123i16));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
Review Comment:
The new example block got copy-pasted into all eight accessors without
updating the method under test. This one is in `as_int16`'s docs but asserts on
`as_int8`.
```suggestion
/// assert_eq!(v3.as_int16(), Some(1i16));
```
##########
parquet-variant/src/variant.rs:
##########
@@ -1073,77 +1031,67 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_u16(), Some(123u16));
///
- /// // or a Decimal4 with scale 0 into u8
- /// let d = VariantDecimal4::try_new(u16::MAX as i32, 0).unwrap();
- /// let v2 = Variant::from(d);
- /// assert_eq!(v2.as_u16(), Some(u16::MAX));
- ///
- /// // or a variant that decimal with scale not equal to zero
- /// let d = VariantDecimal4::try_new(123, 2).unwrap();
- /// let v3 = Variant::from(d);
- /// assert_eq!(v3.as_u16(), Some(1));
+ /// // or from decimal variant with scale = 0
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_u16(), Some(123u16));
///
- /// // or from boolean variant
- /// let v4= Variant::BooleanFalse;
- /// assert_eq!(v4.as_u16(), Some(0));
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
Review Comment:
```suggestion
/// assert_eq!(v3.as_u16(), Some(1u16));
```
##########
parquet-variant/src/variant.rs:
##########
@@ -1155,52 +1103,32 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_u64(), Some(123u64));
///
- /// // or a Decimal16 with scale 0 into u8
- /// let d = VariantDecimal16::try_new(u64::MAX as i128, 0).unwrap();
+ /// // or from a variant decimal with scale = 0
+ /// let d = VariantDecimal16::try_new(1, 0).unwrap();
/// let v2 = Variant::from(d);
- /// assert_eq!(v2.as_u64(), Some(u64::MAX));
- ///
- /// // or a variant that decimal with scale not equal to zero
- /// let d = VariantDecimal16::try_new(123, 2).unwrap();
- /// let v3 = Variant::from(d);
- /// assert_eq!(v3.as_u64(), Some(1));
+ /// assert_eq!(v2.as_u64(), Some(1u64));
///
- /// // or from boolean variant
- /// let v4 = Variant::BooleanFalse;
- /// assert_eq!(v4.as_u64(), Some(0));
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal16::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
Review Comment:
```suggestion
/// assert_eq!(v3.as_u64(), Some(1u64));
```
##########
parquet-variant/src/variant/decimal.rs:
##########
@@ -174,6 +178,14 @@ macro_rules! impl_variant_decimal {
fn scale(&self) -> u8 {
self.scale()
}
+
+ 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)
+ }
Review Comment:
No unit tests pin the new behavior. Worth adding:
- `1234` @ scale 2 → `None`, the guard itself
- `-100` @ scale 2 → `Some(-1)`, `-1234` @ scale 2 → `None`
- divisible but overflowing the target: `VariantDecimal8::try_new(100_000,
2).as_int8()` → `None`
- `VariantDecimal4::try_new(0, 9)` → `Some(0)`, the max-scale boundary
- one end-to-end shred test: `Variant::Decimal4(1200, 2)` landing in an
`Int32` `typed_value` column instead of falling back to `value`
##########
parquet-variant/src/variant.rs:
##########
@@ -663,17 +601,27 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v1 = Variant::from(datetime);
/// assert_eq!(v1.as_timestamp_ntz_micros(), Some(datetime));
///
+ /// // or from a non-UTC-adjusted timestamp nano variant that can fit into
microsecond range
+ /// let datetime_nanos = NaiveDate::from_ymd_opt(2026, 7, 15)
+ /// .unwrap()
+ /// .and_hms_nano_opt(12, 34, 56, 123456000)
+ /// .unwrap();
+ /// // construct the variant directly, because variant::from will treat
this into a timestamp variant
+ /// let v2 = Variant::TimestampNtzNanos(datetime_nanos);
+ /// assert_eq!(v2.as_timestamp_ntz_micros(), Some(datetime_nanos));
+ ///
/// // but not for other variants.
/// let datetime_nanos = NaiveDate::from_ymd_opt(2025, 8, 14)
/// .unwrap()
/// .and_hms_nano_opt(12, 33, 54, 123456789)
/// .unwrap();
- /// let v2 = Variant::from(datetime_nanos);
- /// assert_eq!(v2.as_timestamp_micros(), None);
+ /// let v3 = Variant::from(datetime_nanos);
+ /// assert_eq!(v3.as_timestamp_micros(), None);
Review Comment:
These are `as_timestamp_ntz_micros`'s docs but the assert calls
`as_timestamp_micros`. Fixing it makes the example actually exercise the
`nanosecond() % 1000 == 0` guard below, which has no coverage today. Worth a
unit test for that guard as well.
```suggestion
/// assert_eq!(v3.as_timestamp_ntz_micros(), None);
```
##########
parquet-variant/src/variant.rs:
##########
@@ -837,190 +785,205 @@ 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`.
+ /// Converts this variant to an `i8` for int variants if possible.
///
- /// 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.
- ///
- /// 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));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(1234i64);
- /// assert_eq!(v3.as_int8(), None);
+ /// let v4 = Variant::from(1234i64);
+ /// assert_eq!(v4.as_int8(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int8(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.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) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i16` if possible.
///
- /// Returns `Some(i16)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i16` range
+ /// Returns `Some(i16)` for int variants, decimal variants(scale=0) that
fit in `i16` 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 i16 if it fits
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_int16(), Some(123i16));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int16(), Some(0));
+ /// // or from a decimal variant
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_int16(), Some(123i16));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(123456i64);
- /// assert_eq!(v3.as_int16(), None);
+ /// let v4 = Variant::from(123456i64);
+ /// assert_eq!(v4.as_int16(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int16(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.as_int16(), None);
/// ```
pub fn as_int16(&self) -> Option<i16> {
- self.as_num()
+ match *self {
+ Variant::Int8(i) => Some(i as i16),
+ Variant::Int16(i) => Some(i),
+ Variant::Int32(i) => i.try_into().ok(),
+ Variant::Int64(i) => i.try_into().ok(),
+ Variant::Decimal4(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i32` if possible.
///
- /// Returns `Some(i32)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i32` range
+ /// Returns `Some(i32)` for int variants, decimal variants(scale=0) that
fit in `i32` 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 i32 if it fits
- /// let v1 = Variant::from(123i64);
+ /// // you can read an int32 variant into an i32
+ /// let v1 = Variant::from(123i32);
/// assert_eq!(v1.as_int32(), Some(123i32));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int32(), Some(0));
+ /// // or from an int64 if it fits
+ /// let v2 = Variant::from(1231i64);
+ /// assert_eq!(v2.as_int32(), Some(1231i32));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
+ ///
+ /// // or from decimal variant(scale=0)
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v4 = Variant::from(d);
+ /// assert_eq!(v4.as_int32(), Some(123i32));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(12345678901i64);
- /// assert_eq!(v3.as_int32(), None);
+ /// let v5 = Variant::from(1234567890123i64);
+ /// assert_eq!(v5.as_f32(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int32(), None);
+ /// let v6 = Variant::from("hello");
+ /// assert_eq!(v6.as_f32(), None)
/// ```
pub fn as_int32(&self) -> Option<i32> {
- self.as_num()
+ match *self {
+ Variant::Int8(i) => Some(i as i32),
+ Variant::Int16(i) => Some(i as i32),
+ Variant::Int32(i) => Some(i),
+ Variant::Int64(i) => i.try_into().ok(),
+ Variant::Decimal4(d) => d.as_integer(),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
- /// Converts this variant to an `i64` if possible.
+ /// Converts this variant to an `i64`.
///
- /// Returns `Some(i64)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i64` range
+ /// Returns `Some(i64)` for int variants or decimal variants(scale=0) that
fit in `i64` 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 i64
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_int64(), Some(123i64));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int64(), Some(0));
+ /// // or from a decimal variant
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_int64(), Some(123i64));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
Review Comment:
```suggestion
/// assert_eq!(v3.as_int64(), Some(1i64));
```
##########
parquet-variant/src/variant.rs:
##########
@@ -837,190 +785,205 @@ 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`.
+ /// Converts this variant to an `i8` for int variants if possible.
///
- /// 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.
- ///
- /// 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));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(1234i64);
- /// assert_eq!(v3.as_int8(), None);
+ /// let v4 = Variant::from(1234i64);
+ /// assert_eq!(v4.as_int8(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int8(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.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) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i16` if possible.
///
- /// Returns `Some(i16)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i16` range
+ /// Returns `Some(i16)` for int variants, decimal variants(scale=0) that
fit in `i16` 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 i16 if it fits
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_int16(), Some(123i16));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int16(), Some(0));
+ /// // or from a decimal variant
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_int16(), Some(123i16));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(123456i64);
- /// assert_eq!(v3.as_int16(), None);
+ /// let v4 = Variant::from(123456i64);
+ /// assert_eq!(v4.as_int16(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int16(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.as_int16(), None);
/// ```
pub fn as_int16(&self) -> Option<i16> {
- self.as_num()
+ match *self {
+ Variant::Int8(i) => Some(i as i16),
+ Variant::Int16(i) => Some(i),
+ Variant::Int32(i) => i.try_into().ok(),
+ Variant::Int64(i) => i.try_into().ok(),
+ Variant::Decimal4(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i32` if possible.
///
- /// Returns `Some(i32)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i32` range
+ /// Returns `Some(i32)` for int variants, decimal variants(scale=0) that
fit in `i32` 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 i32 if it fits
- /// let v1 = Variant::from(123i64);
+ /// // you can read an int32 variant into an i32
+ /// let v1 = Variant::from(123i32);
/// assert_eq!(v1.as_int32(), Some(123i32));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int32(), Some(0));
+ /// // or from an int64 if it fits
+ /// let v2 = Variant::from(1231i64);
+ /// assert_eq!(v2.as_int32(), Some(1231i32));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
+ ///
+ /// // or from decimal variant(scale=0)
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v4 = Variant::from(d);
+ /// assert_eq!(v4.as_int32(), Some(123i32));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(12345678901i64);
- /// assert_eq!(v3.as_int32(), None);
+ /// let v5 = Variant::from(1234567890123i64);
+ /// assert_eq!(v5.as_f32(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int32(), None);
+ /// let v6 = Variant::from("hello");
+ /// assert_eq!(v6.as_f32(), None)
Review Comment:
```suggestion
/// assert_eq!(v6.as_int32(), None)
```
##########
parquet-variant/src/variant.rs:
##########
@@ -1032,36 +995,31 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_u8(), Some(123u8));
///
- /// // or a Decimal4 with scale 0 into u8
- /// let d = VariantDecimal4::try_new(26, 0).unwrap();
- /// let v2 = Variant::from(d);
- /// assert_eq!(v2.as_u8(), Some(26u8));
+ /// // or from decimal variant with scale = 0
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_u8(), Some(123u8));
///
- /// // or a variant that decimal with scale not equal to zero
- /// let d = VariantDecimal4::try_new(123, 2).unwrap();
- /// let v3 = Variant::from(d);
- /// assert_eq!(v3.as_u8(), Some(1));
- ///
- /// // or from boolean variant
- /// let v4 = Variant::BooleanFalse;
- /// assert_eq!(v4.as_u8(), Some(0));
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
Review Comment:
```suggestion
/// assert_eq!(v3.as_u8(), Some(1u8));
```
##########
parquet-variant/src/variant.rs:
##########
@@ -837,190 +785,205 @@ 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`.
+ /// Converts this variant to an `i8` for int variants if possible.
///
- /// 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.
- ///
- /// 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));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(1234i64);
- /// assert_eq!(v3.as_int8(), None);
+ /// let v4 = Variant::from(1234i64);
+ /// assert_eq!(v4.as_int8(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int8(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.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) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i16` if possible.
///
- /// Returns `Some(i16)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i16` range
+ /// Returns `Some(i16)` for int variants, decimal variants(scale=0) that
fit in `i16` 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 i16 if it fits
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_int16(), Some(123i16));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int16(), Some(0));
+ /// // or from a decimal variant
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_int16(), Some(123i16));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(123456i64);
- /// assert_eq!(v3.as_int16(), None);
+ /// let v4 = Variant::from(123456i64);
+ /// assert_eq!(v4.as_int16(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int16(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.as_int16(), None);
/// ```
pub fn as_int16(&self) -> Option<i16> {
- self.as_num()
+ match *self {
+ Variant::Int8(i) => Some(i as i16),
+ Variant::Int16(i) => Some(i),
+ Variant::Int32(i) => i.try_into().ok(),
+ Variant::Int64(i) => i.try_into().ok(),
+ Variant::Decimal4(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i32` if possible.
///
- /// Returns `Some(i32)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i32` range
+ /// Returns `Some(i32)` for int variants, decimal variants(scale=0) that
fit in `i32` 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 i32 if it fits
- /// let v1 = Variant::from(123i64);
+ /// // you can read an int32 variant into an i32
+ /// let v1 = Variant::from(123i32);
/// assert_eq!(v1.as_int32(), Some(123i32));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int32(), Some(0));
+ /// // or from an int64 if it fits
+ /// let v2 = Variant::from(1231i64);
+ /// assert_eq!(v2.as_int32(), Some(1231i32));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
Review Comment:
```suggestion
/// assert_eq!(v3.as_int32(), Some(1i32));
```
##########
parquet-variant/src/variant.rs:
##########
@@ -1073,77 +1031,67 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_u16(), Some(123u16));
///
- /// // or a Decimal4 with scale 0 into u8
- /// let d = VariantDecimal4::try_new(u16::MAX as i32, 0).unwrap();
- /// let v2 = Variant::from(d);
- /// assert_eq!(v2.as_u16(), Some(u16::MAX));
- ///
- /// // or a variant that decimal with scale not equal to zero
- /// let d = VariantDecimal4::try_new(123, 2).unwrap();
- /// let v3 = Variant::from(d);
- /// assert_eq!(v3.as_u16(), Some(1));
+ /// // or from decimal variant with scale = 0
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_u16(), Some(123u16));
///
- /// // or from boolean variant
- /// let v4= Variant::BooleanFalse;
- /// assert_eq!(v4.as_u16(), Some(0));
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not a variant that can't fit into the range
- /// let v5 = Variant::from(-1);
- /// assert_eq!(v5.as_u16(), None);
+ /// let v4 = Variant::from(-1);
+ /// assert_eq!(v4.as_u16(), None);
///
/// // or not a variant that cannot be cast into an integer
- /// let v6 = Variant::from("hello!");
- /// assert_eq!(v6.as_u16(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.as_u16(), None);
/// ```
pub fn as_u16(&self) -> Option<u16> {
- self.as_num()
+ Self::convert_to_unsigned_num(self)
}
/// Converts this variant to an `u32` if possible.
///
- /// Returns `Some(u32)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `u32` range
+ /// Returns `Some(u32)` for int variants, decimal variants(scale=0) that
fit in `u32`,
/// `None` for other variants or values that would overflow.
///
/// # Examples
///
/// ```
- /// use parquet_variant::{Variant, VariantDecimal8};
+ /// use parquet_variant::{Variant, VariantDecimal4, VariantDecimal8};
///
/// // you can read an int64 variant into an u32
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_u32(), Some(123u32));
///
- /// // or a Decimal4 with scale 0 into u8
- /// let d = VariantDecimal8::try_new(u32::MAX as i64, 0).unwrap();
+ /// // or from decimal variant with scale = 0
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
/// let v2 = Variant::from(d);
- /// assert_eq!(v2.as_u32(), Some(u32::MAX));
+ /// assert_eq!(v2.as_u32(), Some(123u32));
///
- /// // or a variant that decimal with scale not equal to zero
- /// let d = VariantDecimal8::try_new(123, 2).unwrap();
- /// let v3 = Variant::from(d);
- /// assert_eq!(v3.as_u32(), Some(1));
- ///
- /// // or from boolean variant
- /// let v4 = Variant::BooleanFalse;
- /// assert_eq!(v4.as_u32(), Some(0));
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
Review Comment:
```suggestion
/// assert_eq!(v3.as_u32(), Some(1u32));
```
##########
parquet-variant/src/variant.rs:
##########
@@ -837,190 +785,205 @@ 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`.
+ /// Converts this variant to an `i8` for int variants if possible.
///
- /// 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.
- ///
- /// 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,
Review Comment:
The one-line summaries still state the old contract. `as_integer` now
accepts any decimal with no fractional part, not just `scale=0`.
Same on lines 835, 880, 929, 986, 1022, 1058, 1094.
```suggestion
/// Returns `Some(i8)` for int variants, and decimal variants with no
fractional part, that fit in `i8` range,
```
##########
parquet-variant/src/variant/decimal.rs:
##########
@@ -104,6 +104,10 @@ pub trait VariantDecimalType: Into<super::Variant<'static,
'static>> {
/// Returns the scale (number of digits after the decimal point)
fn scale(&self) -> u8;
+
+ /// Returns the decimal as an integer, if scale is 0 or the unscaled value
is divisible by 10^scale.
+ /// None for other values.
+ fn as_integer(&self) -> Option<Self::Native>;
Review Comment:
Nit: `as_integer` ends up trait-only. `integer()` and `scale()` have
inherent methods on `VariantDecimal4/8/16` that the trait impl forwards to, but
this one is implemented directly in the `impl VariantDecimalType` block. So
`d.as_integer()` requires importing `VariantDecimalType` while `d.integer()`
doesn't.
##########
parquet-variant/src/variant.rs:
##########
@@ -837,190 +785,205 @@ 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`.
+ /// Converts this variant to an `i8` for int variants if possible.
///
- /// 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.
- ///
- /// 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));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(1234i64);
- /// assert_eq!(v3.as_int8(), None);
+ /// let v4 = Variant::from(1234i64);
+ /// assert_eq!(v4.as_int8(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int8(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.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) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i16` if possible.
///
- /// Returns `Some(i16)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i16` range
+ /// Returns `Some(i16)` for int variants, decimal variants(scale=0) that
fit in `i16` 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 i16 if it fits
/// let v1 = Variant::from(123i64);
/// assert_eq!(v1.as_int16(), Some(123i16));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int16(), Some(0));
+ /// // or from a decimal variant
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v2 = Variant::from(d);
+ /// assert_eq!(v2.as_int16(), Some(123i16));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(123456i64);
- /// assert_eq!(v3.as_int16(), None);
+ /// let v4 = Variant::from(123456i64);
+ /// assert_eq!(v4.as_int16(), None);
///
/// // or if the variant cannot be cast into an integer
- /// let v4 = Variant::from("hello!");
- /// assert_eq!(v4.as_int16(), None);
+ /// let v5 = Variant::from("hello");
+ /// assert_eq!(v5.as_int16(), None);
/// ```
pub fn as_int16(&self) -> Option<i16> {
- self.as_num()
+ match *self {
+ Variant::Int8(i) => Some(i as i16),
+ Variant::Int16(i) => Some(i),
+ Variant::Int32(i) => i.try_into().ok(),
+ Variant::Int64(i) => i.try_into().ok(),
+ Variant::Decimal4(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal8(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ Variant::Decimal16(d) => d.as_integer().and_then(|i|
i.try_into().ok()),
+ _ => None,
+ }
}
/// Converts this variant to an `i32` if possible.
///
- /// Returns `Some(i32)` for boolean and numeric variants(integers,
floating-point,
- /// and decimals with scale 0) that fit in `i32` range
+ /// Returns `Some(i32)` for int variants, decimal variants(scale=0) that
fit in `i32` 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 i32 if it fits
- /// let v1 = Variant::from(123i64);
+ /// // you can read an int32 variant into an i32
+ /// let v1 = Variant::from(123i32);
/// assert_eq!(v1.as_int32(), Some(123i32));
///
- /// // or from boolean variant
- /// let v2 = Variant::BooleanFalse;
- /// assert_eq!(v2.as_int32(), Some(0));
+ /// // or from an int64 if it fits
+ /// let v2 = Variant::from(1231i64);
+ /// assert_eq!(v2.as_int32(), Some(1231i32));
+ ///
+ /// // or from a decimal variant that unscaled value is divisible by
10^scale
+ /// let d = VariantDecimal4::try_new(100, 2).unwrap();
+ /// let v3 = Variant::from(d);
+ /// assert_eq!(v3.as_int8(), Some(1i8));
+ ///
+ /// // or from decimal variant(scale=0)
+ /// let d = VariantDecimal4::try_new(123, 0).unwrap();
+ /// let v4 = Variant::from(d);
+ /// assert_eq!(v4.as_int32(), Some(123i32));
///
/// // but not if it would overflow
- /// let v3 = Variant::from(12345678901i64);
- /// assert_eq!(v3.as_int32(), None);
+ /// let v5 = Variant::from(1234567890123i64);
+ /// assert_eq!(v5.as_f32(), None);
Review Comment:
These two asserts in `as_int32`'s docs call `as_f32`. `as_f32` only matches
`Variant::Float`, so both an `Int64` and a `"hello"` hit its catch-all and
return `None`. The assertions pass without ever touching `as_int32`.
Net effect: `as_int32`'s overflow rejection (`1234567890123i64` doesn't fit
`i32`) and its non-numeric rejection are both untested.
```suggestion
/// assert_eq!(v5.as_int32(), None);
```
--
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]
