Dandandan commented on code in PR #7731:
URL: https://github.com/apache/arrow-rs/pull/7731#discussion_r2161241759
##########
arrow-ord/src/cmp.rs:
##########
@@ -565,24 +565,39 @@ impl<'a, T: ByteViewType> ArrayOrd for &'a
GenericByteViewArray<T> {
/// Item.0 is the array, Item.1 is the index
type Item = (&'a GenericByteViewArray<T>, usize);
+ #[inline(always)]
fn is_eq(l: Self::Item, r: Self::Item) -> bool {
- // # Safety
- // The index is within bounds as it is checked in value()
let l_view = unsafe { l.0.views().get_unchecked(l.1) };
- let l_len = *l_view as u32;
-
let r_view = unsafe { r.0.views().get_unchecked(r.1) };
+ if l.0.data_buffers().is_empty() && r.0.data_buffers().is_empty() {
+ // For eq case, we can directly compare the inlined bytes
+ return l_view.cmp(r_view).is_eq();
+ }
+
+ let l_len = *l_view as u32;
let r_len = *r_view as u32;
// This is a fast path for equality check.
// We don't need to look at the actual bytes to determine if they are
equal.
if l_len != r_len {
return false;
}
+ // # Safety
+ // The index is within bounds as it is checked in value()
unsafe { GenericByteViewArray::compare_unchecked(l.0, l.1, r.0,
r.1).is_eq() }
}
+ #[inline(always)]
fn is_lt(l: Self::Item, r: Self::Item) -> bool {
+ if l.0.data_buffers().is_empty() && r.0.data_buffers().is_empty() {
+ let l_view = unsafe { l.0.views().get_unchecked(l.1) };
+ let r_view = unsafe { r.0.views().get_unchecked(r.1) };
+ let l_len = *l_view as u32 as usize;
+ let r_len = *r_view as u32 as usize;
+ let l_bytes = unsafe {
GenericByteViewArray::<T>::inline_value(l_view, l_len) };
Review Comment:
Perhaps the main thing I see is what we could do next is do `inline_value`
on a u128 level rather than &[u8] and compare values.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
