rich-t-kid-datadog commented on code in PR #7933:
URL: https://github.com/apache/arrow-rs/pull/7933#discussion_r2226888468
##########
arrow-ord/src/cmp.rs:
##########
@@ -224,6 +223,14 @@ fn compare_op(op: Op, lhs: &dyn Datum, rhs: &dyn Datum) ->
Result<BooleanArray,
let l_nulls = l.logical_nulls();
let r_nulls = r.logical_nulls();
+ // 1. Handle REE arrays first - extract logical values
+ use arrow_array::unwrap_ree_array;
+ let l_ree = unwrap_ree_array(l);
+ let r_ree = unwrap_ree_array(r);
Review Comment:
You're absolutely right — unpacking the REE array defeats its purpose. I
initially took that approach since we were doing element-by-element
comparisons, but it came with significant overhead: expanding the REE means
allocating a full logical array and then scanning both sides again to produce
the boolean output.
I've since refactored the implementation to avoid that. It now matches and
downcasts the value type directly, then compares values at logical positions by
resolving their physical indices — so there's no need to unpack or duplicate
work.
--
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]
