rdblue commented on a change in pull request #1747:
URL: https://github.com/apache/iceberg/pull/1747#discussion_r529087328
##########
File path: api/src/main/java/org/apache/iceberg/expressions/Expressions.java
##########
@@ -123,6 +123,22 @@ public static Expression not(Expression child) {
return new UnboundPredicate<>(Expression.Operation.NOT_NULL, expr);
}
+ public static <T> UnboundPredicate<T> isNaN(String name) {
+ return new UnboundPredicate<>(Expression.Operation.IS_NAN, ref(name));
+ }
+
+ public static <T> UnboundPredicate<T> isNaN(UnboundTerm<T> expr) {
+ return new UnboundPredicate<>(Expression.Operation.IS_NAN, expr);
+ }
+
+ public static <T> UnboundPredicate<T> notNaN(String name) {
+ return new UnboundPredicate<>(Expression.Operation.NOT_NAN, ref(name));
+ }
+
+ public static <T> UnboundPredicate<T> notNaN(UnboundTerm<T> expr) {
+ return new UnboundPredicate<>(Expression.Operation.NOT_NAN, expr);
+ }
Review comment:
My suggestion to reject invalid predicates should help avoid the problem
of rewriting so much test code. What I'm trying to say is that if it requires
changing the return type of one of the factory methods, let's throw an
`IllegalArgumentException` instead so that we don't need to.
From your list, I agree with 1 and 2, but for 3 I would do the rewrite in
Expressions because `isNan` and `notNaN` rewrites will produce
`UnboundPredicate` and we don't have to change the tests. Only rewriting `in`
and `notIn` would require changing lots of tests, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
