zhuqi-lucas commented on code in PR #9250:
URL: https://github.com/apache/arrow-rs/pull/9250#discussion_r2721414451
##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -795,83 +795,17 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
/// ```
/// # Inlining and Endianness
///
- /// - We start by calling `.to_le_bytes()` on the `raw` `u128`, because
Rust’s native in‑memory
- /// representation is little‑endian on x86/ARM.
- /// - We extract the low 32 bits numerically (`raw as u32`)—this step is
endianness‑free.
- /// - We copy the 12 bytes of inline data (original order) into
`buf[0..12]`.
- /// - We serialize `length` as big‑endian into `buf[12..16]`.
- /// - Finally, `u128::from_be_bytes(buf)` treats `buf[0]` as the most
significant byte
- /// and `buf[15]` as the least significant, producing a `u128` whose
integer value
- /// directly encodes “inline data then length” in big‑endian form.
- ///
- /// This ensures that a simple `u128` comparison is equivalent to the
desired
- /// lexicographical comparison of the inline bytes followed by length.
+ /// This function uses target-endian independent bitwise operations to
construct a 128-bit key:
+ /// - `raw.to_be() << 32` effectively clears the length bits and shifts
the 12-byte inline data
+ /// into the high 96 bits in Big-Endian order. This ensures the first
byte of the string
+ /// is the most significant byte of the resulting `u128`.
+ /// - `raw.to_le() as u32` extracts the length as a native-endian integer,
which is then
+ /// placed in the low 32 bits.
+ /// - The combination ensures that a single `u128` comparison correctly
orders by inline data
+ /// first (lexicographically), then by length (numerically).
#[inline(always)]
pub fn inline_key_fast(raw: u128) -> u128 {
Review Comment:
The datafusion seems also use this function, we also need to run datafusion
based this change to make sure it's not breaking something.
##########
arrow-ord/src/cmp.rs:
##########
@@ -573,36 +573,84 @@ impl<'a, T: ByteViewType> ArrayOrd for &'a
GenericByteViewArray<T> {
return l_view == r_view;
}
+ // Fast path for same view (and both inlined)
+ if l_view == r_view && *l_view as u32 <= 12 {
+ return true;
+ }
+
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.
+ // Lengths differ
if l_len != r_len {
return false;
}
- if l_len == 0 && r_len == 0 {
+
+ // Both are empty
+ if l_len == 0 {
Review Comment:
I found this will compile to same code finally, but it's more clean here, i
agree.
--
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]
