rdblue commented on code in PR #8257:
URL: https://github.com/apache/iceberg/pull/8257#discussion_r1290414961
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -626,38 +628,47 @@ public <T> String predicate(BoundPredicate<T> pred) {
public <T> String predicate(UnboundPredicate<T> pred) {
switch (pred.op()) {
case IS_NULL:
- return pred.ref().name() + " IS NULL";
+ return sqlString(pred.term()) + " IS NULL";
case NOT_NULL:
- return pred.ref().name() + " IS NOT NULL";
+ return sqlString(pred.term()) + " IS NOT NULL";
case IS_NAN:
- return "is_nan(" + pred.ref().name() + ")";
+ return "is_nan(" + sqlString(pred.term()) + ")";
case NOT_NAN:
- return "not_nan(" + pred.ref().name() + ")";
+ return "not_nan(" + sqlString(pred.term()) + ")";
case LT:
- return pred.ref().name() + " < " + sqlString(pred.literal());
+ return sqlString(pred.term()) + " < " + sqlString(pred.literal());
case LT_EQ:
- return pred.ref().name() + " <= " + sqlString(pred.literal());
+ return sqlString(pred.term()) + " <= " + sqlString(pred.literal());
case GT:
- return pred.ref().name() + " > " + sqlString(pred.literal());
+ return sqlString(pred.term()) + " > " + sqlString(pred.literal());
case GT_EQ:
- return pred.ref().name() + " >= " + sqlString(pred.literal());
+ return sqlString(pred.term()) + " >= " + sqlString(pred.literal());
case EQ:
- return pred.ref().name() + " = " + sqlString(pred.literal());
+ return sqlString(pred.term()) + " = " + sqlString(pred.literal());
case NOT_EQ:
- return pred.ref().name() + " != " + sqlString(pred.literal());
+ return sqlString(pred.term()) + " != " + sqlString(pred.literal());
case STARTS_WITH:
- return pred.ref().name() + " LIKE '" + pred.literal().value() + "%'";
+ return sqlString(pred.term()) + " LIKE '" + pred.literal().value() +
"%'";
case NOT_STARTS_WITH:
- return pred.ref().name() + " NOT LIKE '" + pred.literal().value() +
"%'";
+ return sqlString(pred.term()) + " NOT LIKE '" +
pred.literal().value() + "%'";
case IN:
- return pred.ref().name() + " IN (" + sqlString(pred.literals()) +
")";
+ return sqlString(pred.term()) + " IN (" + sqlString(pred.literals())
+ ")";
case NOT_IN:
- return pred.ref().name() + " NOT IN (" + sqlString(pred.literals())
+ ")";
+ return sqlString(pred.term()) + " NOT IN (" +
sqlString(pred.literals()) + ")";
default:
throw new UnsupportedOperationException("Cannot convert predicate to
SQL: " + pred);
}
}
+ private static <T> String sqlString(UnboundTerm<T> term) {
+ if (term instanceof org.apache.iceberg.expressions.NamedReference) {
+ return term.ref().name();
+ } else {
Review Comment:
I think we had this issue in #7886 as well, where an else case resulted in
an unchecked cast. While that assumption works today, there is no guarantee
that it will in the future and this would result in a strange runtime exception
(`ClassCastException`) that doesn't really tell the caller or a user reading
logs what happened.
Cases like this should always use `instanceof` checks and have an `else`
case that has the most reasonable handling (here, it would be throwing
`UnsupportedOperationException`).
--
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]
