From f913780a01bf469d68d22581d75f71779d0fa4c4 Mon Sep 17 00:00:00 2001 From: userzhy <48518279+userzhy@users.noreply.github.com> Date: Thu, 25 Dec 2025 09:07:00 +0000 Subject: [PATCH 1/3] [core] Prevent tag deletion when referenced by branches This closes #6272. When a branch is created from a tag, the tag's snapshot information is copied to the branch directory. However, deleting the tag could cause the snapshot expiration process to clean up data files that are still needed by the branch, making the branch unqueryable. This change adds a check in deleteTag() to verify if any branches were created from the tag before allowing deletion. If branches reference the tag, the deletion is blocked with an error message listing the referencing branches. Changes: - Added branchesCreatedFromTag() method to BranchManager interface with default empty implementation for backward compatibility - Implemented branchesCreatedFromTag() in FileSystemBranchManager to check each branch's tag directory for the specified tag - Added validation in AbstractFileStoreTable.deleteTag() to prevent deletion of tags that are still referenced by branches - Added testDeleteTagReferencedByBranch() test case to verify the fix --- .../paimon/table/AbstractFileStoreTable.java | 8 ++++ .../apache/paimon/utils/BranchManager.java | 10 +++++ .../paimon/utils/FileSystemBranchManager.java | 12 ++++++ .../operation/LocalOrphanFilesCleanTest.java | 6 +++ .../paimon/rest/RESTSimpleTableTest.java | 7 ++++ .../paimon/table/SimpleTableTestBase.java | 40 +++++++++++++++++++ 6 files changed, 83 insertions(+) diff --git a/paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java b/paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java index 35949062e307..a17f55469616 100644 --- a/paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java +++ b/paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java @@ -656,6 +656,14 @@ public void replaceTag( @Override public void deleteTag(String tagName) { + List referencingBranches = branchManager().branchesCreatedFromTag(tagName); + if (!referencingBranches.isEmpty()) { + throw new IllegalStateException( + String.format( + "Cannot delete tag '%s' because it is still referenced by branches: %s. " + + "Please delete these branches first.", + tagName, referencingBranches)); + } tagManager() .deleteTag( tagName, diff --git a/paimon-core/src/main/java/org/apache/paimon/utils/BranchManager.java b/paimon-core/src/main/java/org/apache/paimon/utils/BranchManager.java index fc3defa8f346..74831fc890ea 100644 --- a/paimon-core/src/main/java/org/apache/paimon/utils/BranchManager.java +++ b/paimon-core/src/main/java/org/apache/paimon/utils/BranchManager.java @@ -42,6 +42,16 @@ public interface BranchManager { List branches(); + /** + * Get all branches that were created based on the given tag. + * + * @param tagName the name of the tag to check + * @return list of branch names that reference the given tag + */ + default List branchesCreatedFromTag(String tagName) { + return java.util.Collections.emptyList(); + } + default boolean branchExists(String branchName) { return branches().contains(branchName); } diff --git a/paimon-core/src/main/java/org/apache/paimon/utils/FileSystemBranchManager.java b/paimon-core/src/main/java/org/apache/paimon/utils/FileSystemBranchManager.java index 7a1bdbef2905..14cf837cf75f 100644 --- a/paimon-core/src/main/java/org/apache/paimon/utils/FileSystemBranchManager.java +++ b/paimon-core/src/main/java/org/apache/paimon/utils/FileSystemBranchManager.java @@ -215,6 +215,18 @@ public List branches() { } } + @Override + public List branchesCreatedFromTag(String tagName) { + List result = new java.util.ArrayList<>(); + for (String branchName : branches()) { + TagManager branchTagManager = tagManager.copyWithBranch(branchName); + if (branchTagManager.tagExists(tagName)) { + result.add(branchName); + } + } + return result; + } + private void copySchemasToBranch(String branchName, long schemaId) throws IOException { for (int i = 0; i <= schemaId; i++) { if (schemaManager.schemaExists(i)) { diff --git a/paimon-core/src/test/java/org/apache/paimon/operation/LocalOrphanFilesCleanTest.java b/paimon-core/src/test/java/org/apache/paimon/operation/LocalOrphanFilesCleanTest.java index 9d3a0e9476b4..e960d1e5ca8a 100644 --- a/paimon-core/src/test/java/org/apache/paimon/operation/LocalOrphanFilesCleanTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/operation/LocalOrphanFilesCleanTest.java @@ -185,6 +185,9 @@ public void normallyRemoving(Path dataPath) throws Throwable { expireOptions.set(CoreOptions.SNAPSHOT_NUM_RETAINED_MAX, snapshotCount - expired); table.copy(expireOptions.toMap()).newCommit("").expireSnapshots(); + // delete branch1 first before deleting tags + table.deleteBranch("branch1"); + // randomly delete tags List deleteTags = Collections.emptyList(); deleteTags = randomlyPick(allTags); @@ -288,6 +291,9 @@ public void testNormallyRemovingMixedWithExternalPath() throws Throwable { expireOptions.set(CoreOptions.SNAPSHOT_NUM_RETAINED_MAX, snapshotCount - expired); table.copy(expireOptions.toMap()).newCommit("").expireSnapshots(); + // delete branch1 first before deleting tags + table.deleteBranch("branch1"); + // randomly delete tags List deleteTags = Collections.emptyList(); deleteTags = randomlyPick(allTags); diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/RESTSimpleTableTest.java b/paimon-core/src/test/java/org/apache/paimon/rest/RESTSimpleTableTest.java index 93c8437955c5..7a9ef4fdf2f8 100644 --- a/paimon-core/src/test/java/org/apache/paimon/rest/RESTSimpleTableTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/rest/RESTSimpleTableTest.java @@ -32,6 +32,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; import java.io.IOException; import java.util.ArrayList; @@ -106,4 +108,9 @@ protected FileStoreTable createBranchTable(String branch) throws Exception { new Identifier( identifier.getDatabaseName(), identifier.getTableName(), branch)); } + + @Test + @Disabled("REST catalog does not support branchesCreatedFromTag yet") + @Override + public void testDeleteTagReferencedByBranch() {} } diff --git a/paimon-core/src/test/java/org/apache/paimon/table/SimpleTableTestBase.java b/paimon-core/src/test/java/org/apache/paimon/table/SimpleTableTestBase.java index 69cf5441f571..aadec90bf804 100644 --- a/paimon-core/src/test/java/org/apache/paimon/table/SimpleTableTestBase.java +++ b/paimon-core/src/test/java/org/apache/paimon/table/SimpleTableTestBase.java @@ -1174,6 +1174,46 @@ public void testDeleteBranch() throws Exception { table.deleteBranch("fallback"); } + @Test + public void testDeleteTagReferencedByBranch() throws Exception { + FileStoreTable table = createFileStoreTable(); + + try (StreamTableWrite write = table.newWrite(commitUser); + StreamTableCommit commit = table.newCommit(commitUser)) { + write.write(rowData(1, 10, 100L)); + commit.commit(0, write.prepareCommit(false, 1)); + } + + table.createTag("tag1", 1); + table.createBranch("branch1", "tag1"); + + // verify that deleting a tag referenced by a branch fails + assertThatThrownBy(() -> table.deleteTag("tag1")) + .satisfies( + anyCauseMatches( + IllegalStateException.class, + "Cannot delete tag 'tag1' because it is still referenced by branches: [branch1]")); + + // create another branch from the same tag + table.createBranch("branch2", "tag1"); + + // verify that deleting the tag still fails and shows both branches + assertThatThrownBy(() -> table.deleteTag("tag1")) + .satisfies( + anyCauseMatches( + IllegalStateException.class, + "Cannot delete tag 'tag1' because it is still referenced by branches:")); + + // delete both branches + table.deleteBranch("branch1"); + table.deleteBranch("branch2"); + + // verify that deleting the tag succeeds after branches are deleted + table.deleteTag("tag1"); + TagManager tagManager = new TagManager(table.fileIO(), table.location()); + assertThat(tagManager.tagExists("tag1")).isFalse(); + } + @Test public void testFastForward() throws Exception { FileStoreTable table = createFileStoreTable(); From cf0f6a1534ab8285fa85d4041dcb038e15adbf36 Mon Sep 17 00:00:00 2001 From: userzhy <48518279+userzhy@users.noreply.github.com> Date: Thu, 25 Dec 2025 12:49:06 +0000 Subject: [PATCH 2/3] fix: remove branch files from manuallyAddedFiles after deleteBranch in LocalOrphanFilesCleanTest --- .../paimon/operation/LocalOrphanFilesCleanTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/paimon-core/src/test/java/org/apache/paimon/operation/LocalOrphanFilesCleanTest.java b/paimon-core/src/test/java/org/apache/paimon/operation/LocalOrphanFilesCleanTest.java index e960d1e5ca8a..0d0692ee692a 100644 --- a/paimon-core/src/test/java/org/apache/paimon/operation/LocalOrphanFilesCleanTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/operation/LocalOrphanFilesCleanTest.java @@ -188,6 +188,11 @@ public void normallyRemoving(Path dataPath) throws Throwable { // delete branch1 first before deleting tags table.deleteBranch("branch1"); + // deleteBranch also removes the manually added files in branch directory, + // so we need to remove them from manuallyAddedFiles to avoid validation failure + String branchPathPrefix = branchPath(tablePath, "branch1").toString(); + manuallyAddedFiles.removeIf(path -> path.toString().startsWith(branchPathPrefix)); + // randomly delete tags List deleteTags = Collections.emptyList(); deleteTags = randomlyPick(allTags); @@ -294,6 +299,11 @@ public void testNormallyRemovingMixedWithExternalPath() throws Throwable { // delete branch1 first before deleting tags table.deleteBranch("branch1"); + // deleteBranch also removes the manually added files in branch directory, + // so we need to remove them from manuallyAddedFiles to avoid validation failure + String branchPathPrefix = branchPath(tablePath, "branch1").toString(); + manuallyAddedFiles.removeIf(path -> path.toString().startsWith(branchPathPrefix)); + // randomly delete tags List deleteTags = Collections.emptyList(); deleteTags = randomlyPick(allTags); From a67b45954e03eb8acb9641cffa55ca7297c749d9 Mon Sep 17 00:00:00 2001 From: userzhy <48518279+userzhy@users.noreply.github.com> Date: Thu, 25 Dec 2025 13:36:09 +0000 Subject: [PATCH 3/3] chore: retrigger CI