Use ArrayAccessor and FromIterator in Cast Kernels #2169
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2169 +/- ##
==========================================
- Coverage 82.86% 82.61% -0.25%
==========================================
Files 237 239 +2
Lines 61429 62272 +843
==========================================
+ Hits 50902 51447 +545
- Misses 10527 10825 +298
Help us with your feedback. Take ten seconds to tell us how you rate us. |
arrow/src/compute/kernels/cast.rs
Outdated
| |v| { | ||
| // with_precision_and_scale validates the | ||
| // value is within range for the output precision | ||
| ((num::ToPrimitive::to_f64(&v).unwrap()) * mul) as i128 |
There was a problem hiding this comment.
I think this is now a checked conversion where previously it wasn't?
There was a problem hiding this comment.
Hmm, but I cannot use v as f64 here.
There was a problem hiding this comment.
error[E0605]: non-primitive cast: `<T as native::ArrowPrimitiveType>::Native` as `i128`
--> arrow/src/compute/kernels/cast.rs:300:42
|
300 | cast_primitive_to_decimal(array, |v| v as i128 * mul, precision, scale)
| ^^^^^^^^^ an `as` expression can only be used to convert between primitive types or to coerce to a specific trait object
There was a problem hiding this comment.
Added helper functions to ArrowPrimitiveType.
There was a problem hiding this comment.
Why not use AsPrimitive instead of ToPrimitive?
There was a problem hiding this comment.
I'm not aware of AsPrimitive trait.
arrow/src/compute/kernels/cast.rs
Outdated
| // value is within range for the output precision | ||
| cast_primitive_to_decimal( | ||
| array, | ||
| |v| v.to_isize().unwrap() as i128 * mul, |
arrow/src/datatypes/native.rs
Outdated
| @@ -79,6 +79,18 @@ pub trait ArrowNativeType: | |||
| None | |||
| } | |||
|
|
|||
| /// Convert native type to i128. | |||
| #[inline] | |||
| fn to_i128(&self) -> Option<i128> { | |||
There was a problem hiding this comment.
I'm not a massive fan of more of these, when I hope to remove them in favour of the standard num traits...
arrow/src/compute/kernels/cast.rs
Outdated
| |v| { | ||
| // with_precision_and_scale validates the | ||
| // value is within range for the output precision | ||
| ((num::ToPrimitive::to_f64(&v).unwrap()) * mul) as i128 |
There was a problem hiding this comment.
Why not use AsPrimitive instead of ToPrimitive?
|
Benchmark runs are scheduled for baseline = dfc7ef8 and contender = 445283c. 445283c is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2137.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?