ozankabak commented on code in PR #13770:
URL: https://github.com/apache/datafusion/pull/13770#discussion_r1886874915


##########
datafusion/physical-expr/src/equivalence/class.rs:
##########
@@ -634,6 +634,18 @@ impl EquivalenceGroup {
             JoinType::RightSemi | JoinType::RightAnti => 
right_equivalences.clone(),
         }
     }
+
+    /// Checks if two expressions are equal either directly or through 
equivalence classes.
+    pub fn exprs_equal(

Review Comment:
   An easy way to test would be to look at `a + b` and `x + y` with `a` and `x` 
/ `b` and `y` being equivalent.



##########
datafusion/physical-expr/src/equivalence/class.rs:
##########
@@ -634,6 +634,18 @@ impl EquivalenceGroup {
             JoinType::RightSemi | JoinType::RightAnti => 
right_equivalences.clone(),
         }
     }
+
+    /// Checks if two expressions are equal either directly or through 
equivalence classes.
+    pub fn exprs_equal(

Review Comment:
   This function does not work for two general expressions. In general, 
checking whether two expression trees are equal involves a lot of work: One not 
only needs to make sure all symbols (i.e. columns) are equivalent, but also 
take operator properties like commutativity into account.
   
   For the purposes of this PR, I think it is sufficient to just make sure that:
   1. `left` and `right` expression trees are "the same" except leaves.
   2. Each leaf on the left and right side are equivalent w.r.t. some group.



-- 
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

Reply via email to