rdblue commented on code in PR #4947:
URL: https://github.com/apache/iceberg/pull/4947#discussion_r889286645


##########
api/src/main/java/org/apache/iceberg/expressions/Expression.java:
##########
@@ -124,4 +124,21 @@ public Operation flipLR() {
   default Expression negate() {
     throw new UnsupportedOperationException(String.format("%s cannot be 
negated", this));
   }
+
+  /**
+   * Returns whether this expression will accept the same values as another.
+   * <p>
+   * If this returns true, the expressions are guaranteed to return the same 
evaluation for the same input. However, if
+   * this returns false the expressions may return the same evaluation for the 
same input. That is, expressions may
+   * be equivalent even if this returns false.
+   * <p>
+   * For best results, rewrite not and bind expressions before calling this 
method.
+   *
+   * @param other another expression
+   * @return true if the expressions are equivalent
+   */
+  default boolean isEquivalentTo(Expression other) {

Review Comment:
   I typically try to make the method name form a natural sentence in English, 
like `if (someFilter.isEquivalentTo(other))` reads like "if some filter is 
equivalent to other". In some cases it makes more sense to use other words, 
like `has`.
   
   I wasn't sure what to do with `ExpressionUtil.equivalent` since it compares 
two things, but `areEquivalent(f1, f2, schema)` doesn't feel right. I just left 
that as `ExpressionUtil.equivalent`. Suggestions are welcome!



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to