extract _ignore_nullability methods out and fix ordering

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>

Author: connortsui20

vortex-array/src/dtype/dtype_impl.rs

@@ -122,21 +122,9 @@ impl DType {
                 lhs_size == rhs_size && lhs_dtype.eq_ignore_nullability(rhs_dtype)
             }
             (Struct(lhs_dtype, _), Struct(rhs_dtype, _)) => {
-                (lhs_dtype.names() == rhs_dtype.names())
-                    && (lhs_dtype
-                        .fields()
-                        .zip_eq(rhs_dtype.fields())
-                        .all(|(l, r)| l.eq_ignore_nullability(&r)))
-            }
-            (Union(lhs, _), Union(rhs, _)) => {
-                // Equal `names` implies equal length by FieldNames equality.
-                lhs.names() == rhs.names()
-                    && lhs.type_ids() == rhs.type_ids()
-                    && lhs
-                        .variants()
-                        .zip_eq(rhs.variants())
-                        .all(|(l, r)| l.eq_ignore_nullability(&r))
+                lhs_dtype.eq_ignore_nullability(rhs_dtype)
             }
+            (Union(lhs, _), Union(rhs, _)) => lhs.eq_ignore_nullability(rhs),
             (Variant(_), Variant(_)) => true,
             (Extension(lhs_extdtype), Extension(rhs_extdtype)) => {
                 lhs_extdtype.eq_ignore_nullability(rhs_extdtype)
@@ -155,22 +143,11 @@ impl DType {
             Decimal(decimal, _) => decimal.hash(state),
             List(element, _) => element.hash_ignore_nullability(state),
             FixedSizeList(element, size, _) => {
-                size.hash(state);
                 element.hash_ignore_nullability(state);
+                size.hash(state);
             }
-            Struct(fields, _) => {
-                fields.names().hash(state);
-                for field in fields.fields() {
-                    field.hash_ignore_nullability(state);
-                }
-            }
-            Union(variants) => {
-                variants.names().hash(state);
-                variants.type_ids().hash(state);
-                for variant in variants.variants() {
-                    variant.hash_ignore_nullability(state);
-                }
-            }
+            Struct(fields, _) => fields.hash_ignore_nullability(state),
+            Union(variants, _) => variants.hash_ignore_nullability(state),
             Extension(ext) => ext.hash_ignore_nullability(state),
         }
     }
@@ -586,22 +563,28 @@ mod tests {
 
     #[test]
     fn test_union_dtype_hash_ignores_variant_nullability() -> VortexResult<()> {
-        let lhs = DType::Union(UnionVariants::try_new(
-            ["int", "string"].into(),
-            vec![
-                DType::Primitive(PType::I32, Nullable),
-                DType::Utf8(NonNullable),
-            ],
-            vec![5, 9],
-        )?);
-        let rhs = DType::Union(UnionVariants::try_new(
-            ["int", "string"].into(),
-            vec![
-                DType::Primitive(PType::I32, NonNullable),
-                DType::Utf8(Nullable),
-            ],
-            vec![5, 9],
-        )?);
+        let lhs = DType::Union(
+            UnionVariants::try_new(
+                ["int", "string"].into(),
+                vec![
+                    DType::Primitive(PType::I32, Nullable),
+                    DType::Utf8(NonNullable),
+                ],
+                vec![5, 9],
+            )?,
+            NonNullable,
+        );
+        let rhs = DType::Union(
+            UnionVariants::try_new(
+                ["int", "string"].into(),
+                vec![
+                    DType::Primitive(PType::I32, NonNullable),
+                    DType::Utf8(Nullable),
+                ],
+                vec![5, 9],
+            )?,
+            NonNullable,
+        );
 
         assert!(lhs.eq_ignore_nullability(&rhs));
         assert_eq!(hash_ignore_nullability(&lhs), hash_ignore_nullability(&rhs));

vortex-array/src/dtype/struct_.rs

@@ -4,6 +4,7 @@
 use std::fmt::Display;
 use std::fmt::Formatter;
 use std::hash::Hash;
+use std::hash::Hasher;
 use std::sync::Arc;
 use std::sync::OnceLock;
 
@@ -83,7 +84,7 @@ impl PartialEq for FieldDTypeInner {
 impl Eq for FieldDTypeInner {}
 
 impl Hash for FieldDTypeInner {
-    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+    fn hash<H: Hasher>(&self, state: &mut H) {
         match self {
             FieldDTypeInner::Owned(owned) => {
                 owned.hash(state);
@@ -255,12 +256,33 @@ impl PartialEq for StructFieldsInner {
 impl Eq for StructFieldsInner {}
 
 impl Hash for StructFieldsInner {
-    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+    fn hash<H: Hasher>(&self, state: &mut H) {
         self.names.hash(state);
         self.dtypes.hash(state);
     }
 }
 
+impl StructFields {
+    /// Check if these struct fields are equal, ignoring field dtype nullability recursively.
+    pub fn eq_ignore_nullability(&self, other: &Self) -> bool {
+        Arc::ptr_eq(&self.0, &other.0)
+            || (self.0.names == other.0.names
+                && self
+                    .fields()
+                    .zip_eq(other.fields())
+                    .all(|(lhs, rhs)| lhs.eq_ignore_nullability(&rhs)))
+    }
+
+    /// Hash these struct fields using the same equivalence relation as
+    /// [`Self::eq_ignore_nullability`].
+    pub(crate) fn hash_ignore_nullability<H: Hasher>(&self, state: &mut H) {
+        self.0.names.hash(state);
+        for field in self.fields() {
+            field.hash_ignore_nullability(state);
+        }
+    }
+}
+
 impl Default for StructFields {
     fn default() -> Self {
         Self::empty()

vortex-array/src/dtype/union.rs

@@ -3,6 +3,7 @@
 
 use std::fmt;
 use std::hash::Hash;
+use std::hash::Hasher;
 use std::sync::Arc;
 
 use itertools::Itertools;
@@ -133,13 +134,36 @@ impl PartialEq for UnionVariantsInner {
 impl Eq for UnionVariantsInner {}
 
 impl Hash for UnionVariantsInner {
-    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+    fn hash<H: Hasher>(&self, state: &mut H) {
         self.names.hash(state);
         self.dtypes.hash(state);
         self.type_ids.hash(state);
     }
 }
 
+impl UnionVariants {
+    /// Check if these union variants are equal, ignoring variant dtype nullability recursively.
+    pub fn eq_ignore_nullability(&self, other: &Self) -> bool {
+        Arc::ptr_eq(&self.0, &other.0)
+            || (self.0.names == other.0.names
+                && self
+                    .variants()
+                    .zip_eq(other.variants())
+                    .all(|(lhs, rhs)| lhs.eq_ignore_nullability(&rhs))
+                && self.0.type_ids == other.0.type_ids)
+    }
+
+    /// Hash these union variants using the same equivalence relation as
+    /// [`Self::eq_ignore_nullability`].
+    pub(crate) fn hash_ignore_nullability<H: Hasher>(&self, state: &mut H) {
+        self.0.names.hash(state);
+        for variant in self.variants() {
+            variant.hash_ignore_nullability(state);
+        }
+        self.0.type_ids.hash(state);
+    }
+}
+
 impl Default for UnionVariants {
     fn default() -> Self {
         Self::empty()