This is an automated email from the ASF dual-hosted git repository.
amoghj 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 6fc5be738d API, Core: Fix naming in fastForwardBranch/replaceBranch
APIs (#9134)
6fc5be738d is described below
commit 6fc5be738d317eb1e8a5b525f5b8e40841dee678
Author: Amogh Jahagirdar <[email protected]>
AuthorDate: Mon Nov 27 11:24:39 2023 -0800
API, Core: Fix naming in fastForwardBranch/replaceBranch APIs (#9134)
---
.../java/org/apache/iceberg/ManageSnapshots.java | 22 +++++++-------
.../java/org/apache/iceberg/SnapshotManager.java | 8 ++---
.../iceberg/UpdateSnapshotReferencesOperation.java | 35 +++++++++++-----------
.../org/apache/iceberg/TestSnapshotManager.java | 28 ++++++++++++++---
4 files changed, 57 insertions(+), 36 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/ManageSnapshots.java
b/api/src/main/java/org/apache/iceberg/ManageSnapshots.java
index 2fa60472da..986bbb6f58 100644
--- a/api/src/main/java/org/apache/iceberg/ManageSnapshots.java
+++ b/api/src/main/java/org/apache/iceberg/ManageSnapshots.java
@@ -163,26 +163,26 @@ public interface ManageSnapshots extends
PendingUpdate<Snapshot> {
ManageSnapshots replaceBranch(String name, long snapshotId);
/**
- * Replaces the branch with the given name to point to the source snapshot.
The source branch will
- * remain unchanged, the target branch will retain its retention properties.
+ * Replaces the {@code from} branch to point to the {@code to} snapshot. The
{@code to} will
+ * remain unchanged, and {@code from} branch will retain its retention
properties.
*
- * @param name Branch to replace
- * @param source Source reference for the target to be replaced with
+ * @param from Branch to replace
+ * @param to The branch {@code from} should be replaced with
* @return this for method chaining
*/
- ManageSnapshots replaceBranch(String name, String source);
+ ManageSnapshots replaceBranch(String from, String to);
/**
- * Performs a fast-forward of the given target branch up to the source
snapshot if target is an
- * ancestor of source. The source branch will remain unchanged, the target
branch will retain its
+ * Performs a fast-forward of {@code from} up to the {@code to} snapshot if
{@code from} is an
+ * ancestor of {@code to}. The {@code to} will remain unchanged, and {@code
from} will retain its
* retention properties.
*
- * @param name Branch to fast-forward
- * @param source Source reference for the target to be fast forwarded to
+ * @param from Branch to fast-forward
+ * @param to Ref for the {@code from} branch to be fast forwarded to
* @return this for method chaining
- * @throws IllegalArgumentException if the target branch is not an ancestor
of source
+ * @throws IllegalArgumentException if {@code from} is not an ancestor of
{@code to}
*/
- ManageSnapshots fastForwardBranch(String name, String source);
+ ManageSnapshots fastForwardBranch(String from, String to);
/**
* Updates the minimum number of snapshots to keep for a branch.
diff --git a/core/src/main/java/org/apache/iceberg/SnapshotManager.java
b/core/src/main/java/org/apache/iceberg/SnapshotManager.java
index c9774f3b92..bb7ca4b11c 100644
--- a/core/src/main/java/org/apache/iceberg/SnapshotManager.java
+++ b/core/src/main/java/org/apache/iceberg/SnapshotManager.java
@@ -137,14 +137,14 @@ public class SnapshotManager implements ManageSnapshots {
}
@Override
- public ManageSnapshots replaceBranch(String name, String source) {
- updateSnapshotReferencesOperation().replaceBranch(name, source);
+ public ManageSnapshots replaceBranch(String from, String to) {
+ updateSnapshotReferencesOperation().replaceBranch(from, to);
return this;
}
@Override
- public ManageSnapshots fastForwardBranch(String name, String source) {
- updateSnapshotReferencesOperation().fastForward(name, source);
+ public ManageSnapshots fastForwardBranch(String from, String to) {
+ updateSnapshotReferencesOperation().fastForward(from, to);
return this;
}
diff --git
a/core/src/main/java/org/apache/iceberg/UpdateSnapshotReferencesOperation.java
b/core/src/main/java/org/apache/iceberg/UpdateSnapshotReferencesOperation.java
index b87bac2f01..2c3c6c1f7e 100644
---
a/core/src/main/java/org/apache/iceberg/UpdateSnapshotReferencesOperation.java
+++
b/core/src/main/java/org/apache/iceberg/UpdateSnapshotReferencesOperation.java
@@ -106,40 +106,41 @@ class UpdateSnapshotReferencesOperation implements
PendingUpdate<Map<String, Sna
return this;
}
- public UpdateSnapshotReferencesOperation replaceBranch(String name, String
source) {
- return replaceBranch(name, source, false);
+ public UpdateSnapshotReferencesOperation replaceBranch(String from, String
to) {
+ return replaceBranch(from, to, false);
}
- public UpdateSnapshotReferencesOperation fastForward(String name, String
source) {
- return replaceBranch(name, source, true);
+ public UpdateSnapshotReferencesOperation fastForward(String from, String to)
{
+ return replaceBranch(from, to, true);
}
private UpdateSnapshotReferencesOperation replaceBranch(
- String name, String source, boolean fastForward) {
- Preconditions.checkNotNull(name, "Target branch cannot be null");
- Preconditions.checkNotNull(source, "Source ref cannot be null");
- SnapshotRef sourceRef = updatedRefs.get(source);
- SnapshotRef refToUpdate = updatedRefs.get(name);
- Preconditions.checkArgument(refToUpdate != null, "Target branch does not
exist: %s", name);
- Preconditions.checkArgument(sourceRef != null, "Ref does not exist: %s",
source);
- Preconditions.checkArgument(refToUpdate.isBranch(), "Ref %s is a tag not a
branch", name);
+ String from, String to, boolean fastForward) {
+ Preconditions.checkNotNull(from, "Branch to update cannot be null");
+ Preconditions.checkNotNull(to, "Destination ref cannot be null");
+ SnapshotRef branchToUpdate = updatedRefs.get(from);
+ SnapshotRef toRef = updatedRefs.get(to);
+ Preconditions.checkArgument(
+ branchToUpdate != null, "Branch to update does not exist: %s", from);
+ Preconditions.checkArgument(toRef != null, "Ref does not exist: %s", to);
+ Preconditions.checkArgument(branchToUpdate.isBranch(), "Ref %s is a tag
not a branch", from);
// Nothing to replace
- if (sourceRef.snapshotId() == refToUpdate.snapshotId()) {
+ if (toRef.snapshotId() == branchToUpdate.snapshotId()) {
return this;
}
- SnapshotRef updatedRef = SnapshotRef.builderFrom(refToUpdate,
sourceRef.snapshotId()).build();
+ SnapshotRef updatedRef = SnapshotRef.builderFrom(branchToUpdate,
toRef.snapshotId()).build();
if (fastForward) {
boolean targetIsAncestor =
SnapshotUtil.isAncestorOf(
- sourceRef.snapshotId(), refToUpdate.snapshotId(),
base::snapshot);
+ toRef.snapshotId(), branchToUpdate.snapshotId(), base::snapshot);
Preconditions.checkArgument(
- targetIsAncestor, "Cannot fast-forward: %s is not an ancestor of
%s", name, source);
+ targetIsAncestor, "Cannot fast-forward: %s is not an ancestor of
%s", from, to);
}
- updatedRefs.put(name, updatedRef);
+ updatedRefs.put(from, updatedRef);
return this;
}
diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java
b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java
index d497dbd360..d561d697d3 100644
--- a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java
+++ b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java
@@ -409,15 +409,15 @@ public class TestSnapshotManager extends TableTestBase {
}
@Test
- public void testReplaceBranchNonExistingTargetBranchFails() {
+ public void testReplaceBranchNonExistingBranchToUpdateFails() {
Assertions.assertThatThrownBy(
() -> table.manageSnapshots().replaceBranch("non-existing",
"other-branch").commit())
.isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Target branch does not exist: non-existing");
+ .hasMessage("Branch to update does not exist: non-existing");
}
@Test
- public void testReplaceBranchNonExistingSourceFails() {
+ public void testReplaceBranchNonExistingToBranchFails() {
table.newAppend().appendFile(FILE_A).commit();
long snapshotId = table.currentSnapshot().snapshotId();
table.manageSnapshots().createBranch("branch1", snapshotId).commit();
@@ -427,6 +427,26 @@ public class TestSnapshotManager extends TableTestBase {
.hasMessage("Ref does not exist: non-existing");
}
+ @Test
+ public void testFastForwardBranchNonExistingFromBranchFails() {
+ Assertions.assertThatThrownBy(
+ () ->
+ table.manageSnapshots().fastForwardBranch("non-existing",
"other-branch").commit())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Branch to update does not exist: non-existing");
+ }
+
+ @Test
+ public void testFastForwardBranchNonExistingToFails() {
+ table.newAppend().appendFile(FILE_A).commit();
+ long snapshotId = table.currentSnapshot().snapshotId();
+ table.manageSnapshots().createBranch("branch1", snapshotId).commit();
+ Assertions.assertThatThrownBy(
+ () -> table.manageSnapshots().fastForwardBranch("branch1",
"non-existing").commit())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Ref does not exist: non-existing");
+ }
+
@Test
public void testFastForward() {
table.newAppend().appendFile(FILE_A).commit();
@@ -445,7 +465,7 @@ public class TestSnapshotManager extends TableTestBase {
}
@Test
- public void testFastForwardWhenTargetIsNotAncestorFails() {
+ public void testFastForwardWhenFromIsNotAncestorFails() {
table.newAppend().appendFile(FILE_A).commit();
table.newAppend().appendFile(FILE_B).set("wap.id",
"123456789").stageOnly().commit();
