friendlymatthew commented on code in PR #8838:
URL: https://github.com/apache/arrow-rs/pull/8838#discussion_r2543281181
##########
arrow-ord/src/ord.rs:
##########
@@ -296,6 +296,70 @@ fn compare_struct(
Ok(f)
}
+fn compare_union(
+ left: &dyn Array,
+ right: &dyn Array,
+ opts: SortOptions,
+) -> Result<DynComparator, ArrowError> {
+ let left = left.as_union();
+ let right = right.as_union();
+
+ let (left_fields, left_mode) = match left.data_type() {
Review Comment:
This should be super quick to review:
https://github.com/apache/arrow-rs/pull/8884
Somewhat related but it feels a bit weird that the following works without
any notice to the user:
```rs
#[test]
fn test_union_fields() {
let ids = vec![0, 1, 2];
let field = Field::new("a", DataType::Binary, true);
// different length of ids and fields (we zip so we truncate the longer
vec)
let _out = UnionFields::new(ids.clone(), vec![field.clone()]);
// duplicate fields associated with different type ids!
let _out = UnionFields::new(ids, vec![field.clone(), field]);
}
```
I feel like we could benefit from a bit more validation? We could leave
`UnionFields::new` but also have a `UnionFields::try_new` that checks the above
🤔
--
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]
