[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #6634: Core: Fix for deleting files when commiting transactions with multiple branches

["amogh-jahagirdar (via GitHub)" <gi...@apache.org>]

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


##########
core/src/main/java/org/apache/iceberg/BaseTransaction.java:
##########
@@ -433,17 +425,18 @@ private void commitSimpleTransaction() {
     // the commit succeeded
 
     try {
-      if (currentSnapshotId.get() != -1) {
-        intermediateSnapshotIds.add(currentSnapshotId.get());
+      // clean up the data files that were deleted by each operation. first, get the list of
+      // committed manifests to ensure that no committed manifest is deleted.
+      // A manifest could be deleted in one successful operation commit, but reused in another
+      // successful commit of that operation if the whole transaction is retried.
+      Set<Long> currentSnapshots = Sets.newHashSet();

Review Comment:
   Oh yes `newSnapshots` is much better. This is the delta from before and now.



-- 
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-unsubscribe@iceberg.apache.org

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


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