sort

Author: friendlymatthew

arrow-array/src/array/union_array.rs

@@ -1721,9 +1721,8 @@ mod tests {
             .len(7)
             .buffers(vec![type_ids, value_offsets])
             .child_data(vec![
-                // Child arrays must be in sorted order by type ID: 4, 8, 9
-                int_array.into_data(),
                 string_array.into_data(),
+                int_array.into_data(),
                 float_array.into_data(),
             ])
             .build()
@@ -1834,9 +1833,8 @@ mod tests {
             .len(7)
             .buffers(vec![type_ids, value_offsets])
             .child_data(vec![
-                // Child arrays must be in sorted order by type ID: 4, 8, 9
-                int_array.into_data(),
                 string_array.into_data(),
+                int_array.into_data(),
                 float_array.into_data(),
             ])
             .build()
@@ -1864,10 +1862,9 @@ mod tests {
             ],
         )
         .unwrap();
-        // Child arrays must be in sorted order by type ID: 2, 3
         let children = vec![
-            Arc::new(StringArray::from_iter_values(["c", "d"])) as _,
             Arc::new(StringArray::from_iter_values(["a", "b"])) as _,
+            Arc::new(StringArray::from_iter_values(["c", "d"])) as _,
         ];
 
         let type_ids = vec![3, 3, 2].into();
@@ -1893,10 +1890,9 @@ mod tests {
             "Invalid argument error: Type Ids values must match one of the field type ids"
         );
 
-        // Child arrays must be in sorted order by type ID: 2, 3
         let children = vec![
-            Arc::new(StringArray::from_iter_values(["c"])) as _,
             Arc::new(StringArray::from_iter_values(["a", "b"])) as _,
+            Arc::new(StringArray::from_iter_values(["c"])) as _,
         ];
         let type_ids = ScalarBuffer::from(vec![3_i8, 3, 2]);
         let offsets = Some(vec![0, 1, 0].into());

arrow-avro/src/reader/record.rs

@@ -4330,17 +4330,8 @@ mod tests {
             avro_children.push(AvroDataType::new(codec, Default::default(), None));
             fields.push(arrow_schema::Field::new(name, dt, true));
         }
-        let union_fields = UnionFields::try_new(type_ids.clone(), fields).unwrap();
-
-        // UnionFields are sorted by type_id, so we need to reorder avro_children to match
-        let mut sorted_indices: Vec<usize> = (0..type_ids.len()).collect();
-        sorted_indices.sort_by_key(|&i| type_ids[i]);
-        let sorted_avro_children: Vec<AvroDataType> = sorted_indices
-            .iter()
-            .map(|&i| avro_children[i].clone())
-            .collect();
-
-        let union_codec = Codec::Union(sorted_avro_children.into(), union_fields, UnionMode::Dense);
+        let union_fields = UnionFields::try_new(type_ids, fields).unwrap();
+        let union_codec = Codec::Union(avro_children.into(), union_fields, UnionMode::Dense);
         AvroDataType::new(union_codec, Default::default(), None)
     }
 
@@ -4405,13 +4396,11 @@ mod tests {
             vec![42, 7],
         );
         let mut dec = Decoder::try_new(&union_dt).unwrap();
-        // after sorting by type_id, schema order is [string(7), null(42)]
-        // to encode null, use branch 1; to encode string, use branch 0
-        let r1 = encode_avro_long(1);
+        let r1 = encode_avro_long(0);
         let mut r2 = Vec::new();
-        r2.extend_from_slice(&encode_avro_long(0));
+        r2.extend_from_slice(&encode_avro_long(1));
         r2.extend_from_slice(&encode_avro_bytes(b"abc"));
-        let r3 = encode_avro_long(1);
+        let r3 = encode_avro_long(0);
         dec.decode(&mut AvroCursor::new(&r1)).unwrap();
         dec.decode(&mut AvroCursor::new(&r2)).unwrap();
         dec.decode(&mut AvroCursor::new(&r3)).unwrap();

arrow-schema/src/fields.rs

@@ -15,8 +15,8 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use std::ops::Deref;
 use std::sync::Arc;
-use std::{hash::Hash, ops::Deref};
 
 use crate::{ArrowError, DataType, Field, FieldRef};
 
@@ -318,7 +318,7 @@ impl<'a> IntoIterator for &'a Fields {
 }
 
 /// A cheaply cloneable, owned collection of [`FieldRef`] and their corresponding type ids
-#[derive(Clone, PartialEq, Eq, Hash, Ord, PartialOrd)]
+#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "serde", serde(transparent))]
 pub struct UnionFields(Arc<[(i8, FieldRef)]>);
@@ -402,7 +402,7 @@ impl UnionFields {
 
         loop {
             match (type_ids_iter.next(), fields_iter.next()) {
-                (None, None) => break,
+                (None, None) => return Ok(Self(out.into())),
                 (Some(type_id), Some(field)) => {
                     // check type id is non-negative
                     if type_id < 0 {
@@ -435,11 +435,6 @@ impl UnionFields {
                 }
             }
         }
-
-        // sort by type ids to produce a consistent ordering
-        out.sort_unstable_by_key(|&(i, _)| i);
-
-        Ok(Self(out.into()))
     }
 
     /// Create a new [`UnionFields`] from a collection of fields with automatically
@@ -1044,31 +1039,4 @@ mod tests {
         assert!(res.is_ok());
         assert_eq!(res.unwrap().len(), 3);
     }
-
-    #[test]
-    fn test_union_field_equality() {
-        let ids = vec![0, 1, 2];
-        let fields = vec![
-            Field::new("a", DataType::Binary, true),
-            Field::new("b", DataType::Utf8, true),
-            Field::new("c", DataType::Int16, true),
-        ];
-
-        let u = UnionFields::try_new(ids.clone(), fields.clone()).unwrap();
-        assert_eq!(u.clone(), u.clone());
-
-        let u_rev =
-            UnionFields::try_new(ids.clone().into_iter().rev(), fields.into_iter().rev()).unwrap();
-        assert_eq!(u, u_rev);
-
-        let fields_2 = vec![
-            Field::new("a", DataType::Binary, true),
-            Field::new("b", DataType::Utf8, true),
-            // everything is the same from `fields` except Field "c" is not nullable
-            Field::new("c", DataType::Int16, false),
-        ];
-
-        let u2 = UnionFields::try_new(ids.clone(), fields_2.clone()).unwrap();
-        assert_ne!(u, u2);
-    }
 }