yyanyy commented on a change in pull request #1747:
URL: https://github.com/apache/iceberg/pull/1747#discussion_r529062645
##########
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:
The main reason for me to suggest rewriting in `UnboundPredicate` is
mostly for readability, as `UnboundPredicate` already contains [quite some
rewritings](https://github.com/apache/iceberg/blob/master/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java#L194-L197),
and updating `Expressions` will result in more changes to itself and related
test cases.
I agree with catching problems early, and just to confirm, I guess this only
applies to rejecting invalid input, as rewriting equals/notEquals in
`Expressions` probably won't help with catching problem early as it probably
won't throw anything by rewriting without binding? Based on my understanding I
will do the following:
1. reject `>/>=/</<= NaN` in `Expressions` (currently in `UnboundPredicate`,
will move to `Expressions`)
2. check if `in`/`notIn` literals contain `NaN` and reject
3. rewrite `eq/notEq` in `UnboundPredicate` (already happening)
Please let me know if you have comment!
----------------------------------------------------------------
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]
