-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[Variant] Align cast logic for variant_get to cast kernel for numeric/bool types #9563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,7 @@ use arrow_select::take::take; | |
| use num_traits::{NumCast, ToPrimitive, cast::AsPrimitive}; | ||
|
|
||
| pub use decimal::{DecimalCast, rescale_decimal}; | ||
| pub use string::cast_single_string_to_boolean_default; | ||
|
|
||
| /// CastOptions provides a way to override the default cast behaviors | ||
| #[derive(Debug, Clone, PartialEq, Eq, Hash)] | ||
|
|
@@ -2464,7 +2465,7 @@ where | |
| R::Native: NumCast, | ||
| { | ||
| from.try_unary(|value| { | ||
| num_traits::cast::cast::<T::Native, R::Native>(value).ok_or_else(|| { | ||
| num_cast::<T::Native, R::Native>(value).ok_or_else(|| { | ||
| ArrowError::CastError(format!( | ||
| "Can't cast value {:?} to type {}", | ||
| value, | ||
|
|
@@ -2474,6 +2475,17 @@ where | |
| }) | ||
| } | ||
|
|
||
| /// Natural cast between numeric types | ||
| /// Return None if the input `value` can't be casted to type `O`. | ||
| #[inline] | ||
| pub fn num_cast<I, O>(value: I) -> Option<O> | ||
| where | ||
| I: NumCast, | ||
| O: NumCast, | ||
| { | ||
| num_traits::cast::cast::<I, O>(value) | ||
| } | ||
|
|
||
| // Natural cast between numeric types | ||
| // If the value of T can't be casted to R, it will be converted to null | ||
| fn numeric_cast<T, R>(from: &PrimitiveArray<T>) -> PrimitiveArray<R> | ||
|
|
@@ -2483,7 +2495,7 @@ where | |
| T::Native: NumCast, | ||
| R::Native: NumCast, | ||
| { | ||
| from.unary_opt::<_, R>(num_traits::cast::cast::<T::Native, R::Native>) | ||
| from.unary_opt::<_, R>(num_cast::<T::Native, R::Native>) | ||
| } | ||
|
|
||
| fn cast_numeric_to_binary<FROM: ArrowPrimitiveType, O: OffsetSizeTrait>( | ||
|
|
@@ -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() | ||
| } | ||
|
|
||
| /// Cast Boolean types to numeric | ||
| /// | ||
| /// `false` returns 0 while `true` returns 1 | ||
|
|
@@ -2575,11 +2594,8 @@ where | |
| let iter = (0..from.len()).map(|i| { | ||
| if from.is_null(i) { | ||
| None | ||
| } else if from.value(i) { | ||
| // a workaround to cast a primitive to T::Native, infallible | ||
| num_traits::cast::cast(1) | ||
| } else { | ||
| Some(T::default_value()) | ||
| single_bool_to_numeric::<T::Native>(from.value(i)) | ||
| } | ||
| }); | ||
| // Benefit: | ||
|
|
@@ -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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we could have an impedance mismatch between
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, |
||
| } else { | ||
| Some(O::default()) | ||
| } | ||
| } | ||
|
|
||
| /// Helper function to cast from one `BinaryArray` or 'LargeBinaryArray' to 'FixedSizeBinaryArray'. | ||
| fn cast_binary_to_fixed_size_binary<O: OffsetSizeTrait>( | ||
| array: &dyn Array, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware that rust or SQL allows coercing numbers to bool?
But arrow-cast already allows this, I guess?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meanwhile, this should probably compare directly against
I::ZERO?(to match the other direction, below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, arrorw-cast already allows this, This is the current equivalent conversion for arrow-cast.
arrow-rs/arrow-cast/src/cast/mod.rs
Lines 2534 to 2548 in 88422cb