wypoon commented on code in PR #10935:
URL: https://github.com/apache/iceberg/pull/10935#discussion_r1726379602


##########
data/src/main/java/org/apache/iceberg/data/DeleteFilter.java:
##########
@@ -197,31 +197,31 @@ record -> 
deleteSet.contains(projectRow.wrap(asStructLike(record)));
   }
 
   public CloseableIterable<T> findEqualityDeleteRows(CloseableIterable<T> 
records) {
-    // Predicate to test whether a row has been deleted by equality deletions.
-    Predicate<T> deletedRows = 
applyEqDeletes().stream().reduce(Predicate::or).orElse(t -> false);
-
-    return CloseableIterable.filter(records, deletedRows);
+    return CloseableIterable.filter(records, isEqDeleted());
   }
 
   private CloseableIterable<T> applyEqDeletes(CloseableIterable<T> records) {
-    Predicate<T> isEqDeleted = 
applyEqDeletes().stream().reduce(Predicate::or).orElse(t -> false);
-
-    return createDeleteIterable(records, isEqDeleted);
+    return createDeleteIterable(records, isEqDeleted());
   }
 
   protected void markRowDeleted(T item) {
     throw new UnsupportedOperationException(
         this.getClass().getName() + " does not implement markRowDeleted");
   }
 
-  public Predicate<T> eqDeletedRowFilter() {
+  // Predicate to test whether a row has been deleted by equality deletes
+  public Predicate<T> isEqDeleted() {
     if (eqDeleteRows == null) {
-      eqDeleteRows =
-          
applyEqDeletes().stream().map(Predicate::negate).reduce(Predicate::and).orElse(t
 -> true);
+      eqDeleteRows = applyEqDeletes().stream().reduce(Predicate::or).orElse(t 
-> false);
     }
     return eqDeleteRows;
   }
 
+  // Predicate to test whether a row has not been deleted by equality deletes
+  public Predicate<T> eqDeletedRowFilter() {
+    return isEqDeleted().negate();
+  }

Review Comment:
   This is only used in one place, 
`ColumnarBatchReader.ColumnarBatchLoader::applyEqDelete(ColumnarBatch)`.
   I propose deprecating this and changing the call to the negation of 
`isEqDeleted()` instead. `isEqDeleted()` is more useful. I'll do that in a 
separate PR.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to