This is an automated email from the ASF dual-hosted git repository. fokko pushed a commit to branch 1.7.x in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit b0b7a63ebdd45e8e5713a85f4e707a2fae55c44e Author: Yuya Ebihara <[email protected]> AuthorDate: Wed Dec 25 00:24:22 2024 +0900 Core: Don't clear snapshotLog in `TableMetadata.removeRef` (#11779) --- .../java/org/apache/iceberg/TableMetadata.java | 1 - .../java/org/apache/iceberg/TestTableMetadata.java | 18 ++++++++++++ .../org/apache/iceberg/catalog/CatalogTests.java | 34 ++++++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index d20dd59d2b..08a52bcfd9 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -1256,7 +1256,6 @@ public class TableMetadata implements Serializable { public Builder removeRef(String name) { if (SnapshotRef.MAIN_BRANCH.equals(name)) { this.currentSnapshotId = -1; - snapshotLog.clear(); } SnapshotRef ref = refs.remove(name); diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 71254b3abb..91c5b479a7 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -1636,6 +1636,24 @@ public class TestTableMetadata { .containsExactlyElementsOf(metadata.snapshotLog()); } + @Test + public void removeRefKeepsSnapshotLog() throws Exception { + TableMetadata metadata = + TableMetadataParser.fromJson(readTableMetadataInputFile("TableMetadataV2Valid.json")); + assertThat(metadata.currentSnapshot()).isNotNull(); + assertThat(metadata.snapshots()).hasSize(2); + assertThat(metadata.snapshotLog()).hasSize(2); + + TableMetadata removeRef = + TableMetadata.buildFrom(metadata).removeRef(SnapshotRef.MAIN_BRANCH).build(); + + assertThat(removeRef.currentSnapshot()).isNull(); + assertThat(removeRef.snapshots()).hasSize(2).containsExactlyElementsOf(metadata.snapshots()); + assertThat(removeRef.snapshotLog()) + .hasSize(2) + .containsExactlyElementsOf(metadata.snapshotLog()); + } + @Test public void testConstructV3Metadata() { TableMetadata.newTableMetadata( diff --git a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java index 5402a13d7d..fb0d78d42c 100644 --- a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java +++ b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java @@ -38,6 +38,7 @@ import org.apache.iceberg.DataFiles; import org.apache.iceberg.FileScanTask; import org.apache.iceberg.FilesTable; import org.apache.iceberg.HasTableOperations; +import org.apache.iceberg.HistoryEntry; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.ReachableFileUtil; import org.apache.iceberg.ReplaceSortOrder; @@ -2143,6 +2144,39 @@ public abstract class CatalogTests<C extends Catalog & SupportsNamespaces> { .hasMessageStartingWith("Table does not exist: newdb.table"); } + @Test + public void testReplaceTableKeepsSnapshotLog() { + C catalog = catalog(); + + if (requiresNamespaceCreate()) { + catalog.createNamespace(TABLE.namespace()); + } + + catalog.createTable(TABLE, SCHEMA); + + Table table = catalog.loadTable(TABLE); + table.newAppend().appendFile(FILE_A).commit(); + + List<HistoryEntry> snapshotLogBeforeReplace = + ((BaseTable) table).operations().current().snapshotLog(); + assertThat(snapshotLogBeforeReplace).hasSize(1); + HistoryEntry snapshotBeforeReplace = snapshotLogBeforeReplace.get(0); + + Transaction replaceTableTransaction = catalog.newReplaceTableTransaction(TABLE, SCHEMA, false); + replaceTableTransaction.newAppend().appendFile(FILE_A).commit(); + replaceTableTransaction.commitTransaction(); + table.refresh(); + + List<HistoryEntry> snapshotLogAfterReplace = + ((BaseTable) table).operations().current().snapshotLog(); + HistoryEntry snapshotAfterReplace = snapshotLogAfterReplace.get(1); + + assertThat(snapshotAfterReplace).isNotEqualTo(snapshotBeforeReplace); + assertThat(snapshotLogAfterReplace) + .hasSize(2) + .containsExactly(snapshotBeforeReplace, snapshotAfterReplace); + } + @Test public void testConcurrentReplaceTransactions() { C catalog = catalog();
