singhpk234 commented on code in PR #6752:
URL: https://github.com/apache/iceberg/pull/6752#discussion_r1099477153
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestBranchDDL.java:
##########
@@ -206,9 +212,54 @@ public void testCreateBranchUseCustomMaxRefAge() throws
NoSuchTableException {
tableName, branchName, maxRefAge));
}
- private Table createDefaultTableAndInsert2Row() throws NoSuchTableException {
- sql("CREATE TABLE %s (id INT, data STRING) USING iceberg", tableName);
+ @Test
+ public void testDropBranch() throws NoSuchTableException {
+ insertRows();
+
+ Table table = validationCatalog.loadTable(tableIdent);
+ String branchName = "b1";
+ table.manageSnapshots().createBranch(branchName,
table.currentSnapshot().snapshotId()).commit();
+ SnapshotRef ref = table.refs().get(branchName);
+ Assert.assertEquals(table.currentSnapshot().snapshotId(),
ref.snapshotId());
+
+ sql("ALTER TABLE %s DROP BRANCH %s", tableName, branchName);
+ table.refresh();
+
+ ref = table.refs().get(branchName);
+ Assert.assertNull(ref);
+ }
+
+ @Test
+ public void testDropBranchDoesNotExist() {
+ AssertHelpers.assertThrows(
+ "Cannot perform drop branch on branch which does not exist",
+ IllegalArgumentException.class,
+ "Branch does not exist: someBranch",
+ () -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, "someBranch"));
+ }
+
+ @Test
+ public void testDropMainBranchFails() {
+ AssertHelpers.assertThrows(
+ "Cannot drop the main branch",
+ IllegalArgumentException.class,
+ "Cannot remove main branch",
+ () -> sql("ALTER TABLE %s DROP BRANCH main", tableName, "someBranch"));
Review Comment:
do we need "someBranch" to be passed explicitly as we already made the name
as main ?
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestBranchDDL.java:
##########
@@ -206,9 +212,54 @@ public void testCreateBranchUseCustomMaxRefAge() throws
NoSuchTableException {
tableName, branchName, maxRefAge));
}
- private Table createDefaultTableAndInsert2Row() throws NoSuchTableException {
- sql("CREATE TABLE %s (id INT, data STRING) USING iceberg", tableName);
+ @Test
+ public void testDropBranch() throws NoSuchTableException {
+ insertRows();
+
+ Table table = validationCatalog.loadTable(tableIdent);
+ String branchName = "b1";
+ table.manageSnapshots().createBranch(branchName,
table.currentSnapshot().snapshotId()).commit();
+ SnapshotRef ref = table.refs().get(branchName);
+ Assert.assertEquals(table.currentSnapshot().snapshotId(),
ref.snapshotId());
+
+ sql("ALTER TABLE %s DROP BRANCH %s", tableName, branchName);
+ table.refresh();
+
+ ref = table.refs().get(branchName);
+ Assert.assertNull(ref);
+ }
+
+ @Test
+ public void testDropBranchDoesNotExist() {
+ AssertHelpers.assertThrows(
+ "Cannot perform drop branch on branch which does not exist",
+ IllegalArgumentException.class,
+ "Branch does not exist: someBranch",
+ () -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, "someBranch"));
Review Comment:
[nit] liked the branch name of 'nonExistingBranch' used with IF EXISTS
clause, can we use it here as well ?
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestBranchDDL.java:
##########
@@ -206,9 +212,54 @@ public void testCreateBranchUseCustomMaxRefAge() throws
NoSuchTableException {
tableName, branchName, maxRefAge));
}
- private Table createDefaultTableAndInsert2Row() throws NoSuchTableException {
- sql("CREATE TABLE %s (id INT, data STRING) USING iceberg", tableName);
+ @Test
+ public void testDropBranch() throws NoSuchTableException {
+ insertRows();
+
+ Table table = validationCatalog.loadTable(tableIdent);
+ String branchName = "b1";
+ table.manageSnapshots().createBranch(branchName,
table.currentSnapshot().snapshotId()).commit();
+ SnapshotRef ref = table.refs().get(branchName);
+ Assert.assertEquals(table.currentSnapshot().snapshotId(),
ref.snapshotId());
+
+ sql("ALTER TABLE %s DROP BRANCH %s", tableName, branchName);
+ table.refresh();
+
+ ref = table.refs().get(branchName);
+ Assert.assertNull(ref);
+ }
+
+ @Test
+ public void testDropBranchDoesNotExist() {
+ AssertHelpers.assertThrows(
+ "Cannot perform drop branch on branch which does not exist",
+ IllegalArgumentException.class,
+ "Branch does not exist: someBranch",
+ () -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, "someBranch"));
+ }
+
+ @Test
+ public void testDropMainBranchFails() {
+ AssertHelpers.assertThrows(
+ "Cannot drop the main branch",
+ IllegalArgumentException.class,
+ "Cannot remove main branch",
+ () -> sql("ALTER TABLE %s DROP BRANCH main", tableName, "someBranch"));
+ }
+
+ @Test
+ public void testDropBranchIfExists() {
+ String branchName = "nonExistingBranch";
+ Table table = validationCatalog.loadTable(tableIdent);
+
+ sql("ALTER TABLE %s DROP BRANCH IF EXISTS %s", tableName, branchName);
+ table.refresh();
+
+ SnapshotRef ref = table.refs().get(branchName);
+ Assert.assertNull(ref);
Review Comment:
[minor] do we need to assert this considering the branch was non-existing
already ?
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
