rdblue commented on a change in pull request #2372:
URL: https://github.com/apache/iceberg/pull/2372#discussion_r603700530
##########
File path: data/src/main/java/org/apache/iceberg/data/DeleteFilter.java
##########
@@ -112,11 +112,11 @@ protected long pos(T record) {
return applyEqDeletes(applyPosDeletes(records));
}
- private List<Predicate<T>> applyEqDeletes() {
- List<Predicate<T>> isInDeleteSets = Lists.newArrayList();
+ private Predicate<T> buildEqDeletePredicate() {
if (eqDeletes.isEmpty()) {
- return isInDeleteSets;
+ return null;
}
+ Predicate<T> isDeleted = t -> false;
Review comment:
I think this should be initialized to `null` instead of a predicate.
There is no need to run an extra predicate (with an extra method dispatch for
each row in a data file. That's a tight loop so we should do more work here to
avoid it. Instead of using `isDeleted.or`, this should test whether `isDeleted`
is `null` and either initialize `isDeleted` or call `isDeleted.or`.
--
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]
