alamb commented on code in PR #15348: URL: https://github.com/apache/datafusion/pull/15348#discussion_r2010872746
########## datafusion/physical-plan/src/sorts/cursor.rs: ########## @@ -294,16 +294,44 @@ impl CursorValues for StringViewArray { } fn eq(l: &Self, l_idx: usize, r: &Self, r_idx: usize) -> bool { + // SAFETY: Both l_idx and r_idx are guaranteed to be within bounds, + // and any null-checks are handled in the outer layers. + // Fast path: Compare the lengths (or a proxy of the lengths) before full byte comparison. + + let l_view = unsafe { l.views().get_unchecked(l_idx) }; + let l_len = *l_view as u32; + let r_view = unsafe { r.views().get_unchecked(r_idx) }; + let r_len = *r_view as u32; + if l_len != r_len { + return false; + } + unsafe { GenericByteViewArray::compare_unchecked(l, l_idx, r, r_idx).is_eq() } } fn eq_to_previous(cursor: &Self, idx: usize) -> bool { + // SAFETY: The caller guarantees that idx > 0 and the indices are valid. + // Fast path: Compare the lengths of the current and previous views. + + assert!(idx > 0); + let l_view = unsafe { cursor.views().get_unchecked(idx) }; + let l_len = *l_view as u32; + let r_view = unsafe { cursor.views().get_unchecked(idx - 1) }; + let r_len = *r_view as u32; + if l_len != r_len { + return false; + } + unsafe { GenericByteViewArray::compare_unchecked(cursor, idx, cursor, idx - 1).is_eq() } } fn compare(l: &Self, l_idx: usize, r: &Self, r_idx: usize) -> Ordering { + // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. + // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). + assert!(l_idx < l.len()); Review Comment: `assert`s are left in release builds of rust. ########## datafusion/physical-plan/src/sorts/cursor.rs: ########## @@ -294,16 +294,44 @@ impl CursorValues for StringViewArray { } fn eq(l: &Self, l_idx: usize, r: &Self, r_idx: usize) -> bool { + // SAFETY: Both l_idx and r_idx are guaranteed to be within bounds, + // and any null-checks are handled in the outer layers. + // Fast path: Compare the lengths (or a proxy of the lengths) before full byte comparison. Review Comment: 👍 -- 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...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org