tustvold commented on code in PR #4701:
URL: https://github.com/apache/arrow-rs/pull/4701#discussion_r1295614456


##########
arrow-ord/src/cmp.rs:
##########
@@ -174,21 +179,52 @@ fn compare_op(
         (LargeBinary, LargeBinary) => apply(op, l.as_binary::<i64>(), l_s, 
l_v, r.as_binary::<i64>(), r_s, r_v),
         (FixedSizeBinary(_), FixedSizeBinary(_)) => apply(op, 
l.as_fixed_size_binary(), l_s, l_v, r.as_fixed_size_binary(), r_s, r_v),
         (l_t, r_t) => return 
Err(ArrowError::InvalidArgumentError(format!("Invalid comparison operation: 
{l_t} {op} {r_t}"))),
-    };
+    }.unwrap_or_else(|| {
+        let count = nulls.as_ref().map(|x| x.null_count()).unwrap_or_default();
+        assert_eq!(count, len); // Sanity check
+        BooleanBuffer::new_unset(len)
+    });
 
     assert_eq!(values.len(), len); // Sanity check
     Ok(BooleanArray::new(values, nulls))
 }
 
-fn values(a: &dyn Array) -> (Option<Vec<usize>>, &dyn Array) {
+fn as_dictionary(a: &dyn Array) -> Option<&dyn Dictionary> {
     downcast_dictionary_array! {
-        a => {
-            let v = a.values().as_ref();
-            let v_len = v.len();
-            let keys = a.keys().values().iter().map(|x| 
x.as_usize().min(v_len)).collect();
-            (Some(keys), v)
-        }
-        _ => (None, a)
+        a => Some(a),
+        _ => None
+    }
+}
+
+trait Dictionary: Array {

Review Comment:
   I could see us making this public, perhaps with some additional methods, so 
that it can form the basic pattern for "how to handle dictionaries". FYI @alamb 



-- 
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]

Reply via email to