From 33976cbd525e9bad57d70310ed7430fa35dc5cb7 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Wed, 15 Jan 2025 16:05:44 +0000 Subject: [PATCH 1/4] . --- vortex-dtype/src/dtype.rs | 7 ++++++- vortex-dtype/src/extension.rs | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/vortex-dtype/src/dtype.rs b/vortex-dtype/src/dtype.rs index 6bde6e1b472..ad36da12f3e 100644 --- a/vortex-dtype/src/dtype.rs +++ b/vortex-dtype/src/dtype.rs @@ -110,7 +110,12 @@ impl DType { .all(|(l, r)| l.eq_ignore_nullability(&r))) } (Struct(..), _) => false, - (Extension(lhs_extdtype), Extension(rhs_extdtype)) => lhs_extdtype == rhs_extdtype, + (Extension(lhs_extdtype), Extension(rhs_extdtype)) => { + lhs_extdtype.id() == rhs_extdtype.id() + && lhs_extdtype + .storage_dtype() + .eq_ignore_nullability(rhs_extdtype.storage_dtype()) + } (Extension(_), _) => false, } } diff --git a/vortex-dtype/src/extension.rs b/vortex-dtype/src/extension.rs index 305b3dff4df..32e93aca929 100644 --- a/vortex-dtype/src/extension.rs +++ b/vortex-dtype/src/extension.rs @@ -68,13 +68,14 @@ pub struct ExtDType { impl PartialEq for ExtDType { fn eq(&self, other: &Self) -> bool { - self.id == other.id + self.id == other.id && self.storage_dtype == other.storage_dtype } } impl std::hash::Hash for ExtDType { fn hash(&self, state: &mut H) { self.id.hash(state); + self.storage_dtype.hash(state); } } From 32fed0e3cbaee9fbc9d4d14981065d8c8801d161 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Wed, 15 Jan 2025 16:22:37 +0000 Subject: [PATCH 2/4] . --- vortex-dtype/src/dtype.rs | 1 + vortex-dtype/src/extension.rs | 15 +-------------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/vortex-dtype/src/dtype.rs b/vortex-dtype/src/dtype.rs index ad36da12f3e..cc8357b0ad6 100644 --- a/vortex-dtype/src/dtype.rs +++ b/vortex-dtype/src/dtype.rs @@ -112,6 +112,7 @@ impl DType { (Struct(..), _) => false, (Extension(lhs_extdtype), Extension(rhs_extdtype)) => { lhs_extdtype.id() == rhs_extdtype.id() + && lhs_extdtype.metadata() == rhs_extdtype.metadata() && lhs_extdtype .storage_dtype() .eq_ignore_nullability(rhs_extdtype.storage_dtype()) diff --git a/vortex-dtype/src/extension.rs b/vortex-dtype/src/extension.rs index 32e93aca929..066c3df0af8 100644 --- a/vortex-dtype/src/extension.rs +++ b/vortex-dtype/src/extension.rs @@ -58,7 +58,7 @@ impl From<&[u8]> for ExtMetadata { } /// A type descriptor for an extension type -#[derive(Debug, Clone, PartialOrd, Eq)] +#[derive(Debug, Clone, PartialOrd, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct ExtDType { id: ExtID, @@ -66,19 +66,6 @@ pub struct ExtDType { metadata: Option, } -impl PartialEq for ExtDType { - fn eq(&self, other: &Self) -> bool { - self.id == other.id && self.storage_dtype == other.storage_dtype - } -} - -impl std::hash::Hash for ExtDType { - fn hash(&self, state: &mut H) { - self.id.hash(state); - self.storage_dtype.hash(state); - } -} - impl ExtDType { /// Creates a new `ExtDType`. /// From 97950dfcd4d8f8bd3ea3869c6a8f7ac845234978 Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Wed, 15 Jan 2025 16:24:29 +0000 Subject: [PATCH 3/4] . --- vortex-dtype/src/dtype.rs | 6 +----- vortex-dtype/src/extension.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/vortex-dtype/src/dtype.rs b/vortex-dtype/src/dtype.rs index cc8357b0ad6..c47b44cb240 100644 --- a/vortex-dtype/src/dtype.rs +++ b/vortex-dtype/src/dtype.rs @@ -111,11 +111,7 @@ impl DType { } (Struct(..), _) => false, (Extension(lhs_extdtype), Extension(rhs_extdtype)) => { - lhs_extdtype.id() == rhs_extdtype.id() - && lhs_extdtype.metadata() == rhs_extdtype.metadata() - && lhs_extdtype - .storage_dtype() - .eq_ignore_nullability(rhs_extdtype.storage_dtype()) + lhs_extdtype.as_ref().eq_ignore_nullability(&rhs_extdtype) } (Extension(_), _) => false, } diff --git a/vortex-dtype/src/extension.rs b/vortex-dtype/src/extension.rs index 066c3df0af8..a02475683f4 100644 --- a/vortex-dtype/src/extension.rs +++ b/vortex-dtype/src/extension.rs @@ -136,4 +136,13 @@ impl ExtDType { pub fn metadata(&self) -> Option<&ExtMetadata> { self.metadata.as_ref() } + + /// Check if `self` and `other` are equal, ignoring the storage nullability + pub fn eq_ignore_nullability(&self, other: &Self) -> bool { + self.id() == other.id() + && self.metadata() == other.metadata() + && self + .storage_dtype() + .eq_ignore_nullability(other.storage_dtype()) + } } From 55c60bb56c94acd308417583ad1f20693c60e15b Mon Sep 17 00:00:00 2001 From: Adam Gutglick Date: Wed, 15 Jan 2025 16:25:46 +0000 Subject: [PATCH 4/4] clippy --- vortex-dtype/src/dtype.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vortex-dtype/src/dtype.rs b/vortex-dtype/src/dtype.rs index c47b44cb240..c103ae26221 100644 --- a/vortex-dtype/src/dtype.rs +++ b/vortex-dtype/src/dtype.rs @@ -111,7 +111,7 @@ impl DType { } (Struct(..), _) => false, (Extension(lhs_extdtype), Extension(rhs_extdtype)) => { - lhs_extdtype.as_ref().eq_ignore_nullability(&rhs_extdtype) + lhs_extdtype.as_ref().eq_ignore_nullability(rhs_extdtype) } (Extension(_), _) => false, }