[GitHub] [iceberg] kbendick commented on a change in pull request #2062: [CORE] Add in a NOT_STARTS_WITH operator with filter pushdown

[GitBox <gi...@apache.org>]

kbendick commented on a change in pull request #2062:
URL: https://github.com/apache/iceberg/pull/2062#discussion_r589250942



##########
File path: api/src/main/java/org/apache/iceberg/transforms/Truncate.java
##########
@@ -284,8 +284,30 @@ public boolean satisfiesOrderOf(Transform<?, ?> other) {
 
       if (predicate.isUnaryPredicate()) {
         return Expressions.predicate(predicate.op(), name);
-      } else if (predicate.isLiteralPredicate()) {
-        return ProjectionUtil.truncateArray(name, predicate.asLiteralPredicate(), this);
+      } else if (predicate instanceof BoundLiteralPredicate) {

Review comment:
       That's a good call. I'm not 100% sure why this got changed. If memory serves, I think I might have just used this snippet as it was recommended earlier in the review process (which I think might have had the `instanceof` check). So I chose to err on the side of caution and not deviate from whatever was provided.
   
   Going forward, I will use my own judgement about whether there's a better way to write something and consider that what is suggested might potentially be a reference (particularly in this case where the existing code is already using `isLiteralPredicate` and the original recommendation has clear intent). I agree that keeping `isLiteralPredicate` function is for sure a better idea.
   
   I will update this 👍 




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org