sdf-jkl commented on code in PR #9563:
URL: https://github.com/apache/arrow-rs/pull/9563#discussion_r2988767682
##########
parquet-variant/src/variant.rs:
##########
@@ -760,10 +779,34 @@ impl<'m, 'v> Variant<'m, 'v> {
}
}
+ /// Converts a boolean or numeric variant 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: NumCast + Default,
+ {
+ match *self {
+ Variant::BooleanFalse => single_bool_to_numeric::<T>(false),
+ Variant::BooleanTrue => single_bool_to_numeric::<T>(true),
+ Variant::Int8(i) => num_cast::<_, T>(i),
+ Variant::Int16(i) => num_cast::<_, T>(i),
+ Variant::Int32(i) => num_cast::<_, T>(i),
+ Variant::Int64(i) => num_cast::<_, T>(i),
+ Variant::Float(f) => num_cast::<_, T>(f),
+ Variant::Double(d) => num_cast::<_, T>(d),
+ Variant::Decimal4(d) if d.scale() == 0 => num_cast::<_,
T>(d.integer()),
+ Variant::Decimal8(d) if d.scale() == 0 => num_cast::<_,
T>(d.integer()),
+ Variant::Decimal16(d) if d.scale() == 0 => num_cast::<_,
T>(d.integer()),
+ _ => None,
+ }
+ }
+
/// Converts this variant to an `i8` if possible.
///
- /// Returns `Some(i8)` for integer variants that fit in `i8` range,
- /// `None` for non-integer variants or values that would overflow.
+ /// Returns `Some(i8)` for integer variants that fit in `i8` range and
boolean variant,
Review Comment:
```suggestion
/// Returns `Some(i8)` for boolean and numeric variants (integers,
floating-point, and decimals with scale 0) that fit in `i8` range
```
##########
parquet-variant/src/variant.rs:
##########
@@ -967,27 +978,31 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v2 = Variant::from(d);
/// assert_eq!(v2.as_u16(), Some(u16::MAX));
///
+ /// // or from boolean variant
+ /// let v3= Variant::BooleanFalse;
+ /// assert_eq!(v3.as_int8(), Some(0));
Review Comment:
```suggestion
/// assert_eq!(v3.as_u16(), Some(0));
```
##########
parquet-variant/src/variant.rs:
##########
@@ -931,27 +938,31 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v2 = Variant::from(d);
/// assert_eq!(v2.as_u8(), Some(26u8));
///
+ /// // or from boolean variant
+ /// let v3 = Variant::BooleanFalse;
+ /// assert_eq!(v3.as_int8(), Some(0));
Review Comment:
```suggestion
/// assert_eq!(v3.as_u8(), Some(0));
```
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -2518,7 +2647,7 @@ mod test {
#[test]
fn test_error_message_numeric_type_display() {
let mut builder = VariantArrayBuilder::new(1);
- builder.append_variant(Variant::BooleanTrue);
+ builder.append_variant(Variant::Null);
Review Comment:
Using `Variant::Null` to cause error in strict casting will cause conflict
with the PR I'm working on #9599.
Let's see who merges first 😄
##########
parquet-variant-compute/src/variant_get.rs:
##########
Review Comment:
```suggestion
// Request Timestamp with strict casting to force an error
```
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -2518,7 +2647,7 @@ mod test {
#[test]
fn test_error_message_numeric_type_display() {
let mut builder = VariantArrayBuilder::new(1);
- builder.append_variant(Variant::BooleanTrue);
+ builder.append_variant(Variant::Null);
let variant_array: ArrayRef = ArrayRef::from(builder.build());
// Request Boolean with strict casting to force an error
Review Comment:
```suggestion
// Request Float32 with strict casting to force an error
```
##########
parquet-variant/src/variant.rs:
##########
@@ -1003,27 +1018,31 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v2 = Variant::from(d);
/// assert_eq!(v2.as_u32(), Some(u32::MAX));
///
+ /// // or from boolean variant
+ /// let v3 = Variant::BooleanFalse;
+ /// assert_eq!(v3.as_int8(), Some(0));
Review Comment:
```suggestion
/// assert_eq!(v3.as_u32(), Some(0));
```
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -2589,6 +2605,20 @@ where
unsafe { PrimitiveArray::<T>::from_trusted_len_iter(iter) }
}
+/// Cat single bool value to numeric value.
Review Comment:
```suggestion
/// Cast single bool value to numeric value.
```
##########
parquet-variant/src/variant.rs:
##########
@@ -1039,21 +1058,25 @@ impl<'m, 'v> Variant<'m, 'v> {
/// let v2 = Variant::from(d);
/// assert_eq!(v2.as_u64(), Some(u64::MAX));
///
+ /// // or from boolean variant
+ /// let v3 = Variant::BooleanFalse;
+ /// assert_eq!(v3.as_int8(), Some(0));
Review Comment:
```suggestion
/// assert_eq!(v3.as_u64(), Some(0));
```
--
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]
