scovich commented on code in PR #9563:
URL: https://github.com/apache/arrow-rs/pull/9563#discussion_r2956091018
##########
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:
I wasn't aware that rust or SQL allows coercing numbers to bool?
But arrow-cast already allows this, I guess?
##########
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:
Are these type annotations actually necessary? Seems like the compiler would
infer them easily?
##########
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:
Seems like `T::Native` already provides `ONE` and `ZERO` as constants?
https://docs.rs/arrow/latest/arrow/array/trait.ArrowNativeTypeOp.html
##########
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:
ShortString provides Deref?
```suggestion
Variant::ShortString(s) =>
cast_single_string_to_boolean_default(*s),
```
##########
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:
Meanwhile, this should probably compare directly against `I::ZERO`?
(to match the other direction, below)
##########
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:
I guess we could have an impedance mismatch between `ArrowNativeType`
(arrow) and `NumCast` (num_cast) tho?
--
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]
