amogh-jahagirdar commented on code in PR #10983:
URL: https://github.com/apache/iceberg/pull/10983#discussion_r1733319305


##########
core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java:
##########
@@ -327,4 +342,34 @@ private Set<String> findFilesToDelete(
 
     return filesToDelete;
   }
+
+  private boolean isSafeToDelete(
+      ManifestEntry<?> entry,
+      Map<Long, Long> validSnapshotIdToSequenceNumberMap,
+      Map<Long, Long> expiredSnapshotIdToSequenceNumberMap) {
+    if (validSnapshotIdToSequenceNumberMap.containsKey(entry.snapshotId())
+        || 
!expiredSnapshotIdToSequenceNumberMap.containsKey(entry.snapshotId())) {
+      return false;
+    }
+
+    // The file in DELETE entry can be deleted if there are no Snapshots older 
than
+    // this one
+    if (validSnapshotIdToSequenceNumberMap.keySet().stream()
+        .noneMatch(snapshotId -> snapshotId < entry.snapshotId())) {
+      return true;

Review Comment:
   Ah good point, yes the inheritance chain isn't guaranteed since the parent 
also could've been expired. 
   OK I'll need to think about this a bit more deeply then. On the surface just 
seems like if someone is expiring a specific snapshot where there's a deleted 
entry we may want to do the reachability analysis



-- 
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