tinder-kbendickson commented on a change in pull request #2062:
URL: https://github.com/apache/iceberg/pull/2062#discussion_r566554091
##########
File path: api/src/main/java/org/apache/iceberg/transforms/Truncate.java
##########
@@ -303,11 +303,15 @@ public boolean satisfiesOrderOf(Transform<?, ?> other) {
return Expressions.predicate(predicate.op(), name);
} else if (predicate instanceof BoundLiteralPredicate) {
BoundLiteralPredicate<CharSequence> pred =
predicate.asLiteralPredicate();
- if (pred.op() == Expression.Operation.STARTS_WITH) {
+ if (isStringPrefixPredicate(pred)) {
if (pred.literal().value().length() < width()) {
Review comment:
So I spent some time analyzing this. There never was a case for that
here with STARTS_WITH either.
In the case that `pred.literal().value().length() > width()`, it will hop
out of this block and then hit a `return null` towards the very bottom.
I personally feel it's a bit confusing, but was hesistant to change it given
the large scope of this PR and the fact that it would change existing code
behavior (or we'd be implying that this was for sure the correct thing to do if
we explicitly `return null` in the case that length > width() - which I didn't
want to make that kind of implication in this PR).
I'm happy to update it, but we should be noting that we'll be changing the
existing behavior of STARTS_WITH (unless that block explicitly returns null).
----------------------------------------------------------------
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]
