(iceberg) branch main updated: Core: Fix logic for determining set of committed files in BaseTransaction when there are no new snapshots (#9221)

[blue]

This is an automated email from the ASF dual-hosted git repository.

blue pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/main by this push:
     new faa8b5075c Core: Fix logic for determining set of committed files in 
BaseTransaction when there are no new snapshots (#9221)
faa8b5075c is described below

commit faa8b5075cb70d1cebea54700e39f038c623f08e
Author: Amogh Jahagirdar <am...@tabular.io>
AuthorDate: Tue Dec 5 13:13:33 2023 -0800

    Core: Fix logic for determining set of committed files in BaseTransaction 
when there are no new snapshots (#9221)
---
 .../java/org/apache/iceberg/BaseTransaction.java   | 29 ++++++++++++++--------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/core/src/main/java/org/apache/iceberg/BaseTransaction.java 
b/core/src/main/java/org/apache/iceberg/BaseTransaction.java
index 018f70eb16..30103fd87f 100644
--- a/core/src/main/java/org/apache/iceberg/BaseTransaction.java
+++ b/core/src/main/java/org/apache/iceberg/BaseTransaction.java
@@ -45,6 +45,7 @@ import org.apache.iceberg.metrics.LoggingMetricsReporter;
 import org.apache.iceberg.metrics.MetricsReporter;
 import 
org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting;
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 import org.apache.iceberg.relocated.com.google.common.collect.Sets;
 import org.apache.iceberg.util.PropertyUtil;
@@ -446,16 +447,20 @@ public class BaseTransaction implements Transaction {
       }
 
       Set<String> committedFiles = committedFiles(ops, newSnapshots);
-      // delete all of the files that were deleted in the most recent set of 
operation commits
-      Tasks.foreach(deletedFiles)
-          .suppressFailureWhenFinished()
-          .onFailure((file, exc) -> LOG.warn("Failed to delete uncommitted 
file: {}", file, exc))
-          .run(
-              path -> {
-                if (committedFiles == null || !committedFiles.contains(path)) {
-                  ops.io().deleteFile(path);
-                }
-              });
+      if (committedFiles != null) {
+        // delete all of the files that were deleted in the most recent set of 
operation commits
+        Tasks.foreach(deletedFiles)
+            .suppressFailureWhenFinished()
+            .onFailure((file, exc) -> LOG.warn("Failed to delete uncommitted 
file: {}", file, exc))
+            .run(
+                path -> {
+                  if (!committedFiles.contains(path)) {
+                    ops.io().deleteFile(path);
+                  }
+                });
+      } else {
+        LOG.warn("Failed to load metadata for a committed snapshot, skipping 
clean-up");
+      }
 
     } catch (RuntimeException e) {
       LOG.warn("Failed to load committed metadata, skipping clean-up", e);
@@ -502,9 +507,11 @@ public class BaseTransaction implements Transaction {
     }
   }
 
+  // committedFiles returns null whenever the set of committed files
+  // cannot be determined from the provided snapshots
   private static Set<String> committedFiles(TableOperations ops, Set<Long> 
snapshotIds) {
     if (snapshotIds.isEmpty()) {
-      return null;
+      return ImmutableSet.of();
     }
 
     Set<String> committedFiles = Sets.newHashSet();