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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
