[GitHub] [incubator-iceberg] xabriel commented on a change in pull request #497: Support retaining last N snapshots

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

xabriel commented on a change in pull request #497: Support retaining last N snapshots
URL: https://github.com/apache/incubator-iceberg/pull/497#discussion_r328713783
 
 

 ##########
 File path: core/src/main/java/org/apache/iceberg/RemoveSnapshots.java
 ##########
 @@ -310,6 +323,19 @@ private void deleteDataFiles(Set<String> manifestsToScan, Set<String> manifestsT
     return filesToDelete;
   }
 
+  private Snapshot findNthSnapshot(int snapshotIndex) {
+    Snapshot lastSnapshot = base.currentSnapshot();
+    int snapshotCount = 1;
+    while (lastSnapshot != null) {
+      if (snapshotCount == snapshotIndex) {
+        return lastSnapshot;
+      }
+      snapshotCount++;
+      lastSnapshot = (lastSnapshot.parentId() != null) ? base.snapshot(lastSnapshot.parentId()) : null;
 
 Review comment:
   This implementation would miss snapshots that have been rolled back, as you wouldn't be able to navigate to them via `lastSnapshot.parentId()`.
   
   See: https://iceberg.apache.org/spark/#history

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


With regards,
Apache Git Services

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