updates

apache · Sep 17, 2024 · 8845d10 · 8845d10
1 parent 88bf1e8
commit 8845d10
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 44 deletions.
diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java
@@ -578,10 +578,6 @@ public TableMetadata removeSnapshotsIf(Predicate<Snapshot> removeIf) {
     return new Builder(this).removeSnapshots(toRemove).build();
   }
 
-  public TableMetadata updateMetadataLocation(String newMetadataLocation) {
-    return new Builder(this, newMetadataLocation).build();
-  }
-
   public TableMetadata replaceProperties(Map<String, String> rawProperties) {
     ValidationException.check(rawProperties != null, "Cannot set properties to null");
     Map<String, String> newProperties = unreservedProperties(rawProperties);
@@ -968,44 +964,18 @@ private Builder(TableMetadata base) {
       this.sortOrdersById = Maps.newHashMap(base.sortOrdersById);
     }
 
-    private Builder(TableMetadata base, String metadataLocation) {
-      // this constructor just updates the metadata location of base and carries over
-      // lastUpdatedMillis without updating previousFileLocation
-      this.metadataLocation = metadataLocation;
-      this.lastUpdatedMillis = base.lastUpdatedMillis;
-      this.previousFileLocation = null;
-      this.base = base;
-      this.formatVersion = base.formatVersion;
-      this.uuid = base.uuid;
-      this.location = base.location;
-      this.lastSequenceNumber = base.lastSequenceNumber;
-      this.lastColumnId = base.lastColumnId;
-      this.currentSchemaId = base.currentSchemaId;
-      this.schemas = Lists.newArrayList(base.schemas);
-      this.defaultSpecId = base.defaultSpecId;
-      this.specs = Lists.newArrayList(base.specs);
-      this.lastAssignedPartitionId = base.lastAssignedPartitionId;
-      this.defaultSortOrderId = base.defaultSortOrderId;
-      this.sortOrders = Lists.newArrayList(base.sortOrders);
-      this.properties = Maps.newHashMap(base.properties);
-      this.currentSnapshotId = base.currentSnapshotId;
-      this.snapshots = Lists.newArrayList(base.snapshots());
-      this.changes = Lists.newArrayList(base.changes);
-      this.startingChangeCount = changes.size();
-
-      this.snapshotLog = Lists.newArrayList(base.snapshotLog);
-      this.previousFiles = base.previousFiles;
-      this.refs = Maps.newHashMap(base.refs);
-      this.statisticsFiles = indexStatistics(base.statisticsFiles);
-      this.partitionStatisticsFiles = indexPartitionStatistics(base.partitionStatisticsFiles);
-      this.snapshotsById = Maps.newHashMap(base.snapshotsById);
-      this.schemasById = Maps.newHashMap(base.schemasById);
-      this.specsById = Maps.newHashMap(base.specsById);
-      this.sortOrdersById = Maps.newHashMap(base.sortOrdersById);
-    }
-
     public Builder withMetadataLocation(String newMetadataLocation) {
       this.metadataLocation = newMetadataLocation;
+
+      if (null != base) {
+        // carry over lastUpdatedMillis from base and set previousFileLocation to null to avoid
+        // writing a new metadata log entry
+        // this is safe since setting metadata location doesn't cause any changes and no other
+        // changes can be added when metadata location is configured
+        this.lastUpdatedMillis = base.lastUpdatedMillis();
+        this.previousFileLocation = null;
+      }
+
       return this;
     }
 

diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java
@@ -61,7 +61,7 @@ public String metadataLocation() {
   }
 
   public TableMetadata tableMetadata() {
-    return metadata.updateMetadataLocation(metadataLocation);
+    return TableMetadata.buildFrom(metadata).withMetadataLocation(metadataLocation).build();
   }
 
   public Map<String, String> config() {

diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
@@ -1735,7 +1735,7 @@ public void builderUpdatesMetadataLocationWithTimestampAndWritesMetadataLogEntry
   }
 
   @Test
-  public void onlyMetadataLocationIsUpdated() {
+  public void onlyMetadataLocationIsUpdatedWithoutTimestampAndMetadataLogEntry() {
     String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b";
     TableMetadata metadata =
         TableMetadata.buildFromEmpty()
@@ -1753,12 +1753,16 @@ public void onlyMetadataLocationIsUpdated() {
 
     // this will only update the metadata location without writing a new metadata log entry or
     // updating lastUpdatedMillis
-    TableMetadata newMetadata = metadata.updateMetadataLocation("new-metadata-location");
+    TableMetadata newMetadata =
+        TableMetadata.buildFrom(metadata).withMetadataLocation("new-metadata-location").build();
     assertThat(newMetadata.lastUpdatedMillis()).isEqualTo(metadata.lastUpdatedMillis());
     assertThat(newMetadata.metadataFileLocation()).isEqualTo("new-metadata-location");
     assertThat(newMetadata.previousFiles()).isEmpty();
 
-    TableMetadata updatedMetadata = newMetadata.updateMetadataLocation("updated-metadata-location");
+    TableMetadata updatedMetadata =
+        TableMetadata.buildFrom(newMetadata)
+            .withMetadataLocation("updated-metadata-location")
+            .build();
     assertThat(updatedMetadata.lastUpdatedMillis()).isEqualTo(newMetadata.lastUpdatedMillis());
     assertThat(updatedMetadata.metadataFileLocation()).isEqualTo("updated-metadata-location");
     assertThat(updatedMetadata.previousFiles()).isEmpty();