Rich-T-kid commented on code in PR #11407:
URL: https://github.com/apache/arrow-rs/pull/11407#discussion_r4274465891
##########
arrow-cmp/src/lib.rs:
##########
@@ -234,6 +253,93 @@ fn compare_dict<K: ArrowDictionaryKeyType>(
Ok(f)
}
+#[inline]
+fn compare_dict_bytes<K: ArrowDictionaryKeyType, T: ByteArrayType>(
+ left: &DictionaryArray<K>,
+ right: &DictionaryArray<K>,
+ opts: SortOptions,
+) -> DynComparator {
+ let left_values = left.values().as_bytes::<T>().clone();
+ let right_values = right.values().as_bytes::<T>().clone();
+ let left_keys = left.keys().values().clone();
+ let right_keys = right.keys().values().clone();
+ // SAFETY: row indices are in-bounds per DynComparator contract; keys are
+ // in-bounds for values per DictionaryArray::try_new.
+ if left.logical_null_count() == 0 && right.logical_null_count() == 0 {
+ return if opts.descending {
+ Box::new(move |left_row, right_row| unsafe {
+ let left_bytes: &[u8] = left_values
+
.value_unchecked(left_keys.get_unchecked(left_row).as_usize())
+ .as_ref();
+ let right_bytes: &[u8] = right_values
+
.value_unchecked(right_keys.get_unchecked(right_row).as_usize())
+ .as_ref();
+ right_bytes.cmp(left_bytes)
+ })
+ } else {
+ Box::new(move |left_row, right_row| unsafe {
+ let left_bytes: &[u8] = left_values
+
.value_unchecked(left_keys.get_unchecked(left_row).as_usize())
+ .as_ref();
+ let right_bytes: &[u8] = right_values
+
.value_unchecked(right_keys.get_unchecked(right_row).as_usize())
+ .as_ref();
+ left_bytes.cmp(right_bytes)
+ })
+ };
+ }
+ // SAFETY: same invariants as above.
+ compare(left, right, opts, move |left_row, right_row| unsafe {
+ let left_bytes: &[u8] = left_values
+ .value_unchecked(left_keys.get_unchecked(left_row).as_usize())
+ .as_ref();
+ let right_bytes: &[u8] = right_values
+ .value_unchecked(right_keys.get_unchecked(right_row).as_usize())
+ .as_ref();
+ left_bytes.cmp(right_bytes)
+ })
+}
Review Comment:
makes sense to me, i'm a bit used to adding `[inline]` out of rabbit to make
functions faster but as you mentioned that would have no affect here.
--
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]
