From ac2bda62af967e05d3a88e13f0cb7bf12597d052 Mon Sep 17 00:00:00 2001 From: rahulsmahadev Date: Wed, 10 Jun 2026 05:50:12 +0000 Subject: [PATCH 1/2] Core: Disallow setting main branch ref to a tag The main-branch special case in TableMetadata.Builder#setRef sets currentSnapshotId and appends to the snapshot log without checking the ref type, so a tag named main can be committed (for example createTag("main", id) on a table whose only snapshots are staged). The spec requires main to always be a branch, and once such metadata is written every subsequent write fails with "Cannot update branch: main is a tag". Reject the tag in setRef before any builder state is mutated. Co-authored-by: Isaac --- .../org/apache/iceberg/TableMetadata.java | 5 +++ .../apache/iceberg/TestSnapshotManager.java | 19 ++++++++++ .../org/apache/iceberg/TestTableMetadata.java | 38 +++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index 5b56ca7e882c..3560140a7f19 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -1327,6 +1327,11 @@ public Builder setRef(String name, SnapshotRef ref) { Snapshot snapshot = snapshotsById.get(snapshotId); ValidationException.check( snapshot != null, "Cannot set %s to unknown snapshot: %s", name, snapshotId); + ValidationException.check( + !SnapshotRef.MAIN_BRANCH.equals(name) || ref.isBranch(), + "Cannot set %s to a tag: %s must be a branch", + SnapshotRef.MAIN_BRANCH, + SnapshotRef.MAIN_BRANCH); if (SnapshotRef.MAIN_BRANCH.equals(name)) { this.currentSnapshotId = ref.snapshotId(); diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java index 9f612383d5ba..d166e190064d 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java @@ -323,6 +323,25 @@ public void testCreateTagFailsWhenRefAlreadyExists() { .hasMessage("Ref tag2 already exists"); } + @TestTemplate + public void testCreateTagNamedMainFails() { + // stage a snapshot so that the table has a snapshot but no main branch ref yet + table.newAppend().appendFile(FILE_A).stageOnly().commit(); + Snapshot stagedSnapshot = Iterables.getOnlyElement(table.snapshots()); + + assertThatThrownBy( + () -> + table + .manageSnapshots() + .createTag(SnapshotRef.MAIN_BRANCH, stagedSnapshot.snapshotId()) + .commit()) + .isInstanceOf(ValidationException.class) + .hasMessage("Cannot set main to a tag: main must be a branch"); + + // the failed commit must not have created the main ref + assertThat(table.ops().refresh().ref(SnapshotRef.MAIN_BRANCH)).isNull(); + } + @TestTemplate public void testRemoveBranch() { table.newAppend().appendFile(FILE_A).commit(); diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 8e3f0e638f28..c7b16a4be0b0 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -2148,4 +2148,42 @@ public void testAddSnapshotWithStaleFirstRowIdIsRetryable() { .isInstanceOf(RetryableValidationException.class) .hasMessageContaining("Cannot add a snapshot, first-row-id is behind table next-row-id"); } + + @Test + public void testSetRefRejectsTagForMainBranch() { + TableMetadata base = + TableMetadata.newTableMetadata( + TEST_SCHEMA, PartitionSpec.unpartitioned(), "location", ImmutableMap.of()); + + Snapshot snapshot = + new BaseSnapshot( + 1, + 1L, + null, + System.currentTimeMillis(), + null, + null, + null, + "file:/s1.avro", + null, + null, + null); + TableMetadata withSnapshot = TableMetadata.buildFrom(base).addSnapshot(snapshot).build(); + + assertThatThrownBy( + () -> + TableMetadata.buildFrom(withSnapshot) + .setRef( + SnapshotRef.MAIN_BRANCH, + SnapshotRef.tagBuilder(snapshot.snapshotId()).build())) + .isInstanceOf(ValidationException.class) + .hasMessage("Cannot set main to a tag: main must be a branch"); + + // any other ref name can still be a tag + TableMetadata withTag = + TableMetadata.buildFrom(withSnapshot) + .setRef("tag1", SnapshotRef.tagBuilder(snapshot.snapshotId()).build()) + .build(); + assertThat(withTag.ref("tag1").isTag()).isTrue(); + } } From f0467609e839734cf530258783ea158f0980d2d7 Mon Sep 17 00:00:00 2001 From: rahulsmahadev Date: Fri, 12 Jun 2026 05:08:29 +0000 Subject: [PATCH 2/2] Address review comment: drop repeated 'main' in error message Co-authored-by: Isaac --- core/src/main/java/org/apache/iceberg/TableMetadata.java | 3 +-- core/src/test/java/org/apache/iceberg/TestSnapshotManager.java | 2 +- core/src/test/java/org/apache/iceberg/TestTableMetadata.java | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index 3560140a7f19..b811d8c42f15 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -1329,8 +1329,7 @@ public Builder setRef(String name, SnapshotRef ref) { snapshot != null, "Cannot set %s to unknown snapshot: %s", name, snapshotId); ValidationException.check( !SnapshotRef.MAIN_BRANCH.equals(name) || ref.isBranch(), - "Cannot set %s to a tag: %s must be a branch", - SnapshotRef.MAIN_BRANCH, + "Cannot set %s to a tag, it must be a branch", SnapshotRef.MAIN_BRANCH); if (SnapshotRef.MAIN_BRANCH.equals(name)) { diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java index d166e190064d..f9266c7a81bc 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java @@ -336,7 +336,7 @@ public void testCreateTagNamedMainFails() { .createTag(SnapshotRef.MAIN_BRANCH, stagedSnapshot.snapshotId()) .commit()) .isInstanceOf(ValidationException.class) - .hasMessage("Cannot set main to a tag: main must be a branch"); + .hasMessage("Cannot set main to a tag, it must be a branch"); // the failed commit must not have created the main ref assertThat(table.ops().refresh().ref(SnapshotRef.MAIN_BRANCH)).isNull(); diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index c7b16a4be0b0..ce176dd2bee9 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -2177,7 +2177,7 @@ public void testSetRefRejectsTagForMainBranch() { SnapshotRef.MAIN_BRANCH, SnapshotRef.tagBuilder(snapshot.snapshotId()).build())) .isInstanceOf(ValidationException.class) - .hasMessage("Cannot set main to a tag: main must be a branch"); + .hasMessage("Cannot set main to a tag, it must be a branch"); // any other ref name can still be a tag TableMetadata withTag =