[GitHub] [arrow-rs] alamb commented on a diff in pull request #3981: Add UnionFields (#3955)

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #3981:
URL: https://github.com/apache/arrow-rs/pull/3981#discussion_r1153229128


##########
arrow-schema/src/fields.rs:
##########
@@ -148,3 +150,91 @@ impl<'de> serde::Deserialize<'de> for Fields {
         Ok(Vec::<Field>::deserialize(deserializer)?.into())
     }
 }
+
+/// A cheaply cloneable, owned collection of [`FieldRef`] and their corresponding type ids
+#[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)]>);
+
+impl std::fmt::Debug for UnionFields {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.as_ref().fmt(f)
+    }
+}
+
+impl UnionFields {
+    /// Create a new [`UnionFields`] with no fields
+    pub fn empty() -> Self {
+        Self(Arc::from([]))
+    }
+
+    /// Create a new [`UnionFields`] from a [`Fields`] and array of type_ids
+    pub fn new<F, T>(type_ids: T, fields: F) -> Self

Review Comment:
   Should this be fallible (to prevent repeated type ids, for example)?



##########
arrow-schema/src/fields.rs:
##########
@@ -148,3 +150,91 @@ impl<'de> serde::Deserialize<'de> for Fields {
         Ok(Vec::<Field>::deserialize(deserializer)?.into())
     }
 }
+
+/// A cheaply cloneable, owned collection of [`FieldRef`] and their corresponding type ids
+#[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)]>);
+
+impl std::fmt::Debug for UnionFields {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.as_ref().fmt(f)
+    }
+}
+
+impl UnionFields {
+    /// Create a new [`UnionFields`] with no fields
+    pub fn empty() -> Self {
+        Self(Arc::from([]))
+    }
+
+    /// Create a new [`UnionFields`] from a [`Fields`] and array of type_ids
+    pub fn new<F, T>(type_ids: T, fields: F) -> Self
+    where
+        F: IntoIterator,
+        F::Item: Into<FieldRef>,
+        T: IntoIterator<Item = i8>,
+    {
+        let fields = fields.into_iter().map(Into::into);
+        type_ids.into_iter().zip(fields).collect()
+    }
+
+    /// Return size of this instance in bytes.
+    pub fn size(&self) -> usize {
+        self.iter()
+            .map(|(_, field)| field.size() + std::mem::size_of::<(i8, FieldRef)>())
+            .sum()
+    }
+
+    /// Returns the number of fields in this [`UnionFields`]
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+
+    /// Returns `true` if this is empty
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    /// Returns an iterator over the fields and type ids in this [`UnionFields`]
+    ///
+    /// Note: the iteration order is not guaranteed
+    pub fn iter(&self) -> impl Iterator<Item = (i8, &FieldRef)> + '_ {
+        self.0.iter().map(|(id, f)| (*id, f))
+    }
+
+    pub(crate) fn try_merge(&mut self, other: &Self) -> Result<(), ArrowError> {

Review Comment:
   Can you please add some docs about what this does?



##########
arrow-schema/src/fields.rs:
##########
@@ -148,3 +150,91 @@ impl<'de> serde::Deserialize<'de> for Fields {
         Ok(Vec::<Field>::deserialize(deserializer)?.into())
     }
 }
+
+/// A cheaply cloneable, owned collection of [`FieldRef`] and their corresponding type ids
+#[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)]>);
+
+impl std::fmt::Debug for UnionFields {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.as_ref().fmt(f)
+    }
+}
+
+impl UnionFields {

Review Comment:
   Can we please add documentation that explains:
   1. what type_ids are (a link to the spec / docs)
   2. has an example of how to construct these things



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org