alamb commented on code in PR #8896:
URL: https://github.com/apache/arrow-rs/pull/8896#discussion_r2586196587
##########
arrow-ord/src/ord.rs:
##########
@@ -368,6 +369,52 @@ fn compare_union(
Ok(f)
}
+fn compare_union_to_opaque(
+ union_array: &dyn Array,
+ opaque_array: &dyn Array,
+ opts: SortOptions,
+) -> Result<DynComparator, ArrowError> {
+ let union_array = union_array.as_union();
+
+ let DataType::Union(union_fields, _) = union_array.data_type() else {
+ unreachable!()
+ };
+
+ let opaque_type_id = union_fields
Review Comment:
I am not sure about this -- it seems like it will compare the union array to
the first field that has the same type.
What if the union array has multiple repeated types (I realize that is not a
common use case, but I think it is possible)
##########
arrow-ord/src/ord.rs:
##########
@@ -485,6 +532,8 @@ pub fn make_comparator(
},
(Map(_, _), Map(_, _)) => compare_map(left, right, opts),
(Union(_, _), Union(_, _)) => compare_union(left, right, opts),
+ (Union(_, _), _) => compare_union_to_opaque(left, right, opts),
Review Comment:
Is there any other example of comparing one array to an array of another
type? I think this kernel expects the types to match exactly
I do see the argument about how UnionArray could be treated as a special
case (as some sort of runtime typed thing) 🤔 but I am not sure UnionArrays are
required to behave that way
I wrote up another way that your usecase might be handled
- https://github.com/apache/arrow-rs/issues/8881#issuecomment-3608253049
Let me know what you think
--
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]
