klion26 commented on code in PR #9563:
URL: https://github.com/apache/arrow-rs/pull/9563#discussion_r2958012088
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -2540,16 +2552,23 @@ where
for i in 0..from.len() {
if from.is_null(i) {
b.append_null();
- } else if from.value(i) != T::default_value() {
- b.append_value(true);
} else {
- b.append_value(false);
+ b.append_value(cast_num_to_bool::<T::Native>(from.value(i)));
}
}
Ok(b.finish())
}
+/// Cast numeric types to boolean
+#[inline]
+pub fn cast_num_to_bool<I>(value: I) -> bool
+where
+ I: Default + PartialEq,
+{
+ value != I::default()
Review Comment:
Yes, arrorw-cast already allows this, This is the current equivalent
conversion for arrow-cast.
https://github.com/apache/arrow-rs/blob/88422cbdcbfa8f4e2411d66578dd3582fafbf2a1/arrow-cast/src/cast/mod.rs#L2534-L2548
##########
parquet-variant/src/variant.rs:
##########
@@ -499,6 +502,14 @@ impl<'m, 'v> Variant<'m, 'v> {
match self {
Variant::BooleanTrue => Some(true),
Variant::BooleanFalse => Some(false),
+ Variant::Int8(i) => Some(cast_num_to_bool::<i8>(*i)),
Review Comment:
Yes, the type annotations can be removed.
##########
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.
+#[inline]
+pub fn single_bool_to_numeric<O>(value: bool) -> Option<O>
+where
+ O: num_traits::NumCast + Default,
+{
+ if value {
+ // a workaround to cast a primitive to type O, infallible
+ num_traits::cast::cast(1)
Review Comment:
Yes, `ArrowNativeTypeOp` contains `ONE` and `ZEOR`, but Rust type (i32, etc)
does not have `ZEOR`/`ONE`.
##########
parquet-variant/src/variant.rs:
##########
@@ -499,6 +502,14 @@ impl<'m, 'v> Variant<'m, 'v> {
match self {
Variant::BooleanTrue => Some(true),
Variant::BooleanFalse => Some(false),
+ Variant::Int8(i) => Some(cast_num_to_bool::<i8>(*i)),
+ Variant::Int16(i) => Some(cast_num_to_bool::<i16>(*i)),
+ Variant::Int32(i) => Some(cast_num_to_bool::<i32>(*i)),
+ Variant::Int64(i) => Some(cast_num_to_bool::<i64>(*i)),
+ Variant::Float(f) => Some(cast_num_to_bool::<f32>(*f)),
+ Variant::Double(d) => Some(cast_num_to_bool::<f64>(*d)),
+ Variant::ShortString(s) =>
cast_single_string_to_boolean_default(s.0),
Review Comment:
`s` here is `&ShortString`, `*s` will give `ShortString`, we need to use
`&**s` to convert to `&str`, adjusted to `s.as_ref()`
--
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]
