Complete manual derivations

peter-toth · Oct 24, 2024 · 8b4ef56 · 8b4ef56
1 parent 1aa787d
commit 8b4ef56
Show file tree

Hide file tree

Showing 6 changed files with 107 additions and 25 deletions.
diff --git a/datafusion/physical-expr/src/expressions/is_null.rs b/datafusion/physical-expr/src/expressions/is_null.rs
@@ -30,10 +30,23 @@ use datafusion_common::ScalarValue;
 use datafusion_expr::ColumnarValue;
 
 /// IS NULL expression
-#[derive(Debug, Hash, Eq, PartialEq)]
-pub struct IsNullExpr<DynPhysicalExpr: ?Sized = dyn PhysicalExpr> {
+#[derive(Debug, Eq)]
+pub struct IsNullExpr {
     /// Input expression
-    arg: Arc<DynPhysicalExpr>,
+    arg: Arc<dyn PhysicalExpr>,
+}
+
+// Manually derive PartialEq and Hash to work around https://github.com/rust-lang/rust/issues/78808
+impl PartialEq for IsNullExpr {
+    fn eq(&self, other: &Self) -> bool {
+        self.arg.eq(&other.arg)
+    }
+}
+
+impl Hash for IsNullExpr {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.arg.hash(state);
+    }
 }
 
 impl IsNullExpr {

diff --git a/datafusion/physical-expr/src/expressions/like.rs b/datafusion/physical-expr/src/expressions/like.rs
@@ -26,12 +26,31 @@ use datafusion_expr::ColumnarValue;
 use datafusion_physical_expr_common::datum::apply_cmp;
 
 // Like expression
-#[derive(Debug, Hash, Eq, PartialEq)]
-pub struct LikeExpr<DynPhysicalExpr: ?Sized = dyn PhysicalExpr> {
+#[derive(Debug, Eq)]
+pub struct LikeExpr {
     negated: bool,
     case_insensitive: bool,
-    expr: Arc<DynPhysicalExpr>,
-    pattern: Arc<DynPhysicalExpr>,
+    expr: Arc<dyn PhysicalExpr>,
+    pattern: Arc<dyn PhysicalExpr>,
+}
+
+// Manually derive PartialEq and Hash to work around https://github.com/rust-lang/rust/issues/78808
+impl PartialEq for LikeExpr {
+    fn eq(&self, other: &Self) -> bool {
+        self.negated == other.negated
+            && self.case_insensitive == other.case_insensitive
+            && self.expr.eq(&other.expr)
+            && self.pattern.eq(&other.pattern)
+    }
+}
+
+impl Hash for LikeExpr {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.negated.hash(state);
+        self.case_insensitive.hash(state);
+        self.expr.hash(state);
+        self.pattern.hash(state);
+    }
 }
 
 impl LikeExpr {

diff --git a/datafusion/physical-expr/src/expressions/negative.rs b/datafusion/physical-expr/src/expressions/negative.rs
@@ -37,10 +37,23 @@ use datafusion_expr::{
 };
 
 /// Negative expression
-#[derive(Debug, Hash, Eq, PartialEq)]
-pub struct NegativeExpr<DynPhysicalExpr: ?Sized = dyn PhysicalExpr> {
+#[derive(Debug, Eq)]
+pub struct NegativeExpr {
     /// Input expression
-    arg: Arc<DynPhysicalExpr>,
+    arg: Arc<dyn PhysicalExpr>,
+}
+
+// Manually derive PartialEq and Hash to work around https://github.com/rust-lang/rust/issues/78808
+impl PartialEq for NegativeExpr {
+    fn eq(&self, other: &Self) -> bool {
+        self.arg.eq(&other.arg)
+    }
+}
+
+impl Hash for NegativeExpr {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.arg.hash(state);
+    }
 }
 
 impl NegativeExpr {
@@ -209,9 +222,7 @@ mod tests {
 
     #[test]
     fn test_evaluate_bounds() -> Result<()> {
-        let negative_expr = NegativeExpr::<dyn PhysicalExpr> {
-            arg: Arc::new(Column::new("a", 0)),
-        };
+        let negative_expr = NegativeExpr::new(Arc::new(Column::new("a", 0)));
         let child_interval = Interval::make(Some(-2), Some(1))?;
         let negative_expr_interval = Interval::make(Some(-1), Some(2))?;
         assert_eq!(
@@ -223,9 +234,7 @@ mod tests {
 
     #[test]
     fn test_propagate_constraints() -> Result<()> {
-        let negative_expr = NegativeExpr::<dyn PhysicalExpr> {
-            arg: Arc::new(Column::new("a", 0)),
-        };
+        let negative_expr = NegativeExpr::new(Arc::new(Column::new("a", 0)));
         let original_child_interval = Interval::make(Some(-2), Some(3))?;
         let negative_expr_interval = Interval::make(Some(0), Some(4))?;
         let after_propagation = Some(vec![Interval::make(Some(-2), Some(0))?]);

diff --git a/datafusion/physical-expr/src/expressions/not.rs b/datafusion/physical-expr/src/expressions/not.rs
@@ -29,10 +29,23 @@ use datafusion_common::{cast::as_boolean_array, Result, ScalarValue};
 use datafusion_expr::ColumnarValue;
 
 /// Not expression
-#[derive(Debug, Hash, Eq, PartialEq)]
-pub struct NotExpr<DynPhysicalExpr: ?Sized = dyn PhysicalExpr> {
+#[derive(Debug, Eq)]
+pub struct NotExpr {
     /// Input expression
-    arg: Arc<DynPhysicalExpr>,
+    arg: Arc<dyn PhysicalExpr>,
+}
+
+// Manually derive PartialEq and Hash to work around https://github.com/rust-lang/rust/issues/78808
+impl PartialEq for NotExpr {
+    fn eq(&self, other: &Self) -> bool {
+        self.arg.eq(&other.arg)
+    }
+}
+
+impl Hash for NotExpr {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.arg.hash(state);
+    }
 }
 
 impl NotExpr {

diff --git a/datafusion/physical-expr/src/expressions/try_cast.rs b/datafusion/physical-expr/src/expressions/try_cast.rs
@@ -31,14 +31,28 @@ use datafusion_common::{not_impl_err, Result, ScalarValue};
 use datafusion_expr::ColumnarValue;
 
 /// TRY_CAST expression casts an expression to a specific data type and returns NULL on invalid cast
-#[derive(Debug, Hash, Eq, PartialEq)]
-pub struct TryCastExpr<DynPhysicalExpr: ?Sized = dyn PhysicalExpr> {
+#[derive(Debug, Eq)]
+pub struct TryCastExpr {
     /// The expression to cast
-    expr: Arc<DynPhysicalExpr>,
+    expr: Arc<dyn PhysicalExpr>,
     /// The data type to cast to
     cast_type: DataType,
 }
 
+// Manually derive PartialEq and Hash to work around https://github.com/rust-lang/rust/issues/78808
+impl PartialEq for TryCastExpr {
+    fn eq(&self, other: &Self) -> bool {
+        self.expr.eq(&other.expr) && self.cast_type == other.cast_type
+    }
+}
+
+impl Hash for TryCastExpr {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.expr.hash(state);
+        self.cast_type.hash(state);
+    }
+}
+
 impl TryCastExpr {
     /// Create a new CastExpr
     pub fn new(expr: Arc<dyn PhysicalExpr>, cast_type: DataType) -> Self {

diff --git a/datafusion/proto/tests/cases/roundtrip_physical_plan.rs b/datafusion/proto/tests/cases/roundtrip_physical_plan.rs
@@ -741,10 +741,24 @@ fn roundtrip_parquet_exec_with_custom_predicate_expr() -> Result<()> {
         output_ordering: vec![],
     };
 
-    #[derive(Debug, Hash, Clone, PartialEq, Eq)]
-    struct CustomPredicateExpr<DynPhysicalExpr: ?Sized = dyn PhysicalExpr> {
-        inner: Arc<DynPhysicalExpr>,
+    #[derive(Debug, Clone, Eq)]
+    struct CustomPredicateExpr {
+        inner: Arc<dyn PhysicalExpr>,
     }
+
+    // Manually derive PartialEq and Hash to work around https://github.com/rust-lang/rust/issues/78808
+    impl PartialEq for CustomPredicateExpr {
+        fn eq(&self, other: &Self) -> bool {
+            self.inner.eq(&other.inner)
+        }
+    }
+
+    impl std::hash::Hash for CustomPredicateExpr {
+        fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+            self.inner.hash(state);
+        }
+    }
+
     impl Display for CustomPredicateExpr {
         fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
             write!(f, "CustomPredicateExpr")