scovich commented on code in PR #9563:
URL: https://github.com/apache/arrow-rs/pull/9563#discussion_r3003415413
##########
parquet-variant/src/variant.rs:
##########
@@ -760,10 +779,36 @@ impl<'m, 'v> Variant<'m, 'v> {
}
}
+ /// Converts a boolean or numeric variant(integers, floating-point, and
decimals with scale 0)
+ /// 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),
Review Comment:
I'm surprised type inference doesn't pick up that `T`? Does it fail to
compile if you do this?
```suggestion
Variant::Int8(i) => num_cast(i),
```
##########
arrow-cast/src/cast/string.rs:
##########
@@ -401,25 +401,37 @@ where
let output_array = array
.iter()
.map(|value| match value {
- Some(value) => match value.to_ascii_lowercase().trim() {
- "t" | "tr" | "tru" | "true" | "y" | "ye" | "yes" | "on" | "1"
=> Ok(Some(true)),
- "f" | "fa" | "fal" | "fals" | "false" | "n" | "no" | "of" |
"off" | "0" => {
- Ok(Some(false))
- }
- invalid_value => match cast_options.safe {
- true => Ok(None),
- false => Err(ArrowError::CastError(format!(
- "Cannot cast value '{invalid_value}' to value of
Boolean type",
- ))),
- },
- },
+ Some(value) => cast_single_string_to_boolean(value, cast_options),
None => Ok(None),
})
.collect::<Result<BooleanArray, _>>()?;
Ok(Arc::new(output_array))
}
+fn cast_single_string_to_boolean(
Review Comment:
Does this need `#[inline]` directive to avoid a performance hazard?
--
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]
