tustvold commented on code in PR #2658:
URL: https://github.com/apache/arrow-rs/pull/2658#discussion_r963524100
##########
arrow/src/compute/kernels/aggregate.rs:
##########
@@ -52,44 +52,7 @@ where
T: ArrowNumericType,
T::Native: ArrowNativeType,
{
- min_max_helper(array, |a, b| (!is_nan(*a) & is_nan(*b)) || a < b)
-}
-
-/// Helper function to perform min/max lambda function on values from a
numeric array.
-#[multiversion]
-#[clone(target = "x86_64+avx")]
-fn min_max_helper<T, F>(array: &PrimitiveArray<T>, cmp: F) -> Option<T::Native>
-where
- T: ArrowNumericType,
- F: Fn(&T::Native, &T::Native) -> bool,
-{
- let null_count = array.null_count();
-
- // Includes case array.len() == 0
- if null_count == array.len() {
- return None;
- }
-
- let data = array.data();
- let m = array.values();
- let mut n;
-
- if null_count == 0 {
- // optimized path for arrays without null values
- n = m[1..]
- .iter()
- .fold(m[0], |max, item| if cmp(&max, item) { *item } else { max });
- } else {
- n = T::default_value();
- let mut has_value = false;
- for (i, item) in m.iter().enumerate() {
- if data.is_valid(i) && (!has_value || cmp(&n, item)) {
- has_value = true;
- n = *item
- }
- }
- }
- Some(n)
+ min_max_helper::<T::Native, _, _>(array, |a, b| (!is_nan(*a) & is_nan(*b))
|| a < b)
Review Comment:
It occurs to me that this should probably be using the same total_cmp as the
sort logic
--
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]
