alamb commented on code in PR #13005:
URL: https://github.com/apache/datafusion/pull/13005#discussion_r1815596567
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -525,20 +519,6 @@ impl PhysicalExpr for BinaryExpr {
}
}
-impl PartialEq<dyn Any> for BinaryExpr {
Review Comment:
removing this is great
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -48,11 +47,11 @@ use kernels::{
};
/// Binary expression
-#[derive(Debug, Hash, Clone)]
-pub struct BinaryExpr {
- left: Arc<dyn PhysicalExpr>,
+#[derive(Debug, Hash, Clone, Eq, PartialEq)]
+pub struct BinaryExpr<DynPhysicalExpr: ?Sized = dyn PhysicalExpr> {
Review Comment:
I am playing around with it to see if I can avoid it somehow
##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -48,11 +47,11 @@ use kernels::{
};
/// Binary expression
-#[derive(Debug, Hash, Clone)]
-pub struct BinaryExpr {
- left: Arc<dyn PhysicalExpr>,
+#[derive(Debug, Hash, Clone, Eq, PartialEq)]
+pub struct BinaryExpr<DynPhysicalExpr: ?Sized = dyn PhysicalExpr> {
Review Comment:
The removal of the boiler plate in this PR is great ❤️
I feel like this additional trait / workaround is non ideal as it makes
understanding the `BinaryExpr` very unobvious (there is now a level of
indirection that is irrelevant for most of users of `BinaryExpr`).
--
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-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
