(arrow-rs) branch 57_maintenance updated: [57_maintenance] fix: `UnionFields::from_iter` should validate input (#11726)

[alamb]

This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch 57_maintenance
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git


The following commit(s) were added to refs/heads/57_maintenance by this push:
     new d0ea74f988 [57_maintenance] fix: `UnionFields::from_iter` should 
validate input (#11726)
d0ea74f988 is described below

commit d0ea74f98887c697e80d6c7f5a5ca74546a90e5f
Author: Andrew Lamb <and...@nerdnetworks.org>
AuthorDate: Wed Oct 21 15:33:46 2026 -0400

    [57_maintenance] fix: `UnionFields::from_iter` should validate input 
(#11726)
    
    - Backport https://github.com/apache/arrow-rs/pull/11533
    - For inclusion in https://github.com/apache/arrow-rs/issues/11666
    
    Cherry-pick of
    
https://github.com/apache/arrow-rs/commit/b2e2c2d9c24a3bb9a8105bbf55fb03b337e917dd
    
    Co-authored-by: Jeffrey Vo <jeffrey.vo.austra...@gmail.com>
---
 arrow-schema/src/fields.rs | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/arrow-schema/src/fields.rs b/arrow-schema/src/fields.rs
index 93638181d9..3b33d161b7 100644
--- a/arrow-schema/src/fields.rs
+++ b/arrow-schema/src/fields.rs
@@ -317,7 +317,12 @@ impl<'a> IntoIterator for &'a Fields {
     }
 }
 
-/// A cheaply cloneable, owned collection of [`FieldRef`] and their 
corresponding type ids
+/// A cheaply cloneable, owned collection of [`FieldRef`] and their 
corresponding
+/// type ids.
+///
+/// Invariants:
+/// - All type ids are unique
+/// - All type ids are non-negative
 #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "serde", serde(transparent))]
@@ -358,8 +363,9 @@ impl UnionFields {
     /// # Errors
     ///
     /// This function returns an error if:
+    /// - Iterators are not the same length
     /// - Any type_id appears more than once (duplicate type ids)
-    /// - The type_ids are duplicated
+    /// - There is a negative type_id
     ///
     /// # Examples
     ///
@@ -672,14 +678,26 @@ impl UnionFields {
                 output.push((field_type_id, from_field.clone()))
             }
         }
-        *self = output.into_iter().collect();
+        // TODO: see above comment; can we remove the need to go through 
try_new
+        //       and just validate in this function ourselves?
+        let (type_ids, fields): (Vec<i8>, Vec<FieldRef>) = 
output.into_iter().unzip();
+        *self = Self::try_new(type_ids, fields)?;
         Ok(())
     }
 }
 
 impl FromIterator<(i8, FieldRef)> for UnionFields {
+    /// # Panics
+    ///
+    /// This will panic on invalid inputs:
+    /// - Duplicate type IDs
+    /// - Negative type IDs
+    ///
+    /// Prefer using [`UnionFields::try_new`] for a fallible construction 
method.
     fn from_iter<T: IntoIterator<Item = (i8, FieldRef)>>(iter: T) -> Self {
-        Self(iter.into_iter().collect())
+        let iter = iter.into_iter();
+        let (type_ids, fields): (Vec<i8>, Vec<FieldRef>) = iter.unzip();
+        Self::try_new(type_ids, fields).expect("invalid arguments passed")
     }
 }
 
@@ -1039,4 +1057,16 @@ mod tests {
         assert!(res.is_ok());
         assert_eq!(res.unwrap().len(), 3);
     }
+
+    #[test]
+    #[should_panic(
+        expected = "invalid arguments passed: InvalidArgumentError(\"type ids 
must be non-negative: -1\")"
+    )]
+    fn test_union_fields_from_iter_should_validate() {
+        let fields: Vec<(i8, FieldRef)> = vec![
+            (0, Arc::new(Field::new("a", DataType::Int32, true))),
+            (-1, Arc::new(Field::new("a", DataType::Int32, true))),
+        ];
+        UnionFields::from_iter(fields);
+    }
 }