Fix scalar eq for booleans (#2168)

And derive partial eq where we can
spiraldb · Jan 31, 2025 · 46c770b · 46c770b
1 parent d0294e4
commit 46c770b
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 36 deletions.
diff --git a/vortex-scalar/src/binary.rs b/vortex-scalar/src/binary.rs
@@ -6,23 +6,18 @@ use vortex_error::{vortex_bail, vortex_err, VortexError, VortexExpect as _, Vort
 
 use crate::{InnerScalarValue, Scalar, ScalarValue};
 
-#[derive(Debug, Hash)]
+#[derive(Debug, Hash, PartialEq, Eq)]
 pub struct BinaryScalar<'a> {
     dtype: &'a DType,
     value: Option<ByteBuffer>,
 }
 
-impl PartialEq for BinaryScalar<'_> {
-    fn eq(&self, other: &Self) -> bool {
-        self.dtype == other.dtype && self.value == other.value
-    }
-}
-
-impl Eq for BinaryScalar<'_> {}
-
 /// Ord is not implemented since it's undefined for different nullability
 impl PartialOrd for BinaryScalar<'_> {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        if self.dtype != other.dtype {
+            return None;
+        }
         self.value.partial_cmp(&other.value)
     }
 }

diff --git a/vortex-scalar/src/bool.rs b/vortex-scalar/src/bool.rs
@@ -6,20 +6,12 @@ use vortex_error::{vortex_bail, vortex_err, VortexError, VortexExpect as _, Vort
 
 use crate::{InnerScalarValue, Scalar, ScalarValue};
 
-#[derive(Debug, Hash)]
+#[derive(Debug, Hash, PartialEq, Eq)]
 pub struct BoolScalar<'a> {
     dtype: &'a DType,
     value: Option<bool>,
 }
 
-impl PartialEq for BoolScalar<'_> {
-    fn eq(&self, other: &Self) -> bool {
-        self.value == other.value
-    }
-}
-
-impl Eq for BoolScalar<'_> {}
-
 impl PartialOrd for BoolScalar<'_> {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         if self.dtype != other.dtype {

diff --git a/vortex-scalar/src/primitive.rs b/vortex-scalar/src/primitive.rs
@@ -14,21 +14,13 @@ use vortex_error::{
 use crate::pvalue::PValue;
 use crate::{InnerScalarValue, Scalar, ScalarValue};
 
-#[derive(Debug, Clone, Copy, Hash)]
+#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
 pub struct PrimitiveScalar<'a> {
     dtype: &'a DType,
     ptype: PType,
     pvalue: Option<PValue>,
 }
 
-impl PartialEq for PrimitiveScalar<'_> {
-    fn eq(&self, other: &Self) -> bool {
-        self.dtype() == other.dtype() && self.pvalue == other.pvalue
-    }
-}
-
-impl Eq for PrimitiveScalar<'_> {}
-
 /// Ord is not implemented since it's undefined for different nullability
 impl PartialOrd for PrimitiveScalar<'_> {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {

diff --git a/vortex-scalar/src/utf8.rs b/vortex-scalar/src/utf8.rs
@@ -7,23 +7,18 @@ use vortex_error::{vortex_bail, vortex_err, VortexError, VortexExpect as _, Vort
 
 use crate::{InnerScalarValue, Scalar, ScalarValue};
 
-#[derive(Debug, Hash)]
+#[derive(Debug, Hash, PartialEq, Eq)]
 pub struct Utf8Scalar<'a> {
     dtype: &'a DType,
     value: Option<BufferString>,
 }
 
-impl PartialEq for Utf8Scalar<'_> {
-    fn eq(&self, other: &Self) -> bool {
-        self.dtype == other.dtype && self.value == other.value
-    }
-}
-
-impl Eq for Utf8Scalar<'_> {}
-
 /// Ord is not implemented since it's undefined for different nullability
 impl PartialOrd for Utf8Scalar<'_> {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        if self.dtype != other.dtype {
+            return None;
+        }
         self.value.partial_cmp(&other.value)
     }
 }