alamb commented on code in PR #6089:
URL: https://github.com/apache/arrow-rs/pull/6089#discussion_r1687090077
##########
arrow-arith/src/aggregate.rs:
##########
@@ -410,14 +412,55 @@ where
}
}
+/// Helper to compute min/max of [`GenericByteViewArray<T>`].
+/// The specialized min/max leverages the inlined values to compare the byte
views.
+/// `swap_cond` is the condition to swap current min/max with the new value.
+/// For example, `Ordering::Greater` for max and `Ordering::Less` for min.
+fn min_max_view_helper<T: ByteViewType>(
+ array: &GenericByteViewArray<T>,
+ swap_cond: cmp::Ordering,
Review Comment:
The only other thing I can think of doing here is potentially making this a
const
I actually tried doing so https://github.com/XiangpengHao/arrow-rs/pull/2
and it does not seem to make any performance difference
##########
arrow-ord/src/cmp.rs:
##########
@@ -656,6 +656,7 @@ pub fn compare_byte_view<T: ByteViewType>(
///
/// # Safety
/// The left/right_idx must within range of each array
+#[deprecated(note = "Use `GenericByteViewArray::compare_unchecked` instead")]
Review Comment:
I think this is fine. Thank you for the diligence.
--
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]
