alamb commented on code in PR #2284:
URL: https://github.com/apache/arrow-rs/pull/2284#discussion_r935563939
##########
arrow/src/compute/kernels/cast.rs:
##########
@@ -1247,19 +1251,6 @@ const fn time_unit_multiple(unit: &TimeUnit) -> i64 {
}
}
-/// Number of seconds in a day
Review Comment:
these were already defined in `temporal_conversion.rs` so use those
definitions instead of duplicating them)
##########
arrow/src/compute/kernels/cast.rs:
##########
@@ -1463,35 +1454,28 @@ where
<T as ArrowPrimitiveType>::Native: lexical_core::FromLexical,
{
if cast_options.safe {
Review Comment:
I basically applied the same transformation to all the kernels I modified --
I changed the `safe` path to use `iter().map(|v| v.and_then)` and I changed the
not `safe` path to use `iter().map(|v| v.map().transpose)`
##########
arrow/src/compute/kernels/cast.rs:
##########
@@ -1463,35 +1454,28 @@ where
<T as ArrowPrimitiveType>::Native: lexical_core::FromLexical,
{
if cast_options.safe {
- let iter = (0..from.len()).map(|i| {
- if from.is_null(i) {
- None
- } else {
- lexical_core::parse(from.value(i).as_bytes()).ok()
- }
- });
+ let iter = from
+ .iter()
+ .map(|v| v.and_then(|v| lexical_core::parse(v.as_bytes()).ok()));
// Benefit:
// 20% performance improvement
// Soundness:
// The iterator is trustedLen because it comes from an
`StringArray`.
Ok(unsafe { PrimitiveArray::<T>::from_trusted_len_iter(iter) })
} else {
- let vec = (0..from.len())
- .map(|i| {
- if from.is_null(i) {
- Ok(None)
- } else {
- let string = from.value(i);
Review Comment:
I don't know why this code was creating an owned `String` for each element,
so I removed it. The same pattern was repeated in all the other kernels I
modified in this function
--
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]
