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]