[iceberg] branch master updated: Core: Commit empty operations from SetSnapshotOperation (#5536)

[bl...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new c550e792e9 Core: Commit empty operations from SetSnapshotOperation (#5536)
c550e792e9 is described below

commit c550e792e9d114f28bbd0d7e59fb934c5a2bb440
Author: Adam Szita <40...@users.noreply.github.com>
AuthorDate: Mon Aug 15 21:43:44 2022 +0200

    Core: Commit empty operations from SetSnapshotOperation (#5536)
    
    Change-Id: I9b481ca173a12457cb6773ade0c8a1ebd0b40cbc
---
 .../main/java/org/apache/iceberg/SetSnapshotOperation.java    | 10 ++++------
 .../src/test/java/org/apache/iceberg/TestSnapshotManager.java | 11 +++++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/core/src/main/java/org/apache/iceberg/SetSnapshotOperation.java b/core/src/main/java/org/apache/iceberg/SetSnapshotOperation.java
index 5f3750b58f..0f80b4e1f2 100644
--- a/core/src/main/java/org/apache/iceberg/SetSnapshotOperation.java
+++ b/core/src/main/java/org/apache/iceberg/SetSnapshotOperation.java
@@ -123,12 +123,10 @@ class SetSnapshotOperation implements PendingUpdate<Snapshot> {
                       .setBranchSnapshot(snapshot.snapshotId(), SnapshotRef.MAIN_BRANCH)
                       .build();
 
-              if (updated.changes().isEmpty()) {
-                // do not commit if the metadata has not changed. for example, this may happen when
-                // setting the current
-                // snapshot to an ID that is already current. note that this check uses identity.
-                return;
-              }
+              // Do commit this operation even if the metadata has not changed, as we need to
+              // advance the hasLastOpCommited for the transaction's commit to work properly.
+              // (Without any other operations in the transaction, the commitTransaction() call
+              // will be a no-op anyway)
 
               // if the table UUID is missing, add it here. the UUID will be re-created each time
               // this operation retries
diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java
index 2b8b3cae3e..790fe1030f 100644
--- a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java
+++ b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java
@@ -621,4 +621,15 @@ public class TestSnapshotManager extends TableTestBase {
     Assert.assertEquals(SnapshotRef.branchBuilder(1).build(), actualBranch);
     Assert.assertEquals(SnapshotRef.tagBuilder(1).build(), actualTag);
   }
+
+  @Test
+  public void testAttemptToRollbackToCurrentSnapshot() {
+    table.newAppend().appendFile(FILE_A).commit();
+
+    long currentSnapshotTimestampPlus100 = table.currentSnapshot().timestampMillis() + 100;
+    table.manageSnapshots().rollbackToTime(currentSnapshotTimestampPlus100).commit();
+
+    long currentSnapshotId = table.currentSnapshot().snapshotId();
+    table.manageSnapshots().rollbackTo(currentSnapshotId).commit();
+  }
 }