From 6bc150224b828eaf355da4e2fe14c9bdc6dcda68 Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Wed, 15 Apr 2020 00:19:30 -0700
Subject: [PATCH 01/11] Add partition spec update class for partition spec
 evolution

---
 .../main/java/org/apache/iceberg/Table.java   |  7 ++
 .../java/org/apache/iceberg/Transaction.java  |  7 ++
 .../apache/iceberg/UpdatePartitionSpec.java   | 41 +++++++++
 .../org/apache/iceberg/BaseMetadataTable.java |  5 +
 .../java/org/apache/iceberg/BaseTable.java    |  4 +
 .../org/apache/iceberg/BaseTransaction.java   | 13 +++
 .../iceberg/CommitCallbackTransaction.java    |  5 +
 .../apache/iceberg/PartitionSpecUpdate.java   | 91 +++++++++++++++++++
 .../iceberg/TestPartitionSpecUpdate.java      | 77 ++++++++++++++++
 9 files changed, 250 insertions(+)
 create mode 100644 api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
 create mode 100644 core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
 create mode 100644 core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java

diff --git a/api/src/main/java/org/apache/iceberg/Table.java b/api/src/main/java/org/apache/iceberg/Table.java
index 4536bb2b50e5..9d4c11c10c05 100644
--- a/api/src/main/java/org/apache/iceberg/Table.java
+++ b/api/src/main/java/org/apache/iceberg/Table.java
@@ -115,6 +115,13 @@ public interface Table {
    */
   UpdateSchema updateSchema();
 
+  /**
+   * Create a new {@link UpdatePartitionSpec} to alter the partition specs of this table and commit the change.
+   *
+   * @return a new {@link UpdatePartitionSpec}
+   */
+  UpdatePartitionSpec updatePartitionSpec();
+
   /**
    * Create a new {@link UpdateProperties} to update table properties and commit the changes.
    *
diff --git a/api/src/main/java/org/apache/iceberg/Transaction.java b/api/src/main/java/org/apache/iceberg/Transaction.java
index 3f12db0dd493..84bd2263bf0f 100644
--- a/api/src/main/java/org/apache/iceberg/Transaction.java
+++ b/api/src/main/java/org/apache/iceberg/Transaction.java
@@ -40,6 +40,13 @@ public interface Transaction {
    */
   UpdateSchema updateSchema();
 
+  /**
+   * Create a new {@link UpdatePartitionSpec} to alter the partition specs of this table.
+   *
+   * @return a new {@link UpdatePartitionSpec}
+   */
+  UpdatePartitionSpec updatePartitionSpec();
+
   /**
    * Create a new {@link UpdateProperties} to update table properties.
    *
diff --git a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
new file mode 100644
index 000000000000..d0eb491bb545
--- /dev/null
+++ b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg;
+
+import org.apache.iceberg.exceptions.CommitFailedException;
+
+/**
+ * API for partition spec evolution.
+ * <p>
+ * When committing, these changes will be applied to the current table metadata. Commit conflicts
+ * will not be resolved and will result in a {@link CommitFailedException}.
+ */
+public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
+
+  /** Update the current partition spec to a new partition spec.
+   * <p>
+   * Partition field IDs of the new partitionSpec may be updated during the commit.
+   *
+   * @param partitionSpec new partition spec to update
+   * @return this for method chaining
+   */
+  UpdatePartitionSpec update(PartitionSpec partitionSpec);
+
+}
diff --git a/core/src/main/java/org/apache/iceberg/BaseMetadataTable.java b/core/src/main/java/org/apache/iceberg/BaseMetadataTable.java
index 009cfbda8edc..e7c084be3ccc 100644
--- a/core/src/main/java/org/apache/iceberg/BaseMetadataTable.java
+++ b/core/src/main/java/org/apache/iceberg/BaseMetadataTable.java
@@ -97,6 +97,11 @@ public UpdateSchema updateSchema() {
     throw new UnsupportedOperationException("Cannot update the schema of a metadata table");
   }
 
+  @Override
+  public UpdatePartitionSpec updatePartitionSpec() {
+    throw new UnsupportedOperationException("Cannot update the partition spec of a metadata table");
+  }
+
   @Override
   public UpdateProperties updateProperties() {
     throw new UnsupportedOperationException("Cannot update the properties of a metadata table");
diff --git a/core/src/main/java/org/apache/iceberg/BaseTable.java b/core/src/main/java/org/apache/iceberg/BaseTable.java
index 15b8742691d9..e56d597974f6 100644
--- a/core/src/main/java/org/apache/iceberg/BaseTable.java
+++ b/core/src/main/java/org/apache/iceberg/BaseTable.java
@@ -104,6 +104,10 @@ public UpdateSchema updateSchema() {
     return new SchemaUpdate(ops);
   }
 
+  public UpdatePartitionSpec updatePartitionSpec() {
+    return new PartitionSpecUpdate(ops);
+  }
+
   @Override
   public UpdateProperties updateProperties() {
     return new PropertiesUpdate(ops);
diff --git a/core/src/main/java/org/apache/iceberg/BaseTransaction.java b/core/src/main/java/org/apache/iceberg/BaseTransaction.java
index f1ea1913d1df..87798c2484dd 100644
--- a/core/src/main/java/org/apache/iceberg/BaseTransaction.java
+++ b/core/src/main/java/org/apache/iceberg/BaseTransaction.java
@@ -102,6 +102,14 @@ public UpdateSchema updateSchema() {
     return schemaChange;
   }
 
+  @Override
+  public UpdatePartitionSpec updatePartitionSpec() {
+    checkLastOperationCommitted("UpdatePartitionSpec");
+    UpdatePartitionSpec partitionSpecChange = new PartitionSpecUpdate(transactionOps);
+    updates.add(partitionSpecChange);
+    return partitionSpecChange;
+  }
+
   @Override
   public UpdateProperties updateProperties() {
     checkLastOperationCommitted("UpdateProperties");
@@ -541,6 +549,11 @@ public UpdateSchema updateSchema() {
       return BaseTransaction.this.updateSchema();
     }
 
+    @Override
+    public UpdatePartitionSpec updatePartitionSpec() {
+      return BaseTransaction.this.updatePartitionSpec();
+    }
+
     @Override
     public UpdateProperties updateProperties() {
       return BaseTransaction.this.updateProperties();
diff --git a/core/src/main/java/org/apache/iceberg/CommitCallbackTransaction.java b/core/src/main/java/org/apache/iceberg/CommitCallbackTransaction.java
index 6fa713509b0a..2740bef0fd96 100644
--- a/core/src/main/java/org/apache/iceberg/CommitCallbackTransaction.java
+++ b/core/src/main/java/org/apache/iceberg/CommitCallbackTransaction.java
@@ -42,6 +42,11 @@ public UpdateSchema updateSchema() {
     return wrapped.updateSchema();
   }
 
+  @Override
+  public UpdatePartitionSpec updatePartitionSpec() {
+    return wrapped.updatePartitionSpec();
+  }
+
   @Override
   public UpdateProperties updateProperties() {
     return wrapped.updateProperties();
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
new file mode 100644
index 000000000000..9fc7c0c89818
--- /dev/null
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Maps;
+import java.util.Map;
+
+/**
+ * PartitionSpec evolution API implementation.
+ */
+class PartitionSpecUpdate implements UpdatePartitionSpec {
+
+  private final TableMetadata base;
+  private final TableOperations ops;
+  private PartitionSpec newSpec = null;
+
+  PartitionSpecUpdate(TableOperations ops) {
+    this.ops = ops;
+    this.base = ops.current();
+  }
+
+  @Override
+  public PartitionSpec apply() {
+    return Preconditions.checkNotNull(newSpec, "new spec is not set");
+  }
+
+  @Override
+  public UpdatePartitionSpec update(PartitionSpec partitionSpec) {
+    PartitionSpec.checkCompatibility(partitionSpec, base.schema());
+    newSpec = partitionSpec;
+    return this;
+  }
+
+  @Override
+  public void commit() {
+    TableMetadata update = base.updatePartitionSpec(freshSpecFieldIds(apply()));
+    ops.commit(base, update);
+  }
+
+  private PartitionSpec freshSpecFieldIds(PartitionSpec partitionSpec) {
+    if (base.formatVersion() == 1) {
+      return partitionSpec;
+    }
+
+    int lastAssignedFieldId = 0;
+    Map<String, Integer> partitionFieldIdByName = Maps.newHashMap();
+    for (PartitionSpec spec : base.specs()) {
+      for (PartitionField field : spec.fields()) {
+        partitionFieldIdByName.put(getKey(field), field.fieldId());
+      }
+      lastAssignedFieldId = Math.max(lastAssignedFieldId, spec.lastAssignedFieldId());
+    }
+
+    PartitionSpec.Builder specBuilder = PartitionSpec.builderFor(base.schema())
+        .withSpecId(partitionSpec.specId());
+
+    for (PartitionField field : partitionSpec.fields()) {
+      int assignedFieldId = partitionFieldIdByName.containsKey(getKey(field)) ?
+          partitionFieldIdByName.get(getKey(field)) : ++lastAssignedFieldId;
+
+      specBuilder.add(
+          field.sourceId(),
+          assignedFieldId,
+          field.name(),
+          field.transform().toString());
+    }
+    return specBuilder.build();
+  }
+
+  private static String getKey(PartitionField field) {
+    return field.transform() + "(" + field.sourceId() + ")";
+  }
+}
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
new file mode 100644
index 000000000000..c4521d780433
--- /dev/null
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg;
+
+import org.apache.iceberg.exceptions.ValidationException;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestPartitionSpecUpdate extends TableTestBase {
+
+  @Test
+  public void testCommitUpdatedSpec() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+
+    PartitionSpec spec = PartitionSpec.builderFor(table.schema())
+        .bucket("id", 8)
+        .bucket("data", 16)
+        .build();
+    table.updatePartitionSpec().update(spec).commit();
+
+    Assert.assertEquals("[\n" +
+        "  1001: id_bucket: bucket[8](1)\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+
+    spec = PartitionSpec.builderFor(table.schema())
+        .truncate("data", 8)
+        .build();
+    table.updatePartitionSpec().update(spec).commit();
+
+    Assert.assertEquals("[\n" +
+        "  1002: data_trunc: truncate[8](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+  }
+
+  @Test
+  public void testCommitException() {
+    AssertHelpers.assertThrows("Should throw NullPointerException if no spec to commit",
+        NullPointerException.class, "new spec is not set",
+        () -> table.updatePartitionSpec().commit());
+  }
+
+  @Test
+  public void testUpdateCompatibility() {
+    PartitionSpec spec = PartitionSpec.builderFor(SCHEMA)
+        .bucket("id", 8)
+        .bucket("data", 16)
+        .build();
+
+    AssertHelpers.assertThrows(
+        "Should throw ValidationException if the new spec is not compatible with the table schema",
+        ValidationException.class, "Cannot find source column for partition field",
+        () -> table.updatePartitionSpec().update(spec));
+  }
+}

From 940d9acd6093832b57089bb294318e4836ac0045 Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Sun, 29 Mar 2020 16:24:17 -0700
Subject: [PATCH 02/11] Support builder pattern in UpdatePartitionSPec class

---
 .../apache/iceberg/UpdatePartitionSpec.java   | 182 ++++++++++++++++++
 .../apache/iceberg/PartitionSpecUpdate.java   | 114 ++++++++++-
 .../iceberg/TestPartitionSpecUpdate.java      |   4 +-
 3 files changed, 297 insertions(+), 3 deletions(-)

diff --git a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
index d0eb491bb545..9fad4cb21ff0 100644
--- a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
@@ -38,4 +38,186 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    */
   UpdatePartitionSpec update(PartitionSpec partitionSpec);
 
+  /**
+   * Create a new partition spec builder for a given schema
+   * <p>
+   * Partition field IDs is automatically assigned and will be updated during the commit.
+   *
+   * @param schema the schema for the new partition spec
+   * @return this for method chaining
+   */
+  UpdatePartitionSpec newSpec(Schema schema);
+
+  /**
+   * Add a new partition field with identity transform to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param targetName the name of this partition field
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec identity(String sourceName, String targetName);
+
+  /**
+   * Add a new partition field with identity transform to the partition spec.
+   * <p>
+   * The partition field name is automatically assigned set.
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec identity(String sourceName);
+
+  /**
+   * Add a new partition field with year transform to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param targetName the name of this partition field
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec year(String sourceName, String targetName);
+
+  /**
+   * Add a new partition field with year transform to the partition spec.
+   * <p>
+   * The partition field name is automatically assigned set.
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec year(String sourceName);
+
+  /**
+   * Add a new partition field with month transform to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param targetName the name of this partition field
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec month(String sourceName, String targetName);
+
+  /**
+   * Add a new partition field with month transform to the partition spec.
+   * <p>
+   * The partition field name is automatically assigned set.
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec month(String sourceName);
+
+  /**
+   * Add a new partition field with day transform to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param targetName the name of this partition field
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec day(String sourceName, String targetName);
+
+  /**
+   * Add a new partition field with day transform to the partition spec.
+   * <p>
+   * The partition field name is automatically assigned set.
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec day(String sourceName);
+
+  /**
+   * Add a new partition field with hour transform to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param targetName the name of this partition field
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec hour(String sourceName, String targetName);
+
+  /**
+   * Add a new partition field with hour transform to the partition spec.
+   * <p>
+   * The partition field name is automatically assigned set.
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec hour(String sourceName);
+
+  /**
+   * Add a new partition field with bucket transform to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param numBuckets the number of buckets
+   * @param targetName the name of this partition field
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec bucket(String sourceName, int numBuckets, String targetName);
+
+  /**
+   * Add a new partition field with bucket transform to the partition spec.
+   * <p>
+   * The partition field name is automatically assigned set.
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param numBuckets the number of buckets
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec bucket(String sourceName, int numBuckets);
+
+  /**
+   * Add a new partition field with truncate transform to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param width the width of truncation
+   * @param targetName the name of this partition field
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec truncate(String sourceName, int width, String targetName);
+
+  /**
+   * Add a new partition field with truncate transform to the partition spec.
+   * <p>
+   * The partition field name is automatically assigned set.
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
+   * @param width the width of truncation
+   * @return this for method chaining
+   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
+   */
+  UpdatePartitionSpec truncate(String sourceName, int width);
+
 }
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
index 9fc7c0c89818..82fa8f141b7f 100644
--- a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
@@ -31,6 +31,7 @@ class PartitionSpecUpdate implements UpdatePartitionSpec {
   private final TableMetadata base;
   private final TableOperations ops;
   private PartitionSpec newSpec = null;
+  private PartitionSpec.Builder newSpecBuilder = null;
 
   PartitionSpecUpdate(TableOperations ops) {
     this.ops = ops;
@@ -39,7 +40,14 @@ class PartitionSpecUpdate implements UpdatePartitionSpec {
 
   @Override
   public PartitionSpec apply() {
-    return Preconditions.checkNotNull(newSpec, "new spec is not set");
+    Preconditions.checkArgument(newSpec != null || newSpecBuilder != null,
+        "new partition spec is not set");
+    Preconditions.checkArgument(newSpec == null || newSpecBuilder == null,
+        "use either update() or newSpec() to add a new partition spec");
+    if (newSpecBuilder != null) {
+      return newSpecBuilder.build();
+    }
+    return newSpec;
   }
 
   @Override
@@ -49,6 +57,110 @@ public UpdatePartitionSpec update(PartitionSpec partitionSpec) {
     return this;
   }
 
+  @Override
+  public UpdatePartitionSpec newSpec(Schema schema) {
+    newSpecBuilder = PartitionSpec.builderFor(Preconditions.checkNotNull(schema, "The schema cannot be null."));
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec identity(String sourceName, String targetName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .identity(sourceName, targetName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec identity(String sourceName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .identity(sourceName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec year(String sourceName, String targetName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .year(sourceName, targetName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec year(String sourceName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .year(sourceName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec month(String sourceName, String targetName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .month(sourceName, targetName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec month(String sourceName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .month(sourceName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec day(String sourceName, String targetName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .day(sourceName, targetName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec day(String sourceName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .day(sourceName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec hour(String sourceName, String targetName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .hour(sourceName, targetName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec hour(String sourceName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .hour(sourceName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec bucket(String sourceName, int numBuckets, String targetName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .bucket(sourceName, numBuckets, targetName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec bucket(String sourceName, int numBuckets) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .bucket(sourceName, numBuckets);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec truncate(String sourceName, int width, String targetName) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .truncate(sourceName, width, targetName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec truncate(String sourceName, int width) {
+    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
+        .truncate(sourceName, width);
+    return this;
+  }
+
   @Override
   public void commit() {
     TableMetadata update = base.updatePartitionSpec(freshSpecFieldIds(apply()));
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index c4521d780433..0fa18ed2686a 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -57,8 +57,8 @@ public void testCommitUpdatedSpec() {
 
   @Test
   public void testCommitException() {
-    AssertHelpers.assertThrows("Should throw NullPointerException if no spec to commit",
-        NullPointerException.class, "new spec is not set",
+    AssertHelpers.assertThrows("Should throw IllegalArgumentException if no spec to commit",
+        IllegalArgumentException.class, "new partition spec is not set",
         () -> table.updatePartitionSpec().commit());
   }
 

From acfe7cfdc02588d0dde196c92a8c472837bea17a Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Thu, 16 Apr 2020 23:23:29 -0700
Subject: [PATCH 03/11] rebase and add few unit tests

---
 .../apache/iceberg/UpdatePartitionSpec.java   |  3 +-
 .../org/apache/iceberg/TestMergeAppend.java   |  1 -
 .../iceberg/TestPartitionSpecUpdate.java      | 36 ++++++++++++++++++-
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
index 9fad4cb21ff0..a975753422aa 100644
--- a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
@@ -29,7 +29,8 @@
  */
 public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
-  /** Update the current partition spec to a new partition spec.
+  /**
+   * Update the current partition spec to a new partition spec.
    * <p>
    * Partition field IDs of the new partitionSpec may be updated during the commit.
    *
diff --git a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
index bc6c6a3d5bc2..c00db2120d2b 100644
--- a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
+++ b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
@@ -665,7 +665,6 @@ public void testInvalidAppendManifest() throws IOException {
             .commit());
   }
 
-
   @Test
   public void testUpdatePartitionSpecFieldIdsForV1Table() {
     TableMetadata base = readMetadata();
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index 0fa18ed2686a..e7b5ba1a957e 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -26,7 +26,41 @@
 public class TestPartitionSpecUpdate extends TableTestBase {
 
   @Test
-  public void testCommitUpdatedSpec() {
+  public void testCommitUpdatedSpecForV1Table() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+
+    PartitionSpec spec = PartitionSpec.builderFor(table.schema())
+        .bucket("id", 8)
+        .bucket("data", 16)
+        .build();
+    table.updatePartitionSpec().update(spec).commit();
+
+    Assert.assertEquals("[\n" +
+        "  1000: id_bucket: bucket[8](1)\n" +
+        "  1001: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+
+    spec = PartitionSpec.builderFor(table.schema())
+        .truncate("data", 8)
+        .build();
+    table.updatePartitionSpec().update(spec).commit();
+
+    Assert.assertEquals("[\n" +
+        "  1000: data_trunc: truncate[8](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+  }
+
+  @Test
+  public void testCommitUpdatedSpecForV2Table() {
+    TableOperations ops = table.ops();
+    TableMetadata base = ops.current();
+    ops.commit(base, base.upgradeToFormatVersion(2));
+
     Assert.assertEquals("[\n" +
         "  1000: data_bucket: bucket[16](2)\n" +
         "]", table.spec().toString());

From 9c076b70fa4ee915ab8530f4ef52344f8833453b Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Fri, 24 Apr 2020 23:24:10 -0700
Subject: [PATCH 04/11] add additional tests and rebase

---
 .../org/apache/iceberg/PartitionSpec.java     |  7 ++
 .../apache/iceberg/UpdatePartitionSpec.java   | 14 +--
 .../iceberg/TestPartitionSpecValidation.java  | 14 +++
 .../apache/iceberg/PartitionSpecUpdate.java   | 22 +----
 .../apache/iceberg/TestManifestReader.java    | 36 +++++++-
 .../org/apache/iceberg/TestMergeAppend.java   | 45 +++++-----
 .../apache/iceberg/TestPartitionSpecInfo.java | 27 +++---
 .../iceberg/TestPartitionSpecParser.java      | 35 +++++---
 .../iceberg/TestPartitionSpecUpdate.java      | 89 +++++++------------
 .../org/apache/iceberg/TestTableMetadata.java |  2 +-
 10 files changed, 154 insertions(+), 137 deletions(-)

diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
index 6d402d3d6474..9f4650aef568 100644
--- a/api/src/main/java/org/apache/iceberg/PartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
@@ -353,6 +353,12 @@ private void checkForRedundantPartitions(PartitionField field) {
       timeFields.put(field.sourceId(), field);
     }
 
+    private void checkDuplicateFieldId(int fieldId) {
+      Preconditions.checkArgument(fields.stream().allMatch(f -> f.fieldId() != fieldId),
+          "Field Id %s has already been used in the existing partition fields: %s.",
+          fieldId, fields);
+    }
+
     public Builder withSpecId(int newSpecId) {
       this.specId = newSpecId;
       return this;
@@ -476,6 +482,7 @@ Builder add(int sourceId, int fieldId, String name, String transform) {
       Types.NestedField column = schema.findField(sourceId);
       checkAndAddPartitionName(name, column.fieldId());
       Preconditions.checkNotNull(column, "Cannot find source column: %s", sourceId);
+      checkDuplicateFieldId(fieldId);
       fields.add(new PartitionField(sourceId, fieldId, name, Transforms.fromString(column.type(), transform)));
       lastAssignedFieldId.getAndAccumulate(fieldId, Math::max);
       return this;
diff --git a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
index a975753422aa..4d0a5e742dca 100644
--- a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
@@ -29,25 +29,15 @@
  */
 public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
-  /**
-   * Update the current partition spec to a new partition spec.
-   * <p>
-   * Partition field IDs of the new partitionSpec may be updated during the commit.
-   *
-   * @param partitionSpec new partition spec to update
-   * @return this for method chaining
-   */
-  UpdatePartitionSpec update(PartitionSpec partitionSpec);
-
   /**
    * Create a new partition spec builder for a given schema
    * <p>
    * Partition field IDs is automatically assigned and will be updated during the commit.
+   * Table schema should be obtained from the current table metadata
    *
-   * @param schema the schema for the new partition spec
    * @return this for method chaining
    */
-  UpdatePartitionSpec newSpec(Schema schema);
+  UpdatePartitionSpec newSpec();
 
   /**
    * Add a new partition field with identity transform to the partition spec.
diff --git a/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java b/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
index b22bc7b6c401..35e5d420080c 100644
--- a/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
+++ b/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
@@ -241,11 +241,25 @@ public void testAddPartitionFieldsWithAndWithoutFieldIds() {
         .add(1, "id_partition2", "bucket[5]")
         .add(1, 1005, "id_partition1", "bucket[4]")
         .truncate("s", 1, "custom_truncate")
+        .add(1, 1002, "id_partition3", "bucket[3]")
         .build();
 
     Assert.assertEquals(1000, spec.fields().get(0).fieldId());
     Assert.assertEquals(1005, spec.fields().get(1).fieldId());
     Assert.assertEquals(1006, spec.fields().get(2).fieldId());
+    Assert.assertEquals(1002, spec.fields().get(3).fieldId());
     Assert.assertEquals(1006, spec.lastAssignedFieldId());
   }
+
+  @Test
+  public void testAddPartitionFieldsWithInvalidFieldId() {
+    AssertHelpers.assertThrows("Should detect invalid duplicate field id",
+        IllegalArgumentException.class,
+        "Field Id 1005 has already been used in the existing partition fields",
+        () -> PartitionSpec.builderFor(SCHEMA)
+            .add(1, "id_partition2", "bucket[5]")
+            .add(1, 1005, "id_partition1", "bucket[4]")
+            .add(1, 1005, "id_partition3", "bucket[3]")
+            .build());
+  }
 }
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
index 82fa8f141b7f..aa925c4b5d66 100644
--- a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
@@ -30,7 +30,6 @@ class PartitionSpecUpdate implements UpdatePartitionSpec {
 
   private final TableMetadata base;
   private final TableOperations ops;
-  private PartitionSpec newSpec = null;
   private PartitionSpec.Builder newSpecBuilder = null;
 
   PartitionSpecUpdate(TableOperations ops) {
@@ -40,26 +39,13 @@ class PartitionSpecUpdate implements UpdatePartitionSpec {
 
   @Override
   public PartitionSpec apply() {
-    Preconditions.checkArgument(newSpec != null || newSpecBuilder != null,
-        "new partition spec is not set");
-    Preconditions.checkArgument(newSpec == null || newSpecBuilder == null,
-        "use either update() or newSpec() to add a new partition spec");
-    if (newSpecBuilder != null) {
-      return newSpecBuilder.build();
-    }
-    return newSpec;
-  }
-
-  @Override
-  public UpdatePartitionSpec update(PartitionSpec partitionSpec) {
-    PartitionSpec.checkCompatibility(partitionSpec, base.schema());
-    newSpec = partitionSpec;
-    return this;
+    Preconditions.checkNotNull(newSpecBuilder, "new partition spec is not set");
+    return newSpecBuilder.build();
   }
 
   @Override
-  public UpdatePartitionSpec newSpec(Schema schema) {
-    newSpecBuilder = PartitionSpec.builderFor(Preconditions.checkNotNull(schema, "The schema cannot be null."));
+  public UpdatePartitionSpec newSpec() {
+    newSpecBuilder = PartitionSpec.builderFor(base.schema());
     return this;
   }
 
diff --git a/core/src/test/java/org/apache/iceberg/TestManifestReader.java b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
index 4e91131b6db3..f59987abe61d 100644
--- a/core/src/test/java/org/apache/iceberg/TestManifestReader.java
+++ b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
@@ -34,13 +34,16 @@ public class TestManifestReader extends TableTestBase {
   @Parameterized.Parameters
   public static Object[][] parameters() {
     return new Object[][] {
-        new Object[] { 1 },
-        new Object[] { 2 },
+        new Object[] { 1, new int[]{ 1000, 1001 } },
+        new Object[] { 2, new int[]{ 1001, 1000 }},
     };
   }
 
-  public TestManifestReader(int formatVersion) {
+  private int[] expectedFieldIds;
+
+  public TestManifestReader(int formatVersion, int[] expectedFieldIds) {
     super(formatVersion);
+    this.expectedFieldIds = expectedFieldIds;
   }
 
   @Test
@@ -79,11 +82,12 @@ public void testManifestReaderWithPartitionMetadata() throws IOException {
   }
 
   @Test
-  public void testManifestReaderWithUpdatedPartitionMetadataForV1Table() throws IOException {
+  public void testManifestReaderWithUpdatedPartitionMetadata() throws IOException {
     PartitionSpec spec = PartitionSpec.builderFor(table.schema())
         .bucket("id", 8)
         .bucket("data", 16)
         .build();
+    // commit the new partition spec to the table manually without spec evolution
     table.ops().commit(table.ops().current(), table.ops().current().updatePartitionSpec(spec));
 
     ManifestFile manifest = writeManifest(1000L, manifestEntry(Status.EXISTING, 123L, FILE_A));
@@ -103,4 +107,28 @@ public void testManifestReaderWithUpdatedPartitionMetadataForV1Table() throws IO
     }
   }
 
+  @Test
+  public void testManifestReaderWithPartitionMetadataEvolution() throws IOException {
+    table.updatePartitionSpec().newSpec()
+        .bucket("id", 8)
+        .bucket("data", 16)
+        .commit();
+
+    ManifestFile manifest = writeManifest(1000L, manifestEntry(Status.EXISTING, 123L, FILE_A));
+    try (ManifestReader reader = ManifestFiles.read(manifest, FILE_IO)) {
+      ManifestEntry entry = Iterables.getOnlyElement(reader.entries());
+      Assert.assertEquals(123L, (long) entry.snapshotId());
+
+      List<Types.NestedField> fields = ((PartitionData) entry.file().partition()).getPartitionType().fields();
+      Assert.assertEquals(2, fields.size());
+      Assert.assertEquals(expectedFieldIds[0], fields.get(0).fieldId());
+      Assert.assertEquals("id_bucket", fields.get(0).name());
+      Assert.assertEquals(Types.IntegerType.get(), fields.get(0).type());
+
+      Assert.assertEquals(expectedFieldIds[1], fields.get(1).fieldId());
+      Assert.assertEquals("data_bucket", fields.get(1).name());
+      Assert.assertEquals(Types.IntegerType.get(), fields.get(1).type());
+    }
+  }
+
 }
diff --git a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
index c00db2120d2b..ac3e39f2d0c9 100644
--- a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
+++ b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
@@ -40,13 +40,16 @@ public class TestMergeAppend extends TableTestBase {
   @Parameterized.Parameters
   public static Object[][] parameters() {
     return new Object[][] {
-        new Object[] { 1 },
-        new Object[] { 2 },
+        new Object[] { 1, new int[]{ 1000, 1001, 1002, 1003, 1000, 1001 } },
+        new Object[] { 2, new int[]{ 1001, 1002, 1003, 1000, 1001, 1002 } },
     };
   }
 
-  public TestMergeAppend(int formatVersion) {
+  private int[] expectedFieldIds;
+
+  public TestMergeAppend(int formatVersion, int[] expectedFieldIds) {
     super(formatVersion);
+    this.expectedFieldIds = expectedFieldIds;
   }
 
   @Test
@@ -389,7 +392,7 @@ public void testChangedPartitionSpec() {
         .bucket("id", 4)
         .build();
 
-    // commit the new partition spec to the table manually
+    // commit the new partition spec to the table manually without spec evolution
     table.ops().commit(base, base.updatePartitionSpec(newSpec));
 
     DataFile newFileC = DataFiles.builder(newSpec)
@@ -435,7 +438,7 @@ public void testChangedPartitionSpecMergeExisting() {
         .bucket("id", 4)
         .build();
 
-    // commit the new partition spec to the table manually
+    // commit the new partition spec to the table manually without spec evolution
     table.ops().commit(base, base.updatePartitionSpec(newSpec));
 
     DataFile newFileC = DataFiles.builder(newSpec)
@@ -666,19 +669,15 @@ public void testInvalidAppendManifest() throws IOException {
   }
 
   @Test
-  public void testUpdatePartitionSpecFieldIdsForV1Table() {
+  public void testUpdatePartitionSpecFieldIdsWithSpecEvolution() {
     TableMetadata base = readMetadata();
 
-    // build the new spec using the table's schema, which uses fresh IDs
-    PartitionSpec newSpec = PartitionSpec.builderFor(base.schema())
+    table.updatePartitionSpec().newSpec()
         .bucket("id", 16)
         .identity("data")
         .bucket("data", 4)
         .bucket("data", 16, "data_partition") // reuse field id although different target name
-        .build();
-
-    // commit the new partition spec to the table manually
-    table.ops().commit(base, base.updatePartitionSpec(newSpec));
+        .commit();
 
     List<PartitionSpec> partitionSpecs = table.ops().current().specs();
     PartitionSpec partitionSpec = partitionSpecs.get(0);
@@ -697,17 +696,17 @@ public void testUpdatePartitionSpecFieldIdsForV1Table() {
     fields = structType.fields();
     Assert.assertEquals(4, fields.size());
     Assert.assertEquals("id_bucket", fields.get(0).name());
-    Assert.assertEquals(1000, fields.get(0).fieldId());
+    Assert.assertEquals(expectedFieldIds[0], fields.get(0).fieldId());
     Assert.assertEquals("data", fields.get(1).name());
-    Assert.assertEquals(1001, fields.get(1).fieldId());
+    Assert.assertEquals(expectedFieldIds[1], fields.get(1).fieldId());
     Assert.assertEquals("data_bucket", fields.get(2).name());
-    Assert.assertEquals(1002, fields.get(2).fieldId());
+    Assert.assertEquals(expectedFieldIds[2], fields.get(2).fieldId());
     Assert.assertEquals("data_partition", fields.get(3).name());
-    Assert.assertEquals(1003, fields.get(3).fieldId());
+    Assert.assertEquals(expectedFieldIds[3], fields.get(3).fieldId());
   }
 
   @Test
-  public void testManifestEntryFieldIdsForChangedPartitionSpecForV1Table() {
+  public void testManifestEntryFieldIdsForChangedPartitionSpecWithSpecEvolution() {
     table.newAppend()
         .appendFile(FILE_A)
         .commit();
@@ -717,14 +716,10 @@ public void testManifestEntryFieldIdsForChangedPartitionSpecForV1Table() {
         1, base.currentSnapshot().allManifests().size());
     ManifestFile initialManifest = base.currentSnapshot().allManifests().get(0);
 
-    // build the new spec using the table's schema, which uses fresh IDs
-    PartitionSpec newSpec = PartitionSpec.builderFor(base.schema())
+    table.updatePartitionSpec().newSpec()
         .bucket("id", 8)
         .bucket("data", 8)
-        .build();
-
-    // commit the new partition spec to the table manually
-    table.ops().commit(base, base.updatePartitionSpec(newSpec));
+        .commit();
 
     DataFile newFile = DataFiles.builder(table.spec())
         .copy(FILE_B)
@@ -747,10 +742,10 @@ public void testManifestEntryFieldIdsForChangedPartitionSpecForV1Table() {
     ManifestEntry<DataFile> entry = ManifestFiles.read(pending.allManifests().get(0), FILE_IO)
         .entries().iterator().next();
     Types.NestedField field = ((PartitionData) entry.file().partition()).getPartitionType().fields().get(0);
-    Assert.assertEquals(1000, field.fieldId());
+    Assert.assertEquals(expectedFieldIds[4], field.fieldId());
     Assert.assertEquals("id_bucket", field.name());
     field = ((PartitionData) entry.file().partition()).getPartitionType().fields().get(1);
-    Assert.assertEquals(1001, field.fieldId());
+    Assert.assertEquals(expectedFieldIds[5], field.fieldId());
     Assert.assertEquals("data_bucket", field.name());
 
     entry = ManifestFiles.read(pending.allManifests().get(1), FILE_IO).entries().iterator().next();
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
index b0395f004323..0674048b9a32 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
@@ -44,18 +44,21 @@ public class TestPartitionSpecInfo {
       required(2, "data", Types.StringType.get()));
   private File tableDir = null;
 
+  private int newFieldId;
+
   @Parameterized.Parameters
   public static Object[][] parameters() {
     return new Object[][] {
-        new Object[] { 1 },
-        new Object[] { 2 },
+        new Object[] { 1, 1000 },
+        new Object[] { 2, 1001 },
     };
   }
 
   private final int formatVersion;
 
-  public TestPartitionSpecInfo(int formatVersion) {
+  public TestPartitionSpecInfo(int formatVersion, int newFieldId) {
     this.formatVersion = formatVersion;
+    this.newFieldId = newFieldId;
   }
 
   @Before
@@ -91,7 +94,7 @@ public void testSpecInfoPartitionedTable() {
   }
 
   @Test
-  public void testSpecInfoPartitionSpecEvolutionForV1Table() {
+  public void testSpecInfoPartitionSpecEvolution() {
     PartitionSpec spec = PartitionSpec.builderFor(schema)
         .bucket("data", 4)
         .build();
@@ -99,17 +102,19 @@ public void testSpecInfoPartitionSpecEvolutionForV1Table() {
 
     Assert.assertEquals(spec, table.spec());
 
-    TableMetadata base = TestTables.readMetadata("test");
-    PartitionSpec newSpec = PartitionSpec.builderFor(table.schema())
-        .bucket("data", 10)
+    PartitionSpec expectedNewSpec = PartitionSpec.builderFor(table.schema())
+        .add(2, newFieldId, "data_bucket", "bucket[10]")
         .withSpecId(1)
         .build();
-    table.ops().commit(base, base.updatePartitionSpec(newSpec));
 
-    Assert.assertEquals(newSpec, table.spec());
-    Assert.assertEquals(newSpec, table.specs().get(newSpec.specId()));
+    table.updatePartitionSpec().newSpec()
+        .bucket("data", 10)
+        .commit();
+
+    Assert.assertEquals(expectedNewSpec, table.spec());
+    Assert.assertEquals(expectedNewSpec, table.specs().get(expectedNewSpec.specId()));
     Assert.assertEquals(spec, table.specs().get(spec.specId()));
-    Assert.assertEquals(ImmutableMap.of(spec.specId(), spec, newSpec.specId(), newSpec), table.specs());
+    Assert.assertEquals(ImmutableMap.of(spec.specId(), spec, expectedNewSpec.specId(), expectedNewSpec), table.specs());
     Assert.assertNull(table.specs().get(Integer.MAX_VALUE));
   }
 }
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
index 847ff4283ab1..c2bcb1487d49 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
@@ -21,14 +21,29 @@
 
 import org.junit.Assert;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
+@RunWith(Parameterized.class)
 public class TestPartitionSpecParser extends TableTestBase {
-  public TestPartitionSpecParser() {
-    super(1);
+
+  private int[] expectedFieldIds;
+
+  @Parameterized.Parameters
+  public static Object[][] parameters() {
+    return new Object[][] {
+        new Object[] { 1, new int[]{ 1000, 1001 } },
+        new Object[] { 2, new int[]{ 1001, 1000 } },
+    };
+  }
+
+  public TestPartitionSpecParser(int formatVersion, int[] expectedFieldIds) {
+    super(formatVersion);
+    this.expectedFieldIds = expectedFieldIds;
   }
 
   @Test
-  public void testToJsonForV1Table() {
+  public void testToJson() {
     String expected = "{\n" +
         "  \"spec-id\" : 0,\n" +
         "  \"fields\" : [ {\n" +
@@ -40,12 +55,10 @@ public void testToJsonForV1Table() {
         "}";
     Assert.assertEquals(expected, PartitionSpecParser.toJson(table.spec(), true));
 
-    PartitionSpec spec = PartitionSpec.builderFor(table.schema())
+    table.updatePartitionSpec().newSpec()
         .bucket("id", 8)
         .bucket("data", 16)
-        .build();
-
-    table.ops().commit(table.ops().current(), table.ops().current().updatePartitionSpec(spec));
+        .commit();
 
     expected = "{\n" +
         "  \"spec-id\" : 1,\n" +
@@ -53,13 +66,13 @@ public void testToJsonForV1Table() {
         "    \"name\" : \"id_bucket\",\n" +
         "    \"transform\" : \"bucket[8]\",\n" +
         "    \"source-id\" : 1,\n" +
-        "    \"field-id\" : 1000\n" +
-        "  }, {\n" +
+        "    \"field-id\" : " + expectedFieldIds[0] +
+        "\n  }, {\n" +
         "    \"name\" : \"data_bucket\",\n" +
         "    \"transform\" : \"bucket[16]\",\n" +
         "    \"source-id\" : 2,\n" +
-        "    \"field-id\" : 1001\n" +
-        "  } ]\n" +
+        "    \"field-id\" : " + expectedFieldIds[1] +
+        "\n  } ]\n" +
         "}";
     Assert.assertEquals(expected, PartitionSpecParser.toJson(table.spec(), true));
   }
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index e7b5ba1a957e..402bf1c6a9e7 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -19,93 +19,72 @@
 
 package org.apache.iceberg;
 
-import org.apache.iceberg.exceptions.ValidationException;
 import org.junit.Assert;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
+@RunWith(Parameterized.class)
 public class TestPartitionSpecUpdate extends TableTestBase {
 
-  @Test
-  public void testCommitUpdatedSpecForV1Table() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
-
-    PartitionSpec spec = PartitionSpec.builderFor(table.schema())
-        .bucket("id", 8)
-        .bucket("data", 16)
-        .build();
-    table.updatePartitionSpec().update(spec).commit();
-
-    Assert.assertEquals("[\n" +
-        "  1000: id_bucket: bucket[8](1)\n" +
-        "  1001: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+  private int[] expectedFieldIds;
 
-    spec = PartitionSpec.builderFor(table.schema())
-        .truncate("data", 8)
-        .build();
-    table.updatePartitionSpec().update(spec).commit();
+  @Parameterized.Parameters
+  public static Object[][] parameters() {
+    return new Object[][] {
+        new Object[] { 1, new int[]{ 1000, 1001, 1001, 1000, 1000 } },
+        new Object[] { 2, new int[]{ 1001, 1000, 1001, 1002, 1002 } },
+    };
+  }
 
-    Assert.assertEquals("[\n" +
-        "  1000: data_trunc: truncate[8](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+  public TestPartitionSpecUpdate(int formatVersion, int[] expectedFieldIds) {
+    super(formatVersion);
+    this.expectedFieldIds = expectedFieldIds;
   }
 
   @Test
-  public void testCommitUpdatedSpecForV2Table() {
-    TableOperations ops = table.ops();
-    TableMetadata base = ops.current();
-    ops.commit(base, base.upgradeToFormatVersion(2));
-
+  public void testCommitUpdatedSpec() {
     Assert.assertEquals("[\n" +
         "  1000: data_bucket: bucket[16](2)\n" +
         "]", table.spec().toString());
     Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
 
-    PartitionSpec spec = PartitionSpec.builderFor(table.schema())
+    table.updatePartitionSpec().newSpec()
         .bucket("id", 8)
         .bucket("data", 16)
-        .build();
-    table.updatePartitionSpec().update(spec).commit();
+        .commit();
 
-    Assert.assertEquals("[\n" +
-        "  1001: id_bucket: bucket[8](1)\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
+    Assert.assertEquals("[\n  " +
+        expectedFieldIds[0] + ": id_bucket: bucket[8](1)\n  " +
+        expectedFieldIds[1] + ": data_bucket: bucket[16](2)\n" +
         "]", table.spec().toString());
-    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(expectedFieldIds[2], table.spec().lastAssignedFieldId());
 
-    spec = PartitionSpec.builderFor(table.schema())
+    table.updatePartitionSpec().newSpec()
         .truncate("data", 8)
-        .build();
-    table.updatePartitionSpec().update(spec).commit();
+        .commit();
 
-    Assert.assertEquals("[\n" +
-        "  1002: data_trunc: truncate[8](2)\n" +
+    Assert.assertEquals("[\n  " +
+        expectedFieldIds[3] + ": data_trunc: truncate[8](2)\n" +
         "]", table.spec().toString());
-    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(expectedFieldIds[4], table.spec().lastAssignedFieldId());
   }
 
   @Test
   public void testCommitException() {
     AssertHelpers.assertThrows("Should throw IllegalArgumentException if no spec to commit",
-        IllegalArgumentException.class, "new partition spec is not set",
+        NullPointerException.class, "new partition spec is not set",
         () -> table.updatePartitionSpec().commit());
   }
 
   @Test
-  public void testUpdateCompatibility() {
-    PartitionSpec spec = PartitionSpec.builderFor(SCHEMA)
-        .bucket("id", 8)
-        .bucket("data", 16)
-        .build();
-
+  public void testUpdateException() {
     AssertHelpers.assertThrows(
-        "Should throw ValidationException if the new spec is not compatible with the table schema",
-        ValidationException.class, "Cannot find source column for partition field",
-        () -> table.updatePartitionSpec().update(spec));
+        "Should throw IllegalArgumentException if there is an invalid partition field",
+        IllegalArgumentException.class, "Cannot use partition name more than once: id_bucket",
+        () -> table.updatePartitionSpec().newSpec()
+            .bucket("id", 8)
+            .bucket("id", 16)
+            .commit());
   }
 }
diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
index ac139e94855d..d93884500306 100644
--- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
+++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
@@ -509,7 +509,7 @@ public void testNewTableMetadataReassignmentAllIds() throws Exception {
 
     PartitionSpec spec = PartitionSpec.builderFor(schema).withSpecId(5)
         .add(3, 1005, "x_partition", "bucket[4]")
-        .add(5, 1005, "z_partition", "bucket[8]")
+        .add(5, 1003, "z_partition", "bucket[8]")
         .build();
     String location = "file://tmp/db/table";
     TableMetadata metadata = TableMetadata.newTableMetadata(schema, spec, location, ImmutableMap.of());

From 051441c368dda09e05bf067f553fb56062da8761 Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Wed, 27 May 2020 23:32:58 -0700
Subject: [PATCH 05/11] update the API to be change-based

---
 .../main/java/org/apache/iceberg/Table.java   |  4 +-
 .../java/org/apache/iceberg/Transaction.java  |  4 +-
 .../apache/iceberg/UpdatePartitionSpec.java   | 31 ++++-----
 .../org/apache/iceberg/BaseMetadataTable.java |  2 +-
 .../java/org/apache/iceberg/BaseTable.java    |  2 +-
 .../org/apache/iceberg/BaseTransaction.java   |  8 +--
 .../iceberg/CommitCallbackTransaction.java    |  4 +-
 .../apache/iceberg/PartitionSpecUpdate.java   | 59 ++++++++--------
 .../apache/iceberg/TestManifestReader.java    |  2 +-
 .../org/apache/iceberg/TestMergeAppend.java   |  4 +-
 .../apache/iceberg/TestPartitionSpecInfo.java |  2 +-
 .../iceberg/TestPartitionSpecParser.java      |  2 +-
 .../iceberg/TestPartitionSpecUpdate.java      | 68 ++++++++++++++++---
 13 files changed, 118 insertions(+), 74 deletions(-)

diff --git a/api/src/main/java/org/apache/iceberg/Table.java b/api/src/main/java/org/apache/iceberg/Table.java
index 9d4c11c10c05..e66b4d1a0412 100644
--- a/api/src/main/java/org/apache/iceberg/Table.java
+++ b/api/src/main/java/org/apache/iceberg/Table.java
@@ -116,11 +116,11 @@ public interface Table {
   UpdateSchema updateSchema();
 
   /**
-   * Create a new {@link UpdatePartitionSpec} to alter the partition specs of this table and commit the change.
+   * Create a new {@link UpdatePartitionSpec} to alter the partition spec of this table and commit the change.
    *
    * @return a new {@link UpdatePartitionSpec}
    */
-  UpdatePartitionSpec updatePartitionSpec();
+  UpdatePartitionSpec updateSpec();
 
   /**
    * Create a new {@link UpdateProperties} to update table properties and commit the changes.
diff --git a/api/src/main/java/org/apache/iceberg/Transaction.java b/api/src/main/java/org/apache/iceberg/Transaction.java
index 84bd2263bf0f..d5c8a3ca736f 100644
--- a/api/src/main/java/org/apache/iceberg/Transaction.java
+++ b/api/src/main/java/org/apache/iceberg/Transaction.java
@@ -41,11 +41,11 @@ public interface Transaction {
   UpdateSchema updateSchema();
 
   /**
-   * Create a new {@link UpdatePartitionSpec} to alter the partition specs of this table.
+   * Create a new {@link UpdatePartitionSpec} to alter the partition spec of this table.
    *
    * @return a new {@link UpdatePartitionSpec}
    */
-  UpdatePartitionSpec updatePartitionSpec();
+  UpdatePartitionSpec updateSpec();
 
   /**
    * Create a new {@link UpdateProperties} to update table properties.
diff --git a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
index 4d0a5e742dca..dd4269bbb99e 100644
--- a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
@@ -30,14 +30,15 @@
 public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
-   * Create a new partition spec builder for a given schema
+   * Clear all partition fields in the current partition spec.
    * <p>
+   * This will create a new partition spec without any partition field.
    * Partition field IDs is automatically assigned and will be updated during the commit.
    * Table schema should be obtained from the current table metadata
    *
    * @return this for method chaining
    */
-  UpdatePartitionSpec newSpec();
+  UpdatePartitionSpec clear();
 
   /**
    * Add a new partition field with identity transform to the partition spec.
@@ -47,7 +48,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec identity(String sourceName, String targetName);
 
@@ -59,7 +59,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec identity(String sourceName);
 
@@ -71,7 +70,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec year(String sourceName, String targetName);
 
@@ -83,7 +81,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec year(String sourceName);
 
@@ -95,7 +92,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec month(String sourceName, String targetName);
 
@@ -107,7 +103,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec month(String sourceName);
 
@@ -119,7 +114,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec day(String sourceName, String targetName);
 
@@ -131,7 +125,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec day(String sourceName);
 
@@ -143,7 +136,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec hour(String sourceName, String targetName);
 
@@ -155,7 +147,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec hour(String sourceName);
 
@@ -168,7 +159,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param numBuckets the number of buckets
    * @param targetName the name of this partition field
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec bucket(String sourceName, int numBuckets, String targetName);
 
@@ -181,7 +171,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param numBuckets the number of buckets
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec bucket(String sourceName, int numBuckets);
 
@@ -194,7 +183,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param width the width of truncation
    * @param targetName the name of this partition field
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec truncate(String sourceName, int width, String targetName);
 
@@ -207,8 +195,19 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param width the width of truncation
    * @return this for method chaining
-   * @throws NullPointerException If the table schema is not set by {@link #newSpec} before calling this method
    */
   UpdatePartitionSpec truncate(String sourceName, int width);
 
+  /**
+   * Add a new partition field to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceId the source field id in the {@link PartitionSpec spec's} table schema
+   * @param name the name of this partition field
+   * @param transform the partition transform in string format
+   * @return this for method chaining
+   */
+  UpdatePartitionSpec addField(int sourceId, String name, String transform);
+
 }
diff --git a/core/src/main/java/org/apache/iceberg/BaseMetadataTable.java b/core/src/main/java/org/apache/iceberg/BaseMetadataTable.java
index e7c084be3ccc..15d31e2f5dd7 100644
--- a/core/src/main/java/org/apache/iceberg/BaseMetadataTable.java
+++ b/core/src/main/java/org/apache/iceberg/BaseMetadataTable.java
@@ -98,7 +98,7 @@ public UpdateSchema updateSchema() {
   }
 
   @Override
-  public UpdatePartitionSpec updatePartitionSpec() {
+  public UpdatePartitionSpec updateSpec() {
     throw new UnsupportedOperationException("Cannot update the partition spec of a metadata table");
   }
 
diff --git a/core/src/main/java/org/apache/iceberg/BaseTable.java b/core/src/main/java/org/apache/iceberg/BaseTable.java
index e56d597974f6..d7fc5052c1cc 100644
--- a/core/src/main/java/org/apache/iceberg/BaseTable.java
+++ b/core/src/main/java/org/apache/iceberg/BaseTable.java
@@ -104,7 +104,7 @@ public UpdateSchema updateSchema() {
     return new SchemaUpdate(ops);
   }
 
-  public UpdatePartitionSpec updatePartitionSpec() {
+  public UpdatePartitionSpec updateSpec() {
     return new PartitionSpecUpdate(ops);
   }
 
diff --git a/core/src/main/java/org/apache/iceberg/BaseTransaction.java b/core/src/main/java/org/apache/iceberg/BaseTransaction.java
index 87798c2484dd..e9bd49254ebe 100644
--- a/core/src/main/java/org/apache/iceberg/BaseTransaction.java
+++ b/core/src/main/java/org/apache/iceberg/BaseTransaction.java
@@ -103,8 +103,8 @@ public UpdateSchema updateSchema() {
   }
 
   @Override
-  public UpdatePartitionSpec updatePartitionSpec() {
-    checkLastOperationCommitted("UpdatePartitionSpec");
+  public UpdatePartitionSpec updateSpec() {
+    checkLastOperationCommitted("UpdateSpec");
     UpdatePartitionSpec partitionSpecChange = new PartitionSpecUpdate(transactionOps);
     updates.add(partitionSpecChange);
     return partitionSpecChange;
@@ -550,8 +550,8 @@ public UpdateSchema updateSchema() {
     }
 
     @Override
-    public UpdatePartitionSpec updatePartitionSpec() {
-      return BaseTransaction.this.updatePartitionSpec();
+    public UpdatePartitionSpec updateSpec() {
+      return BaseTransaction.this.updateSpec();
     }
 
     @Override
diff --git a/core/src/main/java/org/apache/iceberg/CommitCallbackTransaction.java b/core/src/main/java/org/apache/iceberg/CommitCallbackTransaction.java
index 2740bef0fd96..ef030b401951 100644
--- a/core/src/main/java/org/apache/iceberg/CommitCallbackTransaction.java
+++ b/core/src/main/java/org/apache/iceberg/CommitCallbackTransaction.java
@@ -43,8 +43,8 @@ public UpdateSchema updateSchema() {
   }
 
   @Override
-  public UpdatePartitionSpec updatePartitionSpec() {
-    return wrapped.updatePartitionSpec();
+  public UpdatePartitionSpec updateSpec() {
+    return wrapped.updateSpec();
   }
 
   @Override
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
index aa925c4b5d66..b347dc25e121 100644
--- a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
@@ -30,11 +30,15 @@ class PartitionSpecUpdate implements UpdatePartitionSpec {
 
   private final TableMetadata base;
   private final TableOperations ops;
-  private PartitionSpec.Builder newSpecBuilder = null;
+  private PartitionSpec.Builder newSpecBuilder;
 
   PartitionSpecUpdate(TableOperations ops) {
     this.ops = ops;
     this.base = ops.current();
+    this.newSpecBuilder = PartitionSpec.builderFor(base.schema());
+    for (PartitionField field : base.spec().fields()) {
+      this.newSpecBuilder.add(field.sourceId(), field.fieldId(), field.name(), field.transform().toString());
+    }
   }
 
   @Override
@@ -44,109 +48,102 @@ public PartitionSpec apply() {
   }
 
   @Override
-  public UpdatePartitionSpec newSpec() {
-    newSpecBuilder = PartitionSpec.builderFor(base.schema());
+  public UpdatePartitionSpec clear() {
+    this.newSpecBuilder = PartitionSpec.builderFor(base.schema());
     return this;
   }
 
   @Override
   public UpdatePartitionSpec identity(String sourceName, String targetName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .identity(sourceName, targetName);
+    newSpecBuilder.identity(sourceName, targetName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec identity(String sourceName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .identity(sourceName);
+    newSpecBuilder.identity(sourceName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec year(String sourceName, String targetName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .year(sourceName, targetName);
+    newSpecBuilder.year(sourceName, targetName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec year(String sourceName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .year(sourceName);
+    newSpecBuilder.year(sourceName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec month(String sourceName, String targetName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .month(sourceName, targetName);
+    newSpecBuilder.month(sourceName, targetName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec month(String sourceName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .month(sourceName);
+    newSpecBuilder.month(sourceName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec day(String sourceName, String targetName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .day(sourceName, targetName);
+    newSpecBuilder.day(sourceName, targetName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec day(String sourceName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .day(sourceName);
+    newSpecBuilder.day(sourceName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec hour(String sourceName, String targetName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .hour(sourceName, targetName);
+    newSpecBuilder.hour(sourceName, targetName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec hour(String sourceName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .hour(sourceName);
+    newSpecBuilder.hour(sourceName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec bucket(String sourceName, int numBuckets, String targetName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .bucket(sourceName, numBuckets, targetName);
+    newSpecBuilder.bucket(sourceName, numBuckets, targetName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec bucket(String sourceName, int numBuckets) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .bucket(sourceName, numBuckets);
+    newSpecBuilder.bucket(sourceName, numBuckets);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec truncate(String sourceName, int width, String targetName) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .truncate(sourceName, width, targetName);
+    newSpecBuilder.truncate(sourceName, width, targetName);
     return this;
   }
 
   @Override
   public UpdatePartitionSpec truncate(String sourceName, int width) {
-    Preconditions.checkNotNull(newSpecBuilder, "Please call newSpec method first")
-        .truncate(sourceName, width);
+    newSpecBuilder.truncate(sourceName, width);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec addField(int sourceId, String name, String transform) {
+    newSpecBuilder.add(sourceId, name, transform);
     return this;
   }
 
+
   @Override
   public void commit() {
     TableMetadata update = base.updatePartitionSpec(freshSpecFieldIds(apply()));
diff --git a/core/src/test/java/org/apache/iceberg/TestManifestReader.java b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
index f59987abe61d..39c1184f9b6e 100644
--- a/core/src/test/java/org/apache/iceberg/TestManifestReader.java
+++ b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
@@ -109,7 +109,7 @@ public void testManifestReaderWithUpdatedPartitionMetadata() throws IOException
 
   @Test
   public void testManifestReaderWithPartitionMetadataEvolution() throws IOException {
-    table.updatePartitionSpec().newSpec()
+    table.updateSpec().clear()
         .bucket("id", 8)
         .bucket("data", 16)
         .commit();
diff --git a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
index ac3e39f2d0c9..6fc2ac7170ba 100644
--- a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
+++ b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
@@ -672,7 +672,7 @@ public void testInvalidAppendManifest() throws IOException {
   public void testUpdatePartitionSpecFieldIdsWithSpecEvolution() {
     TableMetadata base = readMetadata();
 
-    table.updatePartitionSpec().newSpec()
+    table.updateSpec().clear()
         .bucket("id", 16)
         .identity("data")
         .bucket("data", 4)
@@ -716,7 +716,7 @@ public void testManifestEntryFieldIdsForChangedPartitionSpecWithSpecEvolution()
         1, base.currentSnapshot().allManifests().size());
     ManifestFile initialManifest = base.currentSnapshot().allManifests().get(0);
 
-    table.updatePartitionSpec().newSpec()
+    table.updateSpec().clear()
         .bucket("id", 8)
         .bucket("data", 8)
         .commit();
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
index 0674048b9a32..5484163014c0 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
@@ -107,7 +107,7 @@ public void testSpecInfoPartitionSpecEvolution() {
         .withSpecId(1)
         .build();
 
-    table.updatePartitionSpec().newSpec()
+    table.updateSpec().clear()
         .bucket("data", 10)
         .commit();
 
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
index c2bcb1487d49..7916b7951bff 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
@@ -55,7 +55,7 @@ public void testToJson() {
         "}";
     Assert.assertEquals(expected, PartitionSpecParser.toJson(table.spec(), true));
 
-    table.updatePartitionSpec().newSpec()
+    table.updateSpec().clear()
         .bucket("id", 8)
         .bucket("data", 16)
         .commit();
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index 402bf1c6a9e7..7654d3725b1d 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -49,7 +49,7 @@ public void testCommitUpdatedSpec() {
         "]", table.spec().toString());
     Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
 
-    table.updatePartitionSpec().newSpec()
+    table.updateSpec().clear()
         .bucket("id", 8)
         .bucket("data", 16)
         .commit();
@@ -60,7 +60,7 @@ public void testCommitUpdatedSpec() {
         "]", table.spec().toString());
     Assert.assertEquals(expectedFieldIds[2], table.spec().lastAssignedFieldId());
 
-    table.updatePartitionSpec().newSpec()
+    table.updateSpec().clear()
         .truncate("data", 8)
         .commit();
 
@@ -70,21 +70,69 @@ public void testCommitUpdatedSpec() {
     Assert.assertEquals(expectedFieldIds[4], table.spec().lastAssignedFieldId());
   }
 
-  @Test
-  public void testCommitException() {
-    AssertHelpers.assertThrows("Should throw IllegalArgumentException if no spec to commit",
-        NullPointerException.class, "new partition spec is not set",
-        () -> table.updatePartitionSpec().commit());
-  }
-
   @Test
   public void testUpdateException() {
     AssertHelpers.assertThrows(
         "Should throw IllegalArgumentException if there is an invalid partition field",
         IllegalArgumentException.class, "Cannot use partition name more than once: id_bucket",
-        () -> table.updatePartitionSpec().newSpec()
+        () -> table.updateSpec().clear()
             .bucket("id", 8)
             .bucket("id", 16)
             .commit());
   }
+
+  @Test
+  public void testAddDuplicateFieldException() {
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if adding a duplicate partition field",
+        IllegalArgumentException.class, "Cannot use partition name more than once: data_bucket",
+        () -> table.updateSpec()
+            .bucket("data", 16)
+            .commit());
+  }
+
+  @Test
+  public void testAddTheSamePartitionField() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+
+    if (formatVersion == 1) {
+      table.updateSpec()
+          .addField(2, "data_partition", "bucket[16]")
+          .commit();
+      Assert.assertEquals("[\n" +
+          "  1000: data_bucket: bucket[16](2)\n" +
+          "  1001: data_partition: bucket[16](2)\n" +
+          "]", table.spec().toString());
+    } else {
+      AssertHelpers.assertThrows(
+          "Should throw IllegalArgumentException if adding a duplicate partition field",
+          IllegalArgumentException.class,
+          "Field Id 1000 has already been used in the existing partition fields",
+          () -> table.updateSpec()
+              .addField(2, "data_partition", "bucket[16]")
+              .commit());
+    }
+  }
+
+  @Test
+  public void testAddField() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .addField(2, "data_partition", "bucket[8]")
+        .bucket("id", 8)
+        .commit();
+
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "  1001: data_partition: bucket[8](2)\n" +
+        "  1002: id_bucket: bucket[8](1)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+  }
 }

From 51b71d860003d831fa80359eda7a4b64e9696658 Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Wed, 3 Jun 2020 19:21:14 -0700
Subject: [PATCH 06/11] add additional change based APIs

---
 .../org/apache/iceberg/PartitionSpec.java     | 40 +++++++++
 .../apache/iceberg/UpdatePartitionSpec.java   | 83 +++++++++++++++----
 .../apache/iceberg/PartitionSpecUpdate.java   | 56 +++++++++----
 3 files changed, 151 insertions(+), 28 deletions(-)

diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
index 9f4650aef568..8546e44498c0 100644
--- a/api/src/main/java/org/apache/iceberg/PartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
@@ -488,6 +488,46 @@ Builder add(int sourceId, int fieldId, String name, String transform) {
       return this;
     }
 
+    Builder rename(String name, String newName) {
+      Preconditions.checkArgument(partitionNames.contains(name),
+          "Cannot find an existing partition field with the name: %s", name);
+      Preconditions.checkArgument(newName != null && !newName.isEmpty(),
+          "Cannot use empty or null partition name: %s", newName);
+      Preconditions.checkArgument(!partitionNames.contains(newName),
+          "Cannot use partition name more than once: %s", newName);
+
+      for (int i = 0; i < fields.size(); ++i) {
+        PartitionField field = fields.get(i);
+        if (field.name().equals(name)) {
+          fields.set(i, new PartitionField(field.sourceId(), field.fieldId(), newName, field.transform()));
+          partitionNames.remove(name);
+          partitionNames.add(newName);
+          break;
+        }
+      }
+      return this;
+    }
+
+    int remove(String name, boolean softDelete) {
+      for (int i = 0; i < fields.size(); ++i) {
+        PartitionField field = fields.get(i);
+        if (field.name().equals(name)) {
+          if (softDelete) {
+            String newName = field.name() + "_removed"; // rename it for soft delete
+            Preconditions.checkArgument(!partitionNames.contains(newName),
+                "Cannot soft delete the partition %s as its soft delete name %s has already been used", name, newName);
+            fields.set(i, new PartitionField(field.sourceId(), field.fieldId(), newName, Transforms.alwaysNull()));
+            partitionNames.add(newName);
+          } else {
+            fields.remove(i);
+          }
+          partitionNames.remove(name);
+          return field.sourceId();
+        }
+      }
+      throw new IllegalStateException("Cannot find an existing partition field with the name: " + name);
+    }
+
     public PartitionSpec build() {
       PartitionSpec spec = new PartitionSpec(schema, specId, fields, lastAssignedFieldId.get());
       checkCompatibility(spec, schema);
diff --git a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
index dd4269bbb99e..e90a5461596d 100644
--- a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
@@ -20,6 +20,7 @@
 package org.apache.iceberg;
 
 import org.apache.iceberg.exceptions.CommitFailedException;
+import org.apache.iceberg.transforms.Transform;
 
 /**
  * API for partition spec evolution.
@@ -49,7 +50,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param targetName the name of this partition field
    * @return this for method chaining
    */
-  UpdatePartitionSpec identity(String sourceName, String targetName);
+  UpdatePartitionSpec addIdentityField(String sourceName, String targetName);
 
   /**
    * Add a new partition field with identity transform to the partition spec.
@@ -60,7 +61,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
    */
-  UpdatePartitionSpec identity(String sourceName);
+  UpdatePartitionSpec addIdentityField(String sourceName);
 
   /**
    * Add a new partition field with year transform to the partition spec.
@@ -71,7 +72,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param targetName the name of this partition field
    * @return this for method chaining
    */
-  UpdatePartitionSpec year(String sourceName, String targetName);
+  UpdatePartitionSpec addYearField(String sourceName, String targetName);
 
   /**
    * Add a new partition field with year transform to the partition spec.
@@ -82,7 +83,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
    */
-  UpdatePartitionSpec year(String sourceName);
+  UpdatePartitionSpec addYearField(String sourceName);
 
   /**
    * Add a new partition field with month transform to the partition spec.
@@ -93,7 +94,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param targetName the name of this partition field
    * @return this for method chaining
    */
-  UpdatePartitionSpec month(String sourceName, String targetName);
+  UpdatePartitionSpec addMonthField(String sourceName, String targetName);
 
   /**
    * Add a new partition field with month transform to the partition spec.
@@ -104,7 +105,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
    */
-  UpdatePartitionSpec month(String sourceName);
+  UpdatePartitionSpec addMonthField(String sourceName);
 
   /**
    * Add a new partition field with day transform to the partition spec.
@@ -115,7 +116,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param targetName the name of this partition field
    * @return this for method chaining
    */
-  UpdatePartitionSpec day(String sourceName, String targetName);
+  UpdatePartitionSpec addDayField(String sourceName, String targetName);
 
   /**
    * Add a new partition field with day transform to the partition spec.
@@ -126,7 +127,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
    */
-  UpdatePartitionSpec day(String sourceName);
+  UpdatePartitionSpec addDayField(String sourceName);
 
   /**
    * Add a new partition field with hour transform to the partition spec.
@@ -137,7 +138,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param targetName the name of this partition field
    * @return this for method chaining
    */
-  UpdatePartitionSpec hour(String sourceName, String targetName);
+  UpdatePartitionSpec addHourField(String sourceName, String targetName);
 
   /**
    * Add a new partition field with hour transform to the partition spec.
@@ -148,7 +149,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
    */
-  UpdatePartitionSpec hour(String sourceName);
+  UpdatePartitionSpec addHourField(String sourceName);
 
   /**
    * Add a new partition field with bucket transform to the partition spec.
@@ -160,7 +161,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param targetName the name of this partition field
    * @return this for method chaining
    */
-  UpdatePartitionSpec bucket(String sourceName, int numBuckets, String targetName);
+  UpdatePartitionSpec addBucketField(String sourceName, int numBuckets, String targetName);
 
   /**
    * Add a new partition field with bucket transform to the partition spec.
@@ -172,7 +173,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param numBuckets the number of buckets
    * @return this for method chaining
    */
-  UpdatePartitionSpec bucket(String sourceName, int numBuckets);
+  UpdatePartitionSpec addBucketField(String sourceName, int numBuckets);
 
   /**
    * Add a new partition field with truncate transform to the partition spec.
@@ -184,7 +185,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param targetName the name of this partition field
    * @return this for method chaining
    */
-  UpdatePartitionSpec truncate(String sourceName, int width, String targetName);
+  UpdatePartitionSpec addTruncateField(String sourceName, int width, String targetName);
 
   /**
    * Add a new partition field with truncate transform to the partition spec.
@@ -196,7 +197,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    * @param width the width of truncation
    * @return this for method chaining
    */
-  UpdatePartitionSpec truncate(String sourceName, int width);
+  UpdatePartitionSpec addTruncateField(String sourceName, int width);
 
   /**
    * Add a new partition field to the partition spec.
@@ -210,4 +211,58 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    */
   UpdatePartitionSpec addField(int sourceId, String name, String transform);
 
+  /**
+   * Add a new partition field to the partition spec.
+   * <p>
+   * The partition field id is automatically assigned and will be updated during the commit.
+   *
+   * @param sourceId the source field id in the {@link PartitionSpec spec's} table schema
+   * @param name the name of this partition field
+   * @param transform the partition transform
+   * @return this for method chaining
+   */
+  UpdatePartitionSpec addField(int sourceId, String name, Transform<?, ?> transform);
+
+  /**
+   * Rename a partition field in the partition spec.
+   * <p>
+   *
+   * @param name the name of a partition field to be renamed
+   * @param newName the new name of the partition field
+   * @return this for method chaining
+   */
+  UpdatePartitionSpec renameField(String name, String newName);
+
+  /**
+   * Remove a partition field in the partition spec.
+   * <p>
+   * The partition field will be soft deleted for a table with V1 metadata and hard deleted in a higher version.
+   *
+   * @param name the name of a partition field to be removed
+   * @return this for method chaining
+   */
+  UpdatePartitionSpec removeField(String name);
+
+  /**
+   * Replace a partition field with a new transform in the partition spec.
+   * <p>
+   * It is equivalent to remove the partition field and then add it back with the new transform
+   *
+   * @param name the name of a partition field to be replaced
+   * @param transform the new partition transform to be used in string format
+   * @return this for method chaining
+   */
+  UpdatePartitionSpec replaceField(String name, String transform);
+
+  /**
+   * Replace a partition field with a new transform in the partition spec.
+   * <p>
+   * It is equivalent to remove the partition field and then add it back with the new transform
+   *
+   * @param name the name of a partition field to be replaced
+   * @param transform the new partition transform to be used
+   * @return this for method chaining
+   */
+  UpdatePartitionSpec replaceField(String name, Transform<?, ?> transform);
+
 }
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
index b347dc25e121..624270e6b415 100644
--- a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
@@ -22,6 +22,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Maps;
 import java.util.Map;
+import org.apache.iceberg.transforms.Transform;
 
 /**
  * PartitionSpec evolution API implementation.
@@ -54,85 +55,85 @@ public UpdatePartitionSpec clear() {
   }
 
   @Override
-  public UpdatePartitionSpec identity(String sourceName, String targetName) {
+  public UpdatePartitionSpec addIdentityField(String sourceName, String targetName) {
     newSpecBuilder.identity(sourceName, targetName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec identity(String sourceName) {
+  public UpdatePartitionSpec addIdentityField(String sourceName) {
     newSpecBuilder.identity(sourceName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec year(String sourceName, String targetName) {
+  public UpdatePartitionSpec addYearField(String sourceName, String targetName) {
     newSpecBuilder.year(sourceName, targetName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec year(String sourceName) {
+  public UpdatePartitionSpec addYearField(String sourceName) {
     newSpecBuilder.year(sourceName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec month(String sourceName, String targetName) {
+  public UpdatePartitionSpec addMonthField(String sourceName, String targetName) {
     newSpecBuilder.month(sourceName, targetName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec month(String sourceName) {
+  public UpdatePartitionSpec addMonthField(String sourceName) {
     newSpecBuilder.month(sourceName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec day(String sourceName, String targetName) {
+  public UpdatePartitionSpec addDayField(String sourceName, String targetName) {
     newSpecBuilder.day(sourceName, targetName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec day(String sourceName) {
+  public UpdatePartitionSpec addDayField(String sourceName) {
     newSpecBuilder.day(sourceName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec hour(String sourceName, String targetName) {
+  public UpdatePartitionSpec addHourField(String sourceName, String targetName) {
     newSpecBuilder.hour(sourceName, targetName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec hour(String sourceName) {
+  public UpdatePartitionSpec addHourField(String sourceName) {
     newSpecBuilder.hour(sourceName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec bucket(String sourceName, int numBuckets, String targetName) {
+  public UpdatePartitionSpec addBucketField(String sourceName, int numBuckets, String targetName) {
     newSpecBuilder.bucket(sourceName, numBuckets, targetName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec bucket(String sourceName, int numBuckets) {
+  public UpdatePartitionSpec addBucketField(String sourceName, int numBuckets) {
     newSpecBuilder.bucket(sourceName, numBuckets);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec truncate(String sourceName, int width, String targetName) {
+  public UpdatePartitionSpec addTruncateField(String sourceName, int width, String targetName) {
     newSpecBuilder.truncate(sourceName, width, targetName);
     return this;
   }
 
   @Override
-  public UpdatePartitionSpec truncate(String sourceName, int width) {
+  public UpdatePartitionSpec addTruncateField(String sourceName, int width) {
     newSpecBuilder.truncate(sourceName, width);
     return this;
   }
@@ -143,6 +144,33 @@ public UpdatePartitionSpec addField(int sourceId, String name, String transform)
     return this;
   }
 
+  @Override
+  public UpdatePartitionSpec addField(int sourceId, String name, Transform<?, ?> transform) {
+    return addField(sourceId, name, transform.toString());
+  }
+
+  @Override
+  public UpdatePartitionSpec renameField(String name, String newName) {
+    newSpecBuilder.rename(name, newName);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec removeField(String name) {
+    newSpecBuilder.remove(name, base.formatVersion() == 1);
+    return this;
+  }
+
+  @Override
+  public UpdatePartitionSpec replaceField(String name, String transform) {
+    int sourceId = newSpecBuilder.remove(name, base.formatVersion() == 1);
+    return addField(sourceId, name, transform);
+  }
+
+  @Override
+  public UpdatePartitionSpec replaceField(String name, Transform<?, ?> transform) {
+    return replaceField(name, transform.toString());
+  }
 
   @Override
   public void commit() {

From 15f7f4f547a28d5246551da1653b76bacfcd5308 Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Wed, 3 Jun 2020 19:21:34 -0700
Subject: [PATCH 07/11] add additional unit tests

---
 .../apache/iceberg/TestManifestReader.java    |   4 +-
 .../org/apache/iceberg/TestMergeAppend.java   |  12 +-
 .../apache/iceberg/TestPartitionSpecInfo.java |   2 +-
 .../iceberg/TestPartitionSpecParser.java      |   4 +-
 .../iceberg/TestPartitionSpecUpdate.java      | 158 ++++++++++++++++--
 5 files changed, 158 insertions(+), 22 deletions(-)

diff --git a/core/src/test/java/org/apache/iceberg/TestManifestReader.java b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
index 39c1184f9b6e..2e1e4b945eef 100644
--- a/core/src/test/java/org/apache/iceberg/TestManifestReader.java
+++ b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
@@ -110,8 +110,8 @@ public void testManifestReaderWithUpdatedPartitionMetadata() throws IOException
   @Test
   public void testManifestReaderWithPartitionMetadataEvolution() throws IOException {
     table.updateSpec().clear()
-        .bucket("id", 8)
-        .bucket("data", 16)
+        .addBucketField("id", 8)
+        .addBucketField("data", 16)
         .commit();
 
     ManifestFile manifest = writeManifest(1000L, manifestEntry(Status.EXISTING, 123L, FILE_A));
diff --git a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
index 6fc2ac7170ba..6a903f60c2e2 100644
--- a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
+++ b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
@@ -673,10 +673,10 @@ public void testUpdatePartitionSpecFieldIdsWithSpecEvolution() {
     TableMetadata base = readMetadata();
 
     table.updateSpec().clear()
-        .bucket("id", 16)
-        .identity("data")
-        .bucket("data", 4)
-        .bucket("data", 16, "data_partition") // reuse field id although different target name
+        .addBucketField("id", 16)
+        .addIdentityField("data")
+        .addBucketField("data", 4)
+        .addBucketField("data", 16, "data_partition") // reuse field id although different target name
         .commit();
 
     List<PartitionSpec> partitionSpecs = table.ops().current().specs();
@@ -717,8 +717,8 @@ public void testManifestEntryFieldIdsForChangedPartitionSpecWithSpecEvolution()
     ManifestFile initialManifest = base.currentSnapshot().allManifests().get(0);
 
     table.updateSpec().clear()
-        .bucket("id", 8)
-        .bucket("data", 8)
+        .addBucketField("id", 8)
+        .addBucketField("data", 8)
         .commit();
 
     DataFile newFile = DataFiles.builder(table.spec())
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
index 5484163014c0..3abb2c1405a5 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
@@ -108,7 +108,7 @@ public void testSpecInfoPartitionSpecEvolution() {
         .build();
 
     table.updateSpec().clear()
-        .bucket("data", 10)
+        .addBucketField("data", 10)
         .commit();
 
     Assert.assertEquals(expectedNewSpec, table.spec());
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
index 7916b7951bff..4502538fc9ab 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
@@ -56,8 +56,8 @@ public void testToJson() {
     Assert.assertEquals(expected, PartitionSpecParser.toJson(table.spec(), true));
 
     table.updateSpec().clear()
-        .bucket("id", 8)
-        .bucket("data", 16)
+        .addBucketField("id", 8)
+        .addBucketField("data", 16)
         .commit();
 
     expected = "{\n" +
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index 7654d3725b1d..ab45ab06f947 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -19,6 +19,8 @@
 
 package org.apache.iceberg;
 
+import org.apache.iceberg.transforms.Transforms;
+import org.apache.iceberg.types.Types;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -29,17 +31,21 @@ public class TestPartitionSpecUpdate extends TableTestBase {
 
   private int[] expectedFieldIds;
 
+  private String[] expectedSpecs;
+
   @Parameterized.Parameters
   public static Object[][] parameters() {
     return new Object[][] {
-        new Object[] { 1, new int[]{ 1000, 1001, 1001, 1000, 1000 } },
-        new Object[] { 2, new int[]{ 1001, 1000, 1001, 1002, 1002 } },
+        new Object[] { 1, new int[]{ 1000, 1001, 1001, 1000, 1000 },
+            new String[]{"  1000: data_bucket_removed: void(2)\n", "  1002: id_bucket_removed: void(1)\n"} },
+        new Object[] { 2, new int[]{ 1001, 1000, 1001, 1002, 1002 }, new String[]{"", ""} },
     };
   }
 
-  public TestPartitionSpecUpdate(int formatVersion, int[] expectedFieldIds) {
+  public TestPartitionSpecUpdate(int formatVersion, int[] expectedFieldIds, String[] expectedSpecs) {
     super(formatVersion);
     this.expectedFieldIds = expectedFieldIds;
+    this.expectedSpecs = expectedSpecs;
   }
 
   @Test
@@ -50,8 +56,8 @@ public void testCommitUpdatedSpec() {
     Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
 
     table.updateSpec().clear()
-        .bucket("id", 8)
-        .bucket("data", 16)
+        .addBucketField("id", 8)
+        .addBucketField("data", 16)
         .commit();
 
     Assert.assertEquals("[\n  " +
@@ -61,7 +67,7 @@ public void testCommitUpdatedSpec() {
     Assert.assertEquals(expectedFieldIds[2], table.spec().lastAssignedFieldId());
 
     table.updateSpec().clear()
-        .truncate("data", 8)
+        .addTruncateField("data", 8)
         .commit();
 
     Assert.assertEquals("[\n  " +
@@ -76,8 +82,8 @@ public void testUpdateException() {
         "Should throw IllegalArgumentException if there is an invalid partition field",
         IllegalArgumentException.class, "Cannot use partition name more than once: id_bucket",
         () -> table.updateSpec().clear()
-            .bucket("id", 8)
-            .bucket("id", 16)
+            .addBucketField("id", 8)
+            .addBucketField("id", 16)
             .commit());
   }
 
@@ -87,12 +93,12 @@ public void testAddDuplicateFieldException() {
         "Should throw IllegalArgumentException if adding a duplicate partition field",
         IllegalArgumentException.class, "Cannot use partition name more than once: data_bucket",
         () -> table.updateSpec()
-            .bucket("data", 16)
+            .addBucketField("data", 16)
             .commit());
   }
 
   @Test
-  public void testAddTheSamePartitionField() {
+  public void testAddSamePartitionField() {
     Assert.assertEquals("[\n" +
         "  1000: data_bucket: bucket[16](2)\n" +
         "]", table.spec().toString());
@@ -125,14 +131,144 @@ public void testAddField() {
 
     table.updateSpec()
         .addField(2, "data_partition", "bucket[8]")
-        .bucket("id", 8)
+        .addBucketField("id", 8)
+        .addField(2, "data_field", Transforms.bucket(Types.StringType.get(), 6))
         .commit();
 
     Assert.assertEquals("[\n" +
         "  1000: data_bucket: bucket[16](2)\n" +
         "  1001: data_partition: bucket[8](2)\n" +
         "  1002: id_bucket: bucket[8](1)\n" +
+        "  1003: data_field: bucket[6](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1003, table.spec().lastAssignedFieldId());
+  }
+
+  @Test
+  public void testRenameField() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(0, table.spec().specId());
+
+    table.updateSpec()
+        .renameField("data_bucket", "data_partition")
+        .addBucketField("id", 8)
+        .commit();
+
+    Assert.assertEquals("[\n" +
+        "  1000: data_partition: bucket[16](2)\n" +
+        "  1001: id_bucket: bucket[8](1)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(1, table.spec().specId());
+  }
+
+  @Test
+  public void testRenameFieldExceptions() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(0, table.spec().specId());
+
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if only renaming a partition field",
+        IllegalArgumentException.class,
+        "Cannot set default partition spec to the current default",
+        () -> table.updateSpec()
+            .renameField("data_bucket", "data_partition")
+            .commit());
+
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if renaming a non-existing partition field",
+        IllegalArgumentException.class,
+        "Cannot find an existing partition field with the name: not_existing",
+        () -> table.updateSpec()
+            .renameField("not_existing", "data_partition")
+            .commit());
+
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if renaming a partition field to null",
+        IllegalArgumentException.class,
+        "Cannot use empty or null partition name: null",
+        () -> table.updateSpec()
+            .renameField("data_bucket", null)
+            .commit());
+  }
+
+  @Test
+  public void testRemoveField() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(0, table.spec().specId());
+
+    table.updateSpec()
+        .removeField("data_bucket")
+        .addBucketField("id", 8)
+        .commit();
+
+    Assert.assertEquals("[\n" +
+        this.expectedSpecs[0] +
+        "  1001: id_bucket: bucket[8](1)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(1, table.spec().specId());
+  }
+
+  @Test
+  public void testRemoveFieldException() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(0, table.spec().specId());
+
+    AssertHelpers.assertThrows(
+        "Should throw IllegalStateException if removing a non-existing partition field",
+        IllegalStateException.class,
+        "Cannot find an existing partition field with the name: not_existing",
+        () -> table.updateSpec()
+            .removeField("not_existing")
+            .commit());
+  }
+
+  @Test
+  public void testReplaceField() {
+    Assert.assertEquals("[\n" +
+        "  1000: data_bucket: bucket[16](2)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(0, table.spec().specId());
+
+    table.updateSpec()
+        .replaceField("data_bucket", "bucket[8]")
+        .addBucketField("id", 8)
+        .commit();
+
+    Assert.assertEquals("[\n" +
+        this.expectedSpecs[0] +
+        "  1001: data_bucket: bucket[8](2)\n" +
+        "  1002: id_bucket: bucket[8](1)\n" +
         "]", table.spec().toString());
     Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(1, table.spec().specId());
+
+    table.updateSpec()
+        .replaceField("id_bucket", Transforms.bucket(Types.StringType.get(), 16))
+        .commit();
+
+    Assert.assertEquals("[\n" +
+        this.expectedSpecs[0] +
+        "  1001: data_bucket: bucket[8](2)\n" +
+        this.expectedSpecs[1] +
+        "  1003: id_bucket: bucket[16](1)\n" +
+        "]", table.spec().toString());
+    Assert.assertEquals(1003, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(2, table.spec().specId());
   }
+
 }

From 557df5bf3fca9a8558e9a72212bec0f0909f5697 Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Sun, 14 Jun 2020 14:59:23 -0700
Subject: [PATCH 08/11] update the PR and address the comments

---
 .../org/apache/iceberg/PartitionSpec.java     |  40 ------
 .../apache/iceberg/UpdatePartitionSpec.java   |  86 +------------
 .../apache/iceberg/PartitionSpecUpdate.java   | 119 ++++++++++--------
 .../iceberg/TestPartitionSpecUpdate.java      |  47 +------
 4 files changed, 75 insertions(+), 217 deletions(-)

diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
index 8546e44498c0..9f4650aef568 100644
--- a/api/src/main/java/org/apache/iceberg/PartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
@@ -488,46 +488,6 @@ Builder add(int sourceId, int fieldId, String name, String transform) {
       return this;
     }
 
-    Builder rename(String name, String newName) {
-      Preconditions.checkArgument(partitionNames.contains(name),
-          "Cannot find an existing partition field with the name: %s", name);
-      Preconditions.checkArgument(newName != null && !newName.isEmpty(),
-          "Cannot use empty or null partition name: %s", newName);
-      Preconditions.checkArgument(!partitionNames.contains(newName),
-          "Cannot use partition name more than once: %s", newName);
-
-      for (int i = 0; i < fields.size(); ++i) {
-        PartitionField field = fields.get(i);
-        if (field.name().equals(name)) {
-          fields.set(i, new PartitionField(field.sourceId(), field.fieldId(), newName, field.transform()));
-          partitionNames.remove(name);
-          partitionNames.add(newName);
-          break;
-        }
-      }
-      return this;
-    }
-
-    int remove(String name, boolean softDelete) {
-      for (int i = 0; i < fields.size(); ++i) {
-        PartitionField field = fields.get(i);
-        if (field.name().equals(name)) {
-          if (softDelete) {
-            String newName = field.name() + "_removed"; // rename it for soft delete
-            Preconditions.checkArgument(!partitionNames.contains(newName),
-                "Cannot soft delete the partition %s as its soft delete name %s has already been used", name, newName);
-            fields.set(i, new PartitionField(field.sourceId(), field.fieldId(), newName, Transforms.alwaysNull()));
-            partitionNames.add(newName);
-          } else {
-            fields.remove(i);
-          }
-          partitionNames.remove(name);
-          return field.sourceId();
-        }
-      }
-      throw new IllegalStateException("Cannot find an existing partition field with the name: " + name);
-    }
-
     public PartitionSpec build() {
       PartitionSpec spec = new PartitionSpec(schema, specId, fields, lastAssignedFieldId.get());
       checkCompatibility(spec, schema);
diff --git a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
index e90a5461596d..eaa6c5ae5c3d 100644
--- a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
@@ -20,7 +20,6 @@
 package org.apache.iceberg;
 
 import org.apache.iceberg.exceptions.CommitFailedException;
-import org.apache.iceberg.transforms.Transform;
 
 /**
  * API for partition spec evolution.
@@ -33,9 +32,7 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
   /**
    * Clear all partition fields in the current partition spec.
    * <p>
-   * This will create a new partition spec without any partition field.
-   * Partition field IDs is automatically assigned and will be updated during the commit.
-   * Table schema should be obtained from the current table metadata
+   * It will remove all existing fields.
    *
    * @return this for method chaining
    */
@@ -44,7 +41,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
   /**
    * Add a new partition field with identity transform to the partition spec.
    * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
@@ -54,9 +50,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with identity transform to the partition spec.
-   * <p>
-   * The partition field name is automatically assigned set.
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
@@ -65,8 +58,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with year transform to the partition spec.
-   * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
@@ -76,9 +67,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with year transform to the partition spec.
-   * <p>
-   * The partition field name is automatically assigned set.
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
@@ -87,8 +75,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with month transform to the partition spec.
-   * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
@@ -98,9 +84,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with month transform to the partition spec.
-   * <p>
-   * The partition field name is automatically assigned set.
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
@@ -109,8 +92,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with day transform to the partition spec.
-   * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
@@ -120,9 +101,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with day transform to the partition spec.
-   * <p>
-   * The partition field name is automatically assigned set.
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
@@ -131,8 +109,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with hour transform to the partition spec.
-   * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param targetName the name of this partition field
@@ -142,9 +118,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with hour transform to the partition spec.
-   * <p>
-   * The partition field name is automatically assigned set.
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @return this for method chaining
@@ -153,8 +126,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with bucket transform to the partition spec.
-   * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param numBuckets the number of buckets
@@ -165,9 +136,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with bucket transform to the partition spec.
-   * <p>
-   * The partition field name is automatically assigned set.
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param numBuckets the number of buckets
@@ -177,8 +145,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with truncate transform to the partition spec.
-   * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param width the width of truncation
@@ -189,9 +155,6 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
   /**
    * Add a new partition field with truncate transform to the partition spec.
-   * <p>
-   * The partition field name is automatically assigned set.
-   * The partition field id is automatically assigned and will be updated during the commit.
    *
    * @param sourceName the field name of the source field in the {@link PartitionSpec spec's} table schema
    * @param width the width of truncation
@@ -199,33 +162,8 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    */
   UpdatePartitionSpec addTruncateField(String sourceName, int width);
 
-  /**
-   * Add a new partition field to the partition spec.
-   * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
-   *
-   * @param sourceId the source field id in the {@link PartitionSpec spec's} table schema
-   * @param name the name of this partition field
-   * @param transform the partition transform in string format
-   * @return this for method chaining
-   */
-  UpdatePartitionSpec addField(int sourceId, String name, String transform);
-
-  /**
-   * Add a new partition field to the partition spec.
-   * <p>
-   * The partition field id is automatically assigned and will be updated during the commit.
-   *
-   * @param sourceId the source field id in the {@link PartitionSpec spec's} table schema
-   * @param name the name of this partition field
-   * @param transform the partition transform
-   * @return this for method chaining
-   */
-  UpdatePartitionSpec addField(int sourceId, String name, Transform<?, ?> transform);
-
   /**
    * Rename a partition field in the partition spec.
-   * <p>
    *
    * @param name the name of a partition field to be renamed
    * @param newName the new name of the partition field
@@ -243,26 +181,4 @@ public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
    */
   UpdatePartitionSpec removeField(String name);
 
-  /**
-   * Replace a partition field with a new transform in the partition spec.
-   * <p>
-   * It is equivalent to remove the partition field and then add it back with the new transform
-   *
-   * @param name the name of a partition field to be replaced
-   * @param transform the new partition transform to be used in string format
-   * @return this for method chaining
-   */
-  UpdatePartitionSpec replaceField(String name, String transform);
-
-  /**
-   * Replace a partition field with a new transform in the partition spec.
-   * <p>
-   * It is equivalent to remove the partition field and then add it back with the new transform
-   *
-   * @param name the name of a partition field to be replaced
-   * @param transform the new partition transform to be used
-   * @return this for method chaining
-   */
-  UpdatePartitionSpec replaceField(String name, Transform<?, ?> transform);
-
 }
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
index 624270e6b415..dcf4fe9ccef0 100644
--- a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
@@ -19,10 +19,17 @@
 
 package org.apache.iceberg;
 
-import com.google.common.base.Preconditions;
-import com.google.common.collect.Maps;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
-import org.apache.iceberg.transforms.Transform;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.transforms.Transforms;
 
 /**
  * PartitionSpec evolution API implementation.
@@ -31,147 +38,159 @@ class PartitionSpecUpdate implements UpdatePartitionSpec {
 
   private final TableMetadata base;
   private final TableOperations ops;
-  private PartitionSpec.Builder newSpecBuilder;
+  private final List<Consumer<PartitionSpec.Builder>> newSpecFields = new ArrayList<>();
+  private final Map<String, PartitionField> curSpecFields;
 
   PartitionSpecUpdate(TableOperations ops) {
     this.ops = ops;
     this.base = ops.current();
-    this.newSpecBuilder = PartitionSpec.builderFor(base.schema());
-    for (PartitionField field : base.spec().fields()) {
-      this.newSpecBuilder.add(field.sourceId(), field.fieldId(), field.name(), field.transform().toString());
-    }
+    this.curSpecFields = base.spec().fields().stream().collect(
+        Collectors.toMap(
+            PartitionField::name,
+            Function.identity(),
+            (n1, n2) -> {
+              throw new IllegalStateException(String.format("Duplicate partition field found: %s", n1));
+            },
+            LinkedHashMap::new
+        )
+    );
   }
 
   @Override
   public PartitionSpec apply() {
-    Preconditions.checkNotNull(newSpecBuilder, "new partition spec is not set");
-    return newSpecBuilder.build();
+    PartitionSpec.Builder specBuilder = PartitionSpec.builderFor(base.schema());
+    curSpecFields.values().forEach(field ->
+        specBuilder.add(
+            field.sourceId(),
+            field.fieldId(),
+            field.name(),
+            field.transform().toString())
+    );
+    newSpecFields.forEach(c -> c.accept(specBuilder));
+    return specBuilder.build();
   }
 
   @Override
   public UpdatePartitionSpec clear() {
-    this.newSpecBuilder = PartitionSpec.builderFor(base.schema());
+    newSpecFields.clear();
+    curSpecFields.clear();
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addIdentityField(String sourceName, String targetName) {
-    newSpecBuilder.identity(sourceName, targetName);
+    newSpecFields.add(builder -> builder.identity(sourceName, targetName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addIdentityField(String sourceName) {
-    newSpecBuilder.identity(sourceName);
+    newSpecFields.add(builder -> builder.identity(sourceName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addYearField(String sourceName, String targetName) {
-    newSpecBuilder.year(sourceName, targetName);
+    newSpecFields.add(builder -> builder.year(sourceName, targetName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addYearField(String sourceName) {
-    newSpecBuilder.year(sourceName);
+    newSpecFields.add(builder -> builder.year(sourceName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addMonthField(String sourceName, String targetName) {
-    newSpecBuilder.month(sourceName, targetName);
+    newSpecFields.add(builder -> builder.month(sourceName, targetName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addMonthField(String sourceName) {
-    newSpecBuilder.month(sourceName);
+    newSpecFields.add(builder -> builder.month(sourceName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addDayField(String sourceName, String targetName) {
-    newSpecBuilder.day(sourceName, targetName);
+    newSpecFields.add(builder -> builder.day(sourceName, targetName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addDayField(String sourceName) {
-    newSpecBuilder.day(sourceName);
+    newSpecFields.add(builder -> builder.day(sourceName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addHourField(String sourceName, String targetName) {
-    newSpecBuilder.hour(sourceName, targetName);
+    newSpecFields.add(builder -> builder.hour(sourceName, targetName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addHourField(String sourceName) {
-    newSpecBuilder.hour(sourceName);
+    newSpecFields.add(builder -> builder.hour(sourceName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addBucketField(String sourceName, int numBuckets, String targetName) {
-    newSpecBuilder.bucket(sourceName, numBuckets, targetName);
+    newSpecFields.add(builder -> builder.bucket(sourceName, numBuckets, targetName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addBucketField(String sourceName, int numBuckets) {
-    newSpecBuilder.bucket(sourceName, numBuckets);
+    newSpecFields.add(builder -> builder.bucket(sourceName, numBuckets));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addTruncateField(String sourceName, int width, String targetName) {
-    newSpecBuilder.truncate(sourceName, width, targetName);
+    newSpecFields.add(builder -> builder.truncate(sourceName, width, targetName));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec addTruncateField(String sourceName, int width) {
-    newSpecBuilder.truncate(sourceName, width);
-    return this;
-  }
-
-  @Override
-  public UpdatePartitionSpec addField(int sourceId, String name, String transform) {
-    newSpecBuilder.add(sourceId, name, transform);
+    newSpecFields.add(builder -> builder.truncate(sourceName, width));
     return this;
   }
 
-  @Override
-  public UpdatePartitionSpec addField(int sourceId, String name, Transform<?, ?> transform) {
-    return addField(sourceId, name, transform.toString());
-  }
-
   @Override
   public UpdatePartitionSpec renameField(String name, String newName) {
-    newSpecBuilder.rename(name, newName);
+    Preconditions.checkArgument(curSpecFields.containsKey(name),
+        "Cannot find an existing partition field with the name: %s", name);
+    Preconditions.checkArgument(newName != null && !newName.isEmpty(),
+        "Cannot use empty or null partition name: %s", newName);
+    Preconditions.checkArgument(!curSpecFields.containsKey(newName),
+        "Cannot use partition name more than once: %s", newName);
+
+    PartitionField field = curSpecFields.remove(name);
+    curSpecFields.put(newName,
+        new PartitionField(field.sourceId(), field.fieldId(), newName, field.transform()));
     return this;
   }
 
   @Override
   public UpdatePartitionSpec removeField(String name) {
-    newSpecBuilder.remove(name, base.formatVersion() == 1);
+    Preconditions.checkArgument(curSpecFields.containsKey(name),
+        "Cannot find an existing partition field with the name: %s", name);
+    if (base.formatVersion() == 1) {
+      PartitionField field = curSpecFields.remove(name);
+      String newName = field.name() + "_removed"; // rename it for soft delete
+      curSpecFields.put(newName,
+          new PartitionField(field.sourceId(), field.fieldId(), newName, Transforms.alwaysNull()));
+    } else {
+      curSpecFields.remove(name);
+    }
     return this;
   }
 
-  @Override
-  public UpdatePartitionSpec replaceField(String name, String transform) {
-    int sourceId = newSpecBuilder.remove(name, base.formatVersion() == 1);
-    return addField(sourceId, name, transform);
-  }
-
-  @Override
-  public UpdatePartitionSpec replaceField(String name, Transform<?, ?> transform) {
-    return replaceField(name, transform.toString());
-  }
-
   @Override
   public void commit() {
     TableMetadata update = base.updatePartitionSpec(freshSpecFieldIds(apply()));
@@ -184,7 +203,7 @@ private PartitionSpec freshSpecFieldIds(PartitionSpec partitionSpec) {
     }
 
     int lastAssignedFieldId = 0;
-    Map<String, Integer> partitionFieldIdByName = Maps.newHashMap();
+    Map<String, Integer> partitionFieldIdByName = new HashMap<>();
     for (PartitionSpec spec : base.specs()) {
       for (PartitionField field : spec.fields()) {
         partitionFieldIdByName.put(getKey(field), field.fieldId());
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index ab45ab06f947..cf9b3e7fdc16 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -19,8 +19,6 @@
 
 package org.apache.iceberg;
 
-import org.apache.iceberg.transforms.Transforms;
-import org.apache.iceberg.types.Types;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -105,7 +103,7 @@ public void testAddSamePartitionField() {
 
     if (formatVersion == 1) {
       table.updateSpec()
-          .addField(2, "data_partition", "bucket[16]")
+          .addBucketField("data", 16, "data_partition")
           .commit();
       Assert.assertEquals("[\n" +
           "  1000: data_bucket: bucket[16](2)\n" +
@@ -117,7 +115,7 @@ public void testAddSamePartitionField() {
           IllegalArgumentException.class,
           "Field Id 1000 has already been used in the existing partition fields",
           () -> table.updateSpec()
-              .addField(2, "data_partition", "bucket[16]")
+              .addBucketField("data", 16, "data_partition")
               .commit());
     }
   }
@@ -130,9 +128,9 @@ public void testAddField() {
     Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
 
     table.updateSpec()
-        .addField(2, "data_partition", "bucket[8]")
+        .addBucketField("data", 8, "data_partition")
         .addBucketField("id", 8)
-        .addField(2, "data_field", Transforms.bucket(Types.StringType.get(), 6))
+        .addBucketField("data", 6, "data_field")
         .commit();
 
     Assert.assertEquals("[\n" +
@@ -229,46 +227,11 @@ public void testRemoveFieldException() {
 
     AssertHelpers.assertThrows(
         "Should throw IllegalStateException if removing a non-existing partition field",
-        IllegalStateException.class,
+        IllegalArgumentException.class,
         "Cannot find an existing partition field with the name: not_existing",
         () -> table.updateSpec()
             .removeField("not_existing")
             .commit());
   }
 
-  @Test
-  public void testReplaceField() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(0, table.spec().specId());
-
-    table.updateSpec()
-        .replaceField("data_bucket", "bucket[8]")
-        .addBucketField("id", 8)
-        .commit();
-
-    Assert.assertEquals("[\n" +
-        this.expectedSpecs[0] +
-        "  1001: data_bucket: bucket[8](2)\n" +
-        "  1002: id_bucket: bucket[8](1)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(1, table.spec().specId());
-
-    table.updateSpec()
-        .replaceField("id_bucket", Transforms.bucket(Types.StringType.get(), 16))
-        .commit();
-
-    Assert.assertEquals("[\n" +
-        this.expectedSpecs[0] +
-        "  1001: data_bucket: bucket[8](2)\n" +
-        this.expectedSpecs[1] +
-        "  1003: id_bucket: bucket[16](1)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1003, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(2, table.spec().specId());
-  }
-
 }

From be507305df9df699fc3b2d0e452c875bed1f50f9 Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Sun, 14 Jun 2020 15:27:24 -0700
Subject: [PATCH 09/11] add and fix unit tests

---
 .../org/apache/iceberg/TestManifestReader.java |  4 ++--
 .../iceberg/TestPartitionSpecUpdate.java       | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/core/src/test/java/org/apache/iceberg/TestManifestReader.java b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
index 2e1e4b945eef..2120d1403fe8 100644
--- a/core/src/test/java/org/apache/iceberg/TestManifestReader.java
+++ b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
@@ -115,8 +115,8 @@ public void testManifestReaderWithPartitionMetadataEvolution() throws IOExceptio
         .commit();
 
     ManifestFile manifest = writeManifest(1000L, manifestEntry(Status.EXISTING, 123L, FILE_A));
-    try (ManifestReader reader = ManifestFiles.read(manifest, FILE_IO)) {
-      ManifestEntry entry = Iterables.getOnlyElement(reader.entries());
+    try (ManifestReader<DataFile> reader = ManifestFiles.read(manifest, FILE_IO)) {
+      ManifestEntry<DataFile> entry  = Iterables.getOnlyElement(reader.entries());
       Assert.assertEquals(123L, (long) entry.snapshotId());
 
       List<Types.NestedField> fields = ((PartitionData) entry.file().partition()).getPartitionType().fields();
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index cf9b3e7fdc16..88a1295d270e 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -194,6 +194,15 @@ public void testRenameFieldExceptions() {
         () -> table.updateSpec()
             .renameField("data_bucket", null)
             .commit());
+
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if renaming a not committed new partition field",
+        IllegalArgumentException.class,
+        "Cannot find an existing partition field with the name: not_committed",
+        () -> table.updateSpec()
+            .addBucketField("data", 6, "not_committed")
+            .renameField("not_committed", "data_partition")
+            .commit());
   }
 
   @Test
@@ -232,6 +241,15 @@ public void testRemoveFieldException() {
         () -> table.updateSpec()
             .removeField("not_existing")
             .commit());
+
+    AssertHelpers.assertThrows(
+        "Should throw IllegalStateException if removing a not committed new partition field",
+        IllegalArgumentException.class,
+        "Cannot find an existing partition field with the name: not_committed",
+        () -> table.updateSpec()
+            .addBucketField("data", 6, "not_committed")
+            .removeField("not_committed")
+            .commit());
   }
 
 }

From 4fcfe38be0383d8b1b93850daa4c616b8c2f4560 Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Sat, 27 Jun 2020 14:01:34 -0700
Subject: [PATCH 10/11] Address the feedback and improve the implementation

---
 .../org/apache/iceberg/PartitionSpec.java     |  61 ++-
 .../apache/iceberg/UpdatePartitionSpec.java   |   9 -
 .../iceberg/TestPartitionSpecValidation.java  |  44 ++-
 .../apache/iceberg/PartitionSpecUpdate.java   | 169 ++++++---
 .../org/apache/iceberg/TableMetadata.java     |  13 +-
 .../apache/iceberg/TestManifestReader.java    |  20 +-
 .../org/apache/iceberg/TestMergeAppend.java   |  45 +--
 .../apache/iceberg/TestPartitionSpecInfo.java |  31 +-
 .../iceberg/TestPartitionSpecParser.java      |  59 ++-
 .../iceberg/TestPartitionSpecUpdate.java      | 349 ++++++++++--------
 .../org/apache/iceberg/TestTableMetadata.java |   2 +-
 .../iceberg/TestTablePartitionSpecUpdate.java | 310 ++++++++++++++++
 12 files changed, 785 insertions(+), 327 deletions(-)
 create mode 100644 core/src/test/java/org/apache/iceberg/TestTablePartitionSpecUpdate.java

diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
index 9f4650aef568..010e0b337a75 100644
--- a/api/src/main/java/org/apache/iceberg/PartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
@@ -49,7 +49,7 @@
  */
 public class PartitionSpec implements Serializable {
   // IDs for partition fields start at 1000
-  private static final int PARTITION_DATA_ID_START = 1000;
+  static final int PARTITION_DATA_ID_START = 1000;
 
   private final Schema schema;
 
@@ -311,7 +311,7 @@ public static class Builder {
     private final Schema schema;
     private final List<PartitionField> fields = Lists.newArrayList();
     private final Set<String> partitionNames = Sets.newHashSet();
-    private Map<Integer, PartitionField> timeFields = Maps.newHashMap();
+    private Map<String, PartitionField> partitionFields = Maps.newHashMap();
     private int specId = 0;
     private final AtomicInteger lastAssignedFieldId = new AtomicInteger(PARTITION_DATA_ID_START - 1);
 
@@ -346,17 +346,34 @@ private void checkAndAddPartitionName(String name, Integer identitySourceColumnI
       partitionNames.add(name);
     }
 
+    private String getDedupKey(PartitionField field) {
+      String transform = field.transform().toString();
+      String type;
+      if ("hour".equalsIgnoreCase(transform) || "day".equalsIgnoreCase(transform) ||
+          "month".equalsIgnoreCase(transform) || "year".equalsIgnoreCase(transform)) {
+        type = "time";
+      } else if (transform.startsWith("bucket[")) {
+        type = "bucket";
+      } else {
+        return null;
+      }
+      return type + "(" + field.sourceId() + ")";
+    }
+
     private void checkForRedundantPartitions(PartitionField field) {
-      PartitionField timeField = timeFields.get(field.sourceId());
-      Preconditions.checkArgument(timeField == null,
-          "Cannot add redundant partition: %s conflicts with %s", timeField, field);
-      timeFields.put(field.sourceId(), field);
+      String dedupKey = getDedupKey(field);
+      if (dedupKey == null) {
+        return;
+      }
+      PartitionField partitionField = partitionFields.get(dedupKey);
+      Preconditions.checkArgument(partitionField == null,
+          "Cannot add redundant partition: %s conflicts with %s", partitionField, field);
+      partitionFields.put(dedupKey, field);
     }
 
     private void checkDuplicateFieldId(int fieldId) {
       Preconditions.checkArgument(fields.stream().allMatch(f -> f.fieldId() != fieldId),
-          "Field Id %s has already been used in the existing partition fields: %s.",
-          fieldId, fields);
+          "Cannot add a partition that duplicates another within %s.", fields);
     }
 
     public Builder withSpecId(int newSpecId) {
@@ -383,11 +400,11 @@ public Builder identity(String sourceName) {
     }
 
     public Builder year(String sourceName, String targetName) {
-      checkAndAddPartitionName(targetName);
       Types.NestedField sourceColumn = findSourceColumn(sourceName);
       PartitionField field = new PartitionField(
           sourceColumn.fieldId(), nextFieldId(), targetName, Transforms.year(sourceColumn.type()));
       checkForRedundantPartitions(field);
+      checkAndAddPartitionName(targetName);
       fields.add(field);
       return this;
     }
@@ -397,11 +414,11 @@ public Builder year(String sourceName) {
     }
 
     public Builder month(String sourceName, String targetName) {
-      checkAndAddPartitionName(targetName);
       Types.NestedField sourceColumn = findSourceColumn(sourceName);
       PartitionField field = new PartitionField(
           sourceColumn.fieldId(), nextFieldId(), targetName, Transforms.month(sourceColumn.type()));
       checkForRedundantPartitions(field);
+      checkAndAddPartitionName(targetName);
       fields.add(field);
       return this;
     }
@@ -411,11 +428,11 @@ public Builder month(String sourceName) {
     }
 
     public Builder day(String sourceName, String targetName) {
-      checkAndAddPartitionName(targetName);
       Types.NestedField sourceColumn = findSourceColumn(sourceName);
       PartitionField field = new PartitionField(
           sourceColumn.fieldId(), nextFieldId(), targetName, Transforms.day(sourceColumn.type()));
       checkForRedundantPartitions(field);
+      checkAndAddPartitionName(targetName);
       fields.add(field);
       return this;
     }
@@ -425,11 +442,11 @@ public Builder day(String sourceName) {
     }
 
     public Builder hour(String sourceName, String targetName) {
-      checkAndAddPartitionName(targetName);
       Types.NestedField sourceColumn = findSourceColumn(sourceName);
       PartitionField field = new PartitionField(
           sourceColumn.fieldId(), nextFieldId(), targetName, Transforms.hour(sourceColumn.type()));
       checkForRedundantPartitions(field);
+      checkAndAddPartitionName(targetName);
       fields.add(field);
       return this;
     }
@@ -439,10 +456,12 @@ public Builder hour(String sourceName) {
     }
 
     public Builder bucket(String sourceName, int numBuckets, String targetName) {
-      checkAndAddPartitionName(targetName);
       Types.NestedField sourceColumn = findSourceColumn(sourceName);
-      fields.add(new PartitionField(
-          sourceColumn.fieldId(), nextFieldId(), targetName, Transforms.bucket(sourceColumn.type(), numBuckets)));
+      PartitionField field = new PartitionField(
+          sourceColumn.fieldId(), nextFieldId(), targetName, Transforms.bucket(sourceColumn.type(), numBuckets));
+      checkForRedundantPartitions(field);
+      checkAndAddPartitionName(targetName);
+      fields.add(field);
       return this;
     }
 
@@ -488,6 +507,18 @@ Builder add(int sourceId, int fieldId, String name, String transform) {
       return this;
     }
 
+    Builder addAll(Iterable<PartitionField> fieldsToAdd) {
+      fieldsToAdd.forEach(field -> {
+        checkForRedundantPartitions(field);
+        add(field.sourceId(), field.fieldId(), field.name(), field.transform().toString());
+      });
+      return this;
+    }
+
+    PartitionField getLastPartitionField() {
+      return fields.get(fields.size() - 1);
+    }
+
     public PartitionSpec build() {
       PartitionSpec spec = new PartitionSpec(schema, specId, fields, lastAssignedFieldId.get());
       checkCompatibility(spec, schema);
diff --git a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
index eaa6c5ae5c3d..249e0d81e4ca 100644
--- a/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/UpdatePartitionSpec.java
@@ -29,15 +29,6 @@
  */
 public interface UpdatePartitionSpec extends PendingUpdate<PartitionSpec> {
 
-  /**
-   * Clear all partition fields in the current partition spec.
-   * <p>
-   * It will remove all existing fields.
-   *
-   * @return this for method chaining
-   */
-  UpdatePartitionSpec clear();
-
   /**
    * Add a new partition field with identity transform to the partition spec.
    * <p>
diff --git a/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java b/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
index 35e5d420080c..62771ecd9624 100644
--- a/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
+++ b/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
@@ -38,6 +38,9 @@ public class TestPartitionSpecValidation {
   public void testMultipleTimestampPartitions() {
     AssertHelpers.assertThrows("Should not allow year(ts) and year(ts)",
         IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).year("ts", "year").year("another_ts", "year").build());
+    AssertHelpers.assertThrows("Should not allow year(ts) and year(ts)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).year("ts").year("ts").build());
     AssertHelpers.assertThrows("Should not allow year(ts) and month(ts)",
         IllegalArgumentException.class, "Cannot add redundant partition",
@@ -51,6 +54,9 @@ public void testMultipleTimestampPartitions() {
 
     AssertHelpers.assertThrows("Should not allow month(ts) and month(ts)",
         IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).month("ts", "month").month("another_ts", "month").build());
+    AssertHelpers.assertThrows("Should not allow month(ts) and month(ts)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).month("ts").month("ts").build());
     AssertHelpers.assertThrows("Should not allow month(ts) and day(ts)",
         IllegalArgumentException.class, "Cannot add redundant partition",
@@ -61,6 +67,9 @@ public void testMultipleTimestampPartitions() {
 
     AssertHelpers.assertThrows("Should not allow day(ts) and day(ts)",
         IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).day("ts", "day").day("another_ts", "day").build());
+    AssertHelpers.assertThrows("Should not allow day(ts) and day(ts)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).day("ts").day("ts").build());
     AssertHelpers.assertThrows("Should not allow day(ts) and hour(ts)",
         IllegalArgumentException.class, "Cannot add redundant partition",
@@ -68,6 +77,9 @@ public void testMultipleTimestampPartitions() {
 
     AssertHelpers.assertThrows("Should not allow hour(ts) and hour(ts)",
         IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).hour("ts", "hour").hour("another_ts", "hour").build());
+    AssertHelpers.assertThrows("Should not allow hour(ts) and hour(ts)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).hour("ts").hour("ts").build());
   }
 
@@ -75,6 +87,9 @@ public void testMultipleTimestampPartitions() {
   public void testMultipleDatePartitions() {
     AssertHelpers.assertThrows("Should not allow year(d) and year(d)",
         IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).year("d", "year").year("another_d", "year").build());
+    AssertHelpers.assertThrows("Should not allow year(d) and year(d)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).year("d").year("d").build());
     AssertHelpers.assertThrows("Should not allow year(d) and month(d)",
         IllegalArgumentException.class, "Cannot add redundant partition",
@@ -85,6 +100,9 @@ public void testMultipleDatePartitions() {
 
     AssertHelpers.assertThrows("Should not allow month(d) and month(d)",
         IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).month("d", "month").month("another_d", "month").build());
+    AssertHelpers.assertThrows("Should not allow month(d) and month(d)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).month("d").month("d").build());
     AssertHelpers.assertThrows("Should not allow month(d) and day(d)",
         IllegalArgumentException.class, "Cannot add redundant partition",
@@ -92,7 +110,11 @@ public void testMultipleDatePartitions() {
 
     AssertHelpers.assertThrows("Should not allow day(d) and day(d)",
         IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).day("d", "day").day("another_d", "day").build());
+    AssertHelpers.assertThrows("Should not allow day(d) and day(d)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).day("d").day("d").build());
+
   }
 
   @Test
@@ -255,11 +277,31 @@ public void testAddPartitionFieldsWithAndWithoutFieldIds() {
   public void testAddPartitionFieldsWithInvalidFieldId() {
     AssertHelpers.assertThrows("Should detect invalid duplicate field id",
         IllegalArgumentException.class,
-        "Field Id 1005 has already been used in the existing partition fields",
+        "Cannot add a partition that duplicates another within",
         () -> PartitionSpec.builderFor(SCHEMA)
             .add(1, "id_partition2", "bucket[5]")
             .add(1, 1005, "id_partition1", "bucket[4]")
             .add(1, 1005, "id_partition3", "bucket[3]")
             .build());
   }
+
+  @Test
+  public void testMultipleBucketPartitions() {
+    AssertHelpers.assertThrows("Should not allow bucket[8](id) and bucket[16](id)",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA)
+            .bucket("id", 8, "bucket")
+            .bucket("s", 16, "bucket").build());
+
+    AssertHelpers.assertThrows("Should not allow bucket[8](id) and bucket[16](id)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
+        () -> PartitionSpec.builderFor(SCHEMA).bucket("id", 8).bucket("id", 16).build());
+
+    AssertHelpers.assertThrows("Should not allow bucket[8](id) and bucket[16](id)",
+        IllegalArgumentException.class, "Cannot add redundant partition",
+        () -> PartitionSpec.builderFor(SCHEMA)
+            .bucket("id", 8, "id_bucket1")
+            .bucket("id", 16, "id_bucket2").build());
+
+  }
 }
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
index dcf4fe9ccef0..36c30d9a2eae 100644
--- a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
@@ -19,16 +19,20 @@
 
 package org.apache.iceberg;
 
-
-import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting;
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
 import org.apache.iceberg.transforms.Transforms;
 
 /**
@@ -36,45 +40,63 @@
  */
 class PartitionSpecUpdate implements UpdatePartitionSpec {
 
+  private static final String SOFT_DELETE_POSTFIX = "__[removed]";
+
   private final TableMetadata base;
   private final TableOperations ops;
-  private final List<Consumer<PartitionSpec.Builder>> newSpecFields = new ArrayList<>();
+  private final List<PartitionSpec> specs;
+  private final Schema schema;
   private final Map<String, PartitionField> curSpecFields;
+  private final List<Consumer<PartitionSpec.Builder>> newSpecFields = Lists.newArrayList();
+  private final Map<String, PartitionField> newRemovedFields = Maps.newHashMap();
 
   PartitionSpecUpdate(TableOperations ops) {
     this.ops = ops;
     this.base = ops.current();
-    this.curSpecFields = base.spec().fields().stream().collect(
+    this.specs = ImmutableList.<PartitionSpec>builder().addAll(base.specs()).add(base.spec()).build();
+    this.schema = base.schema();
+    this.curSpecFields = base.spec().fields().stream().filter(PartitionSpecUpdate::notSoftDeleted).collect(
         Collectors.toMap(
             PartitionField::name,
-            Function.identity(),
-            (n1, n2) -> {
-              throw new IllegalStateException(String.format("Duplicate partition field found: %s", n1));
-            },
-            LinkedHashMap::new
+            Function.identity()
         )
     );
   }
 
+  /**
+   * For testing only.
+   */
+  @VisibleForTesting
+  PartitionSpecUpdate(List<PartitionSpec> partitionSpecs) {
+    this.ops = null;
+    this.base = null;
+    this.specs = partitionSpecs;
+    this.schema = partitionSpecs.get(partitionSpecs.size() - 1).schema();
+    this.curSpecFields = partitionSpecs.get(partitionSpecs.size() - 1).fields().stream()
+        .filter(PartitionSpecUpdate::notSoftDeleted).collect(
+            Collectors.toMap(
+                PartitionField::name,
+                Function.identity()));
+  }
+
   @Override
-  public PartitionSpec apply() {
-    PartitionSpec.Builder specBuilder = PartitionSpec.builderFor(base.schema());
-    curSpecFields.values().forEach(field ->
-        specBuilder.add(
-            field.sourceId(),
-            field.fieldId(),
-            field.name(),
-            field.transform().toString())
-    );
-    newSpecFields.forEach(c -> c.accept(specBuilder));
-    return specBuilder.build();
+  public void commit() {
+    PartitionSpec newSpec = apply(); // V2 ready partition spec
+    if (base.formatVersion() == 1) {
+      newSpec = fillGapsByNullFields(newSpec);
+    }
+    TableMetadata update = base.updatePartitionSpec(newSpec);
+    ops.commit(base, update);
   }
 
   @Override
-  public UpdatePartitionSpec clear() {
-    newSpecFields.clear();
-    curSpecFields.clear();
-    return this;
+  public PartitionSpec apply() {
+    PartitionSpec.Builder specBuilder = PartitionSpec.builderFor(schema).addAll(curSpecFields.values());
+    newSpecFields.forEach(c -> {
+      c.accept(specBuilder);
+      checkIfRemoved(specBuilder.getLastPartitionField());
+    });
+    return freshSpecFieldIds(specBuilder.build());
   }
 
   @Override
@@ -166,12 +188,10 @@ public UpdatePartitionSpec renameField(String name, String newName) {
     Preconditions.checkArgument(curSpecFields.containsKey(name),
         "Cannot find an existing partition field with the name: %s", name);
     Preconditions.checkArgument(newName != null && !newName.isEmpty(),
-        "Cannot use empty or null partition name: %s", newName);
-    Preconditions.checkArgument(!curSpecFields.containsKey(newName),
-        "Cannot use partition name more than once: %s", newName);
+        "Cannot use an empty or null partition name: %s", newName);
 
-    PartitionField field = curSpecFields.remove(name);
-    curSpecFields.put(newName,
+    PartitionField field = curSpecFields.get(name);
+    curSpecFields.put(name,
         new PartitionField(field.sourceId(), field.fieldId(), newName, field.transform()));
     return this;
   }
@@ -180,54 +200,85 @@ public UpdatePartitionSpec renameField(String name, String newName) {
   public UpdatePartitionSpec removeField(String name) {
     Preconditions.checkArgument(curSpecFields.containsKey(name),
         "Cannot find an existing partition field with the name: %s", name);
-    if (base.formatVersion() == 1) {
-      PartitionField field = curSpecFields.remove(name);
-      String newName = field.name() + "_removed"; // rename it for soft delete
-      curSpecFields.put(newName,
-          new PartitionField(field.sourceId(), field.fieldId(), newName, Transforms.alwaysNull()));
-    } else {
-      curSpecFields.remove(name);
-    }
+    PartitionField field = curSpecFields.remove(name);
+    newRemovedFields.put(getKey(field), field);
     return this;
   }
 
-  @Override
-  public void commit() {
-    TableMetadata update = base.updatePartitionSpec(freshSpecFieldIds(apply()));
-    ops.commit(base, update);
+  private static boolean notSoftDeleted(PartitionField field) {
+    return !(field.name().endsWith(SOFT_DELETE_POSTFIX) && Transforms.alwaysNull().equals(field.transform()));
   }
 
-  private PartitionSpec freshSpecFieldIds(PartitionSpec partitionSpec) {
-    if (base.formatVersion() == 1) {
-      return partitionSpec;
-    }
+  private static String getKey(PartitionField field) {
+    return field.transform() + "(" + field.sourceId() + ")";
+  }
+
+  private void checkIfRemoved(PartitionField addedField) {
+    String key = getKey(addedField);
+    Preconditions.checkArgument(!newRemovedFields.containsKey(key),
+        "Cannot add a partition field (%s) because it is compatible with a previously removed field: %s",
+        addedField, newRemovedFields.get(key));
+  }
 
+  private PartitionSpec freshSpecFieldIds(PartitionSpec partitionSpec) {
     int lastAssignedFieldId = 0;
-    Map<String, Integer> partitionFieldIdByName = new HashMap<>();
-    for (PartitionSpec spec : base.specs()) {
+    Map<String, Integer> partitionFieldIdByKey = Maps.newHashMap();
+    for (PartitionSpec spec : specs) {
       for (PartitionField field : spec.fields()) {
-        partitionFieldIdByName.put(getKey(field), field.fieldId());
+        if (notSoftDeleted(field)) {
+          partitionFieldIdByKey.put(getKey(field), field.fieldId());
+        }
       }
       lastAssignedFieldId = Math.max(lastAssignedFieldId, spec.lastAssignedFieldId());
     }
 
-    PartitionSpec.Builder specBuilder = PartitionSpec.builderFor(base.schema())
-        .withSpecId(partitionSpec.specId());
-
+    List<PartitionField> partitionFields = Lists.newArrayList();
     for (PartitionField field : partitionSpec.fields()) {
-      int assignedFieldId = partitionFieldIdByName.containsKey(getKey(field)) ?
-          partitionFieldIdByName.get(getKey(field)) : ++lastAssignedFieldId;
+      String key = getKey(field);
+      if (!partitionFieldIdByKey.containsKey(key)) {
+        lastAssignedFieldId++;
+      }
+      int assignedFieldId = partitionFieldIdByKey.getOrDefault(key, lastAssignedFieldId);
 
-      specBuilder.add(
+      partitionFields.add(new PartitionField(
           field.sourceId(),
           assignedFieldId,
           field.name(),
-          field.transform().toString());
+          field.transform()));
     }
-    return specBuilder.build();
+    partitionFields.sort(Comparator.comparingInt(PartitionField::fieldId));
+
+    return PartitionSpec.builderFor(schema).withSpecId(partitionSpec.specId()).addAll(partitionFields).build();
   }
 
-  private static String getKey(PartitionField field) {
-    return field.transform() + "(" + field.sourceId() + ")";
+  private PartitionSpec fillGapsByNullFields(PartitionSpec partitionSpec) {
+    Map<Integer, Integer> sourceIdByFieldId = specs.stream().flatMap(spec -> spec.fields().stream()).collect(
+        Collectors.toMap(
+            PartitionField::fieldId,
+            PartitionField::sourceId,
+            (n1, n2) -> n2,
+            HashMap::new
+        ));
+
+    int startId = PartitionSpec.PARTITION_DATA_ID_START;
+    PartitionField[] partitionFields = new PartitionField[partitionSpec.lastAssignedFieldId() - startId + 1];
+    partitionSpec.fields().forEach(field -> partitionFields[field.fieldId() - startId] = field);
+    for (int i = 0; i < partitionFields.length; ++i) {
+      if (partitionFields[i] == null) {
+        int fieldId = startId + i;
+        ValidationException.check(sourceIdByFieldId.containsKey(fieldId),
+            "Invalid partition specs, which miss partition field info for id %s.", fieldId);
+        partitionFields[i] = new PartitionField(
+            sourceIdByFieldId.get(fieldId),
+            fieldId,
+            fieldId + SOFT_DELETE_POSTFIX,
+            Transforms.alwaysNull());
+      }
+    }
+
+    return PartitionSpec.builderFor(schema).withSpecId(partitionSpec.specId())
+        .addAll(Arrays.asList(partitionFields))
+        .build();
   }
+
 }
diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java
index b7fb7cb5a3d3..d3014ca31ed7 100644
--- a/core/src/main/java/org/apache/iceberg/TableMetadata.java
+++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java
@@ -429,11 +429,16 @@ public TableMetadata updatePartitionSpec(PartitionSpec newPartitionSpec) {
       }
     }
 
-    Preconditions.checkArgument(defaultSpecId != newDefaultSpecId,
-        "Cannot set default partition spec to the current default");
+    // Always setting default partition spec to the new partition spec
+    ImmutableList.Builder<PartitionSpec> builder = ImmutableList.builder();
+    for (PartitionSpec spec : specs) {
+      if (spec.specId() == newDefaultSpecId) {
+        builder.add(freshSpec(newDefaultSpecId, schema, newPartitionSpec));
+      } else {
+        builder.add(spec);
+      }
+    }
 
-    ImmutableList.Builder<PartitionSpec> builder = ImmutableList.<PartitionSpec>builder()
-        .addAll(specs);
     if (!specsById.containsKey(newDefaultSpecId)) {
       // get a fresh spec to ensure the spec ID is set to the new default
       builder.add(freshSpec(newDefaultSpecId, schema, newPartitionSpec));
diff --git a/core/src/test/java/org/apache/iceberg/TestManifestReader.java b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
index 2120d1403fe8..0d17e684d08d 100644
--- a/core/src/test/java/org/apache/iceberg/TestManifestReader.java
+++ b/core/src/test/java/org/apache/iceberg/TestManifestReader.java
@@ -34,16 +34,13 @@ public class TestManifestReader extends TableTestBase {
   @Parameterized.Parameters
   public static Object[][] parameters() {
     return new Object[][] {
-        new Object[] { 1, new int[]{ 1000, 1001 } },
-        new Object[] { 2, new int[]{ 1001, 1000 }},
+        new Object[] { 1 },
+        new Object[] { 2 },
     };
   }
 
-  private int[] expectedFieldIds;
-
-  public TestManifestReader(int formatVersion, int[] expectedFieldIds) {
+  public TestManifestReader(int formatVersion) {
     super(formatVersion);
-    this.expectedFieldIds = expectedFieldIds;
   }
 
   @Test
@@ -109,9 +106,8 @@ public void testManifestReaderWithUpdatedPartitionMetadata() throws IOException
 
   @Test
   public void testManifestReaderWithPartitionMetadataEvolution() throws IOException {
-    table.updateSpec().clear()
+    table.updateSpec()
         .addBucketField("id", 8)
-        .addBucketField("data", 16)
         .commit();
 
     ManifestFile manifest = writeManifest(1000L, manifestEntry(Status.EXISTING, 123L, FILE_A));
@@ -121,12 +117,12 @@ public void testManifestReaderWithPartitionMetadataEvolution() throws IOExceptio
 
       List<Types.NestedField> fields = ((PartitionData) entry.file().partition()).getPartitionType().fields();
       Assert.assertEquals(2, fields.size());
-      Assert.assertEquals(expectedFieldIds[0], fields.get(0).fieldId());
-      Assert.assertEquals("id_bucket", fields.get(0).name());
+      Assert.assertEquals(1000, fields.get(0).fieldId());
+      Assert.assertEquals("data_bucket", fields.get(0).name());
       Assert.assertEquals(Types.IntegerType.get(), fields.get(0).type());
 
-      Assert.assertEquals(expectedFieldIds[1], fields.get(1).fieldId());
-      Assert.assertEquals("data_bucket", fields.get(1).name());
+      Assert.assertEquals(1001, fields.get(1).fieldId());
+      Assert.assertEquals("id_bucket", fields.get(1).name());
       Assert.assertEquals(Types.IntegerType.get(), fields.get(1).type());
     }
   }
diff --git a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
index 6a903f60c2e2..ddfdceef415b 100644
--- a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
+++ b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
@@ -40,16 +40,13 @@ public class TestMergeAppend extends TableTestBase {
   @Parameterized.Parameters
   public static Object[][] parameters() {
     return new Object[][] {
-        new Object[] { 1, new int[]{ 1000, 1001, 1002, 1003, 1000, 1001 } },
-        new Object[] { 2, new int[]{ 1001, 1002, 1003, 1000, 1001, 1002 } },
+        new Object[] { 1 },
+        new Object[] { 2 },
     };
   }
 
-  private int[] expectedFieldIds;
-
-  public TestMergeAppend(int formatVersion, int[] expectedFieldIds) {
+  public TestMergeAppend(int formatVersion) {
     super(formatVersion);
-    this.expectedFieldIds = expectedFieldIds;
   }
 
   @Test
@@ -670,13 +667,11 @@ public void testInvalidAppendManifest() throws IOException {
 
   @Test
   public void testUpdatePartitionSpecFieldIdsWithSpecEvolution() {
-    TableMetadata base = readMetadata();
-
-    table.updateSpec().clear()
+    table.updateSpec()
+        .removeField("data_bucket")
         .addBucketField("id", 16)
         .addIdentityField("data")
         .addBucketField("data", 4)
-        .addBucketField("data", 16, "data_partition") // reuse field id although different target name
         .commit();
 
     List<PartitionSpec> partitionSpecs = table.ops().current().specs();
@@ -694,15 +689,15 @@ public void testUpdatePartitionSpecFieldIdsWithSpecEvolution() {
 
     structType = partitionSpec.partitionType();
     fields = structType.fields();
-    Assert.assertEquals(4, fields.size());
-    Assert.assertEquals("id_bucket", fields.get(0).name());
-    Assert.assertEquals(expectedFieldIds[0], fields.get(0).fieldId());
-    Assert.assertEquals("data", fields.get(1).name());
-    Assert.assertEquals(expectedFieldIds[1], fields.get(1).fieldId());
-    Assert.assertEquals("data_bucket", fields.get(2).name());
-    Assert.assertEquals(expectedFieldIds[2], fields.get(2).fieldId());
-    Assert.assertEquals("data_partition", fields.get(3).name());
-    Assert.assertEquals(expectedFieldIds[3], fields.get(3).fieldId());
+
+    int offset = 2 - formatVersion; // 1 for V1 and 0 for V2
+    Assert.assertEquals("field size should be 4 in V1 and 3 in V2", 3 + offset, fields.size());
+    V1Assert.assertEquals("removed field should be soft deleted in V1",
+        "1000: 1000__[removed]: optional string", fields.get(0).toString());
+
+    Assert.assertEquals("id bucket field", "1001: id_bucket: optional int", fields.get(0 + offset).toString());
+    Assert.assertEquals("data identity field", "1002: data: optional string", fields.get(1 + offset).toString());
+    Assert.assertEquals("data bucket field", "1003: data_bucket: optional int", fields.get(2 + offset).toString());
   }
 
   @Test
@@ -716,9 +711,8 @@ public void testManifestEntryFieldIdsForChangedPartitionSpecWithSpecEvolution()
         1, base.currentSnapshot().allManifests().size());
     ManifestFile initialManifest = base.currentSnapshot().allManifests().get(0);
 
-    table.updateSpec().clear()
+    table.updateSpec()
         .addBucketField("id", 8)
-        .addBucketField("data", 8)
         .commit();
 
     DataFile newFile = DataFiles.builder(table.spec())
@@ -738,15 +732,14 @@ public void testManifestEntryFieldIdsForChangedPartitionSpecWithSpecEvolution()
     Assert.assertEquals("Second manifest should be the initial manifest with the old spec",
         initialManifest, pending.allManifests().get(1));
 
-    // field ids of manifest entries in two manifests with different specs of the same source field should be different
     ManifestEntry<DataFile> entry = ManifestFiles.read(pending.allManifests().get(0), FILE_IO)
         .entries().iterator().next();
     Types.NestedField field = ((PartitionData) entry.file().partition()).getPartitionType().fields().get(0);
-    Assert.assertEquals(expectedFieldIds[4], field.fieldId());
-    Assert.assertEquals("id_bucket", field.name());
-    field = ((PartitionData) entry.file().partition()).getPartitionType().fields().get(1);
-    Assert.assertEquals(expectedFieldIds[5], field.fieldId());
+    Assert.assertEquals(1000, field.fieldId());
     Assert.assertEquals("data_bucket", field.name());
+    field = ((PartitionData) entry.file().partition()).getPartitionType().fields().get(1);
+    Assert.assertEquals(1001, field.fieldId());
+    Assert.assertEquals("id_bucket", field.name());
 
     entry = ManifestFiles.read(pending.allManifests().get(1), FILE_IO).entries().iterator().next();
     field = ((PartitionData) entry.file().partition()).getPartitionType().fields().get(0);
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
index 3abb2c1405a5..7515fa870483 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecInfo.java
@@ -44,21 +44,18 @@ public class TestPartitionSpecInfo {
       required(2, "data", Types.StringType.get()));
   private File tableDir = null;
 
-  private int newFieldId;
-
   @Parameterized.Parameters
   public static Object[][] parameters() {
     return new Object[][] {
-        new Object[] { 1, 1000 },
-        new Object[] { 2, 1001 },
+        new Object[] { 1 },
+        new Object[] { 2 },
     };
   }
 
   private final int formatVersion;
 
-  public TestPartitionSpecInfo(int formatVersion, int newFieldId) {
+  public TestPartitionSpecInfo(int formatVersion) {
     this.formatVersion = formatVersion;
-    this.newFieldId = newFieldId;
   }
 
   @Before
@@ -102,15 +99,25 @@ public void testSpecInfoPartitionSpecEvolution() {
 
     Assert.assertEquals(spec, table.spec());
 
-    PartitionSpec expectedNewSpec = PartitionSpec.builderFor(table.schema())
-        .add(2, newFieldId, "data_bucket", "bucket[10]")
-        .withSpecId(1)
-        .build();
-
-    table.updateSpec().clear()
+    table.updateSpec()
+        .removeField("data_bucket")
         .addBucketField("data", 10)
         .commit();
 
+    PartitionSpec expectedNewSpec;
+    if (formatVersion == 1) {
+      expectedNewSpec = PartitionSpec.builderFor(table.schema())
+          .add(2, 1000, "1000__[removed]", "void")
+          .add(2, 1001, "data_bucket", "bucket[10]")
+          .withSpecId(1)
+          .build();
+    } else {
+      expectedNewSpec = PartitionSpec.builderFor(table.schema())
+          .add(2, 1001, "data_bucket", "bucket[10]")
+          .withSpecId(1)
+          .build();
+    }
+
     Assert.assertEquals(expectedNewSpec, table.spec());
     Assert.assertEquals(expectedNewSpec, table.specs().get(expectedNewSpec.specId()));
     Assert.assertEquals(spec, table.specs().get(spec.specId()));
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
index 4502538fc9ab..6f774202e39b 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecParser.java
@@ -19,28 +19,17 @@
 
 package org.apache.iceberg;
 
+import org.apache.iceberg.types.Types;
 import org.junit.Assert;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
 
-@RunWith(Parameterized.class)
-public class TestPartitionSpecParser extends TableTestBase {
+import static org.apache.iceberg.types.Types.NestedField.required;
 
-  private int[] expectedFieldIds;
-
-  @Parameterized.Parameters
-  public static Object[][] parameters() {
-    return new Object[][] {
-        new Object[] { 1, new int[]{ 1000, 1001 } },
-        new Object[] { 2, new int[]{ 1001, 1000 } },
-    };
-  }
-
-  public TestPartitionSpecParser(int formatVersion, int[] expectedFieldIds) {
-    super(formatVersion);
-    this.expectedFieldIds = expectedFieldIds;
-  }
+public class TestPartitionSpecParser {
+  private static final Schema SCHEMA = new Schema(
+      required(1, "id", Types.IntegerType.get()),
+      required(2, "data", Types.StringType.get())
+  );
 
   @Test
   public void testToJson() {
@@ -53,28 +42,36 @@ public void testToJson() {
         "    \"field-id\" : 1000\n" +
         "  } ]\n" +
         "}";
-    Assert.assertEquals(expected, PartitionSpecParser.toJson(table.spec(), true));
+    PartitionSpec initialSpec = PartitionSpec.builderFor(SCHEMA).bucket("data", 16).build();
+    Assert.assertEquals(expected, PartitionSpecParser.toJson(initialSpec, true));
 
-    table.updateSpec().clear()
-        .addBucketField("id", 8)
-        .addBucketField("data", 16)
-        .commit();
+    PartitionSpec evolvedSpec = PartitionSpec.builderFor(SCHEMA)
+        .withSpecId(1)
+        .alwaysNull("data", "1000__[removed]")
+        .bucket("id", 8)
+        .bucket("data", 4)
+        .build();
 
     expected = "{\n" +
         "  \"spec-id\" : 1,\n" +
         "  \"fields\" : [ {\n" +
+        "    \"name\" : \"1000__[removed]\",\n" +
+        "    \"transform\" : \"void\",\n" +
+        "    \"source-id\" : 2,\n" +
+        "    \"field-id\" : 1000\n" +
+        "  }, {\n" +
         "    \"name\" : \"id_bucket\",\n" +
         "    \"transform\" : \"bucket[8]\",\n" +
         "    \"source-id\" : 1,\n" +
-        "    \"field-id\" : " + expectedFieldIds[0] +
-        "\n  }, {\n" +
+        "    \"field-id\" : 1001\n" +
+        "  }, {\n" +
         "    \"name\" : \"data_bucket\",\n" +
-        "    \"transform\" : \"bucket[16]\",\n" +
+        "    \"transform\" : \"bucket[4]\",\n" +
         "    \"source-id\" : 2,\n" +
-        "    \"field-id\" : " + expectedFieldIds[1] +
-        "\n  } ]\n" +
+        "    \"field-id\" : 1002\n" +
+        "  } ]\n" +
         "}";
-    Assert.assertEquals(expected, PartitionSpecParser.toJson(table.spec(), true));
+    Assert.assertEquals(expected, PartitionSpecParser.toJson(evolvedSpec, true));
   }
 
   @Test
@@ -94,7 +91,7 @@ public void testFromJsonWithFieldId() {
         "  } ]\n" +
         "}";
 
-    PartitionSpec spec = PartitionSpecParser.fromJson(table.schema(), specString);
+    PartitionSpec spec = PartitionSpecParser.fromJson(SCHEMA, specString);
 
     Assert.assertEquals(2, spec.fields().size());
     // should be the field ids in the JSON
@@ -117,7 +114,7 @@ public void testFromJsonWithoutFieldId() {
         "  } ]\n" +
         "}";
 
-    PartitionSpec spec = PartitionSpecParser.fromJson(table.schema(), specString);
+    PartitionSpec spec = PartitionSpecParser.fromJson(SCHEMA, specString);
 
     Assert.assertEquals(2, spec.fields().size());
     // should be the default assignment
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index 88a1295d270e..34e2060a8da6 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -19,237 +19,272 @@
 
 package org.apache.iceberg;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.iceberg.types.Types;
 import org.junit.Assert;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
 
-@RunWith(Parameterized.class)
-public class TestPartitionSpecUpdate extends TableTestBase {
+import static org.apache.iceberg.types.Types.NestedField.optional;
+import static org.apache.iceberg.types.Types.NestedField.required;
 
-  private int[] expectedFieldIds;
+public class TestPartitionSpecUpdate {
 
-  private String[] expectedSpecs;
+  private static final Schema SCHEMA = new Schema(
+      required(1, "id", Types.IntegerType.get()),
+      required(2, "data", Types.StringType.get()),
+      optional(3, "ts", Types.TimestampType.withZone())
+  );
 
-  @Parameterized.Parameters
-  public static Object[][] parameters() {
-    return new Object[][] {
-        new Object[] { 1, new int[]{ 1000, 1001, 1001, 1000, 1000 },
-            new String[]{"  1000: data_bucket_removed: void(2)\n", "  1002: id_bucket_removed: void(1)\n"} },
-        new Object[] { 2, new int[]{ 1001, 1000, 1001, 1002, 1002 }, new String[]{"", ""} },
-    };
-  }
+  List<PartitionSpec> initialSpecs = Collections.singletonList(
+      PartitionSpec.builderFor(SCHEMA).bucket("data", 16).build());
+  PartitionSpecUpdate specUpdate;
 
-  public TestPartitionSpecUpdate(int formatVersion, int[] expectedFieldIds, String[] expectedSpecs) {
-    super(formatVersion);
-    this.expectedFieldIds = expectedFieldIds;
-    this.expectedSpecs = expectedSpecs;
+  @Test
+  public void testAddAndRemoveField() {
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    Assert.assertEquals(
+        PartitionSpec.builderFor(SCHEMA)
+            .add(2, 1001, "data_bucket", "bucket[8]")
+            .build(),
+        specUpdate
+            .removeField("data_bucket")
+            .addBucketField("data", 8)
+            .apply());
+
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    Assert.assertEquals(
+        PartitionSpec.builderFor(SCHEMA)
+            .add(2, 1001, "data_bucket", "bucket[8]")
+            .build(),
+        specUpdate
+            .addBucketField("data", 8)
+            .removeField("data_bucket")
+            .apply());
   }
 
   @Test
-  public void testCommitUpdatedSpec() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
-
-    table.updateSpec().clear()
-        .addBucketField("id", 8)
-        .addBucketField("data", 16)
-        .commit();
-
-    Assert.assertEquals("[\n  " +
-        expectedFieldIds[0] + ": id_bucket: bucket[8](1)\n  " +
-        expectedFieldIds[1] + ": data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(expectedFieldIds[2], table.spec().lastAssignedFieldId());
-
-    table.updateSpec().clear()
-        .addTruncateField("data", 8)
-        .commit();
+  public void testAddRemovedFieldException() {
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if adding back a pending removed field",
+        IllegalArgumentException.class, "Cannot add a partition field",
+        () -> specUpdate
+            .removeField("data_bucket")
+            .addBucketField("id", 8)
+            .addBucketField("data", 16, "data_partition")
+            .apply());
 
-    Assert.assertEquals("[\n  " +
-        expectedFieldIds[3] + ": data_trunc: truncate[8](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(expectedFieldIds[4], table.spec().lastAssignedFieldId());
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if adding back a pending removed field",
+        IllegalArgumentException.class, "Cannot add a partition field",
+        () -> specUpdate
+            .addBucketField("id", 8)
+            .addBucketField("data", 16)
+            .removeField("data_bucket")
+            .apply());
   }
 
   @Test
   public void testUpdateException() {
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
     AssertHelpers.assertThrows(
         "Should throw IllegalArgumentException if there is an invalid partition field",
-        IllegalArgumentException.class, "Cannot use partition name more than once: id_bucket",
-        () -> table.updateSpec().clear()
+        IllegalArgumentException.class, "Cannot find an existing partition field with the name: id_bucket",
+        () -> specUpdate
+            .removeField("id_bucket")
+            .apply());
+
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if there is a redundant partition field",
+        IllegalArgumentException.class, "Cannot add redundant partition",
+        () -> specUpdate
+            .removeField("data_bucket")
             .addBucketField("id", 8)
             .addBucketField("id", 16)
-            .commit());
+            .apply());
   }
 
   @Test
   public void testAddDuplicateFieldException() {
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
     AssertHelpers.assertThrows(
         "Should throw IllegalArgumentException if adding a duplicate partition field",
         IllegalArgumentException.class, "Cannot use partition name more than once: data_bucket",
-        () -> table.updateSpec()
+        () -> specUpdate
+            .addTruncateField("data", 16, "data_bucket")
+            .apply());
+
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if adding a duplicate partition field",
+        IllegalArgumentException.class, "Cannot add redundant partition",
+        () -> specUpdate
             .addBucketField("data", 16)
-            .commit());
-  }
+            .apply());
 
-  @Test
-  public void testAddSamePartitionField() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-
-    if (formatVersion == 1) {
-      table.updateSpec()
-          .addBucketField("data", 16, "data_partition")
-          .commit();
-      Assert.assertEquals("[\n" +
-          "  1000: data_bucket: bucket[16](2)\n" +
-          "  1001: data_partition: bucket[16](2)\n" +
-          "]", table.spec().toString());
-    } else {
-      AssertHelpers.assertThrows(
-          "Should throw IllegalArgumentException if adding a duplicate partition field",
-          IllegalArgumentException.class,
-          "Field Id 1000 has already been used in the existing partition fields",
-          () -> table.updateSpec()
-              .addBucketField("data", 16, "data_partition")
-              .commit());
-    }
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if adding a redundant partition field",
+        IllegalArgumentException.class,
+        "Cannot add redundant partition:",
+        () -> specUpdate
+            .addBucketField("data", 16, "data_partition")
+            .apply());
+
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if adding a redundant partition field",
+        IllegalArgumentException.class, "Cannot add redundant partition",
+        () -> specUpdate
+            .addBucketField("data", 8, "data_partition")
+            .apply());
+
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if adding a redundant partition field",
+        IllegalArgumentException.class, "Cannot add redundant partition",
+        () -> specUpdate
+            .addBucketField("id", 8, "id_partition1")
+            .addBucketField("id", 16, "id_partition2")
+            .apply());
   }
 
   @Test
   public void testAddField() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
-
-    table.updateSpec()
-        .addBucketField("data", 8, "data_partition")
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    PartitionSpec newSpec = specUpdate
         .addBucketField("id", 8)
-        .addBucketField("data", 6, "data_field")
-        .commit();
-
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "  1001: data_partition: bucket[8](2)\n" +
-        "  1002: id_bucket: bucket[8](1)\n" +
-        "  1003: data_field: bucket[6](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1003, table.spec().lastAssignedFieldId());
+        .addTruncateField("data", 8)
+        .apply();
+
+    PartitionSpec evolvedSpec = PartitionSpec.builderFor(SCHEMA)
+        .bucket("data", 16)
+        .bucket("id", 8)
+        .truncate("data", 8)
+        .build();
+
+    Assert.assertEquals("should match evolved spec", evolvedSpec, newSpec);
+    Assert.assertEquals(1002, newSpec.lastAssignedFieldId());
+
+    specUpdate = new PartitionSpecUpdate(
+        Arrays.asList(
+            PartitionSpec.builderFor(SCHEMA).bucket("data", 16).bucket("id", 8).build(),
+            PartitionSpec.builderFor(SCHEMA).bucket("data", 16).build()));
+
+    Assert.assertEquals("Should not reuse field id for a new field",
+        PartitionSpec.builderFor(SCHEMA)
+            .add(2, 1000, "data_bucket", "bucket[16]")
+            .add(2, 1002, "data", "identity")
+            .build(),
+        specUpdate
+            .addIdentityField("data")
+            .apply());
   }
 
   @Test
   public void testRenameField() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(0, table.spec().specId());
-
-    table.updateSpec()
-        .renameField("data_bucket", "data_partition")
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    PartitionSpec newSpec = specUpdate
         .addBucketField("id", 8)
-        .commit();
-
-    Assert.assertEquals("[\n" +
-        "  1000: data_partition: bucket[16](2)\n" +
-        "  1001: id_bucket: bucket[8](1)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(1, table.spec().specId());
+        .renameField("data_bucket", "data_partition")
+        .apply();
+
+    PartitionSpec evolvedSpec = PartitionSpec.builderFor(SCHEMA)
+        .bucket("data", 16, "data_partition")
+        .bucket("id", 8, "id_bucket")
+        .build();
+
+    Assert.assertEquals("should match evolved spec", evolvedSpec, newSpec);
+    Assert.assertEquals(1001, newSpec.lastAssignedFieldId());
   }
 
   @Test
   public void testRenameFieldExceptions() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(0, table.spec().specId());
-
-    AssertHelpers.assertThrows(
-        "Should throw IllegalArgumentException if only renaming a partition field",
-        IllegalArgumentException.class,
-        "Cannot set default partition spec to the current default",
-        () -> table.updateSpec()
-            .renameField("data_bucket", "data_partition")
-            .commit());
-
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
     AssertHelpers.assertThrows(
         "Should throw IllegalArgumentException if renaming a non-existing partition field",
         IllegalArgumentException.class,
         "Cannot find an existing partition field with the name: not_existing",
-        () -> table.updateSpec()
+        () -> specUpdate
             .renameField("not_existing", "data_partition")
-            .commit());
+            .apply());
 
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
     AssertHelpers.assertThrows(
         "Should throw IllegalArgumentException if renaming a partition field to null",
         IllegalArgumentException.class,
-        "Cannot use empty or null partition name: null",
-        () -> table.updateSpec()
+        "Cannot use an empty or null partition name: null",
+        () -> specUpdate
             .renameField("data_bucket", null)
-            .commit());
+            .apply());
 
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
     AssertHelpers.assertThrows(
         "Should throw IllegalArgumentException if renaming a not committed new partition field",
         IllegalArgumentException.class,
         "Cannot find an existing partition field with the name: not_committed",
-        () -> table.updateSpec()
+        () -> specUpdate
             .addBucketField("data", 6, "not_committed")
             .renameField("not_committed", "data_partition")
-            .commit());
-  }
+            .apply());
 
-  @Test
-  public void testRemoveField() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(0, table.spec().specId());
-
-    table.updateSpec()
-        .removeField("data_bucket")
-        .addBucketField("id", 8)
-        .commit();
-
-    Assert.assertEquals("[\n" +
-        this.expectedSpecs[0] +
-        "  1001: id_bucket: bucket[8](1)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(1, table.spec().specId());
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if renaming a removed field",
+        IllegalArgumentException.class,
+        "Cannot find an existing partition field with the name: data_bucket",
+        () -> specUpdate
+            .removeField("data_bucket")
+            .renameField("data_bucket", "data_partition")
+            .apply());
+
+    specUpdate = new PartitionSpecUpdate(
+        Arrays.asList(
+            PartitionSpec.builderFor(SCHEMA).bucket("data", 16).build(),
+            PartitionSpec.builderFor(SCHEMA).bucket("data", 16).bucket("id", 8).build()));
+
+    AssertHelpers.assertThrows(
+        "Should throw IllegalArgumentException if renaming a field to the same name as another field",
+        IllegalArgumentException.class,
+        "Cannot use partition name more than once",
+        () -> specUpdate
+            .renameField("data_bucket", "id_bucket")
+            .apply());
   }
 
   @Test
   public void testRemoveFieldException() {
-    Assert.assertEquals("[\n" +
-        "  1000: data_bucket: bucket[16](2)\n" +
-        "]", table.spec().toString());
-    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
-    Assert.assertEquals(0, table.spec().specId());
-
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
     AssertHelpers.assertThrows(
         "Should throw IllegalStateException if removing a non-existing partition field",
         IllegalArgumentException.class,
         "Cannot find an existing partition field with the name: not_existing",
-        () -> table.updateSpec()
+        () -> specUpdate
             .removeField("not_existing")
-            .commit());
+            .apply());
 
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
     AssertHelpers.assertThrows(
         "Should throw IllegalStateException if removing a not committed new partition field",
         IllegalArgumentException.class,
         "Cannot find an existing partition field with the name: not_committed",
-        () -> table.updateSpec()
-            .addBucketField("data", 6, "not_committed")
+        () -> specUpdate
+            .addBucketField("id", 6, "not_committed")
             .removeField("not_committed")
-            .commit());
-  }
+            .apply());
 
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    AssertHelpers.assertThrows(
+        "Should throw IllegalStateException if removing a partition field more than once",
+        IllegalArgumentException.class,
+        "Cannot find an existing partition field with the name: data_bucket",
+        () -> specUpdate
+            .removeField("data_bucket")
+            .removeField("data_bucket")
+            .apply());
+  }
 }
diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
index d93884500306..6730f826a533 100644
--- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
+++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
@@ -533,7 +533,7 @@ public void testInvalidUpdatePartitionSpecForV1Table() throws Exception {
         .add(1, 1005, "x_partition", "bucket[4]")
         .build();
     String location = "file://tmp/db/table";
-    TableMetadata metadata = TableMetadata.newTableMetadata(schema, spec, location, ImmutableMap.of());
+    TableMetadata metadata = TableMetadata.newTableMetadata(schema, spec, location, ImmutableMap.of(), 1);
 
     AssertHelpers.assertThrows("Should fail to update an invalid partition spec",
         ValidationException.class, "Spec does not use sequential IDs that are required in v1",
diff --git a/core/src/test/java/org/apache/iceberg/TestTablePartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestTablePartitionSpecUpdate.java
new file mode 100644
index 000000000000..b3ff37ef66a0
--- /dev/null
+++ b/core/src/test/java/org/apache/iceberg/TestTablePartitionSpecUpdate.java
@@ -0,0 +1,310 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class TestTablePartitionSpecUpdate extends TableTestBase {
+
+  @Parameterized.Parameters
+  public static Object[][] parameters() {
+    return new Object[][] {
+        new Object[] { 1 },
+        new Object[] { 2 },
+    };
+  }
+
+  public TestTablePartitionSpecUpdate(int formatVersion) {
+    super(formatVersion);
+  }
+
+  @Before
+  public void verifyInitialSpec() {
+    PartitionSpec initialSpec = PartitionSpec.builderFor(table.schema()).bucket("data", 16).build();
+    Assert.assertEquals("Should use the expected initial spec", initialSpec, table.spec());
+    Assert.assertEquals(1000, table.spec().lastAssignedFieldId());
+    Assert.assertEquals(0, table.spec().specId());
+  }
+
+  @Test
+  public void testCommitUpdatedSpec() {
+    table.updateSpec()
+        .addBucketField("id", 8)
+        .commit();
+
+    PartitionSpec evolvedSpec = PartitionSpec.builderFor(table.schema())
+        .withSpecId(1)
+        .bucket("data", 16)
+        .bucket("id", 8)
+        .build();
+    Assert.assertEquals("Should append a partition field to the spec", evolvedSpec, table.spec());
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .removeField("id_bucket")
+        .removeField("data_bucket")
+        .addTruncateField("data", 8)
+        .commit();
+
+    V1Assert.assertEquals("Should soft delete id and data buckets", PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .alwaysNull("data", "1000__[removed]")
+        .alwaysNull("id", "1001__[removed]")
+        .truncate("data", 8)
+        .build(), table.spec());
+
+    V2Assert.assertEquals("Should hard delete id and data buckets", PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .add(2, 1002, "data_trunc", "truncate[8]")
+        .build(), table.spec());
+
+    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+  }
+
+  @Test
+  public void testRenameField() {
+    table.updateSpec()
+        .renameField("data_bucket", "data_partition")
+        .addBucketField("id", 8)
+        .commit();
+
+    PartitionSpec evolvedSpec = PartitionSpec.builderFor(table.schema())
+        .withSpecId(1)
+        .bucket("data", 16, "data_partition")
+        .bucket("id", 8, "id_bucket")
+        .build();
+
+    Assert.assertEquals("should match evolved spec", evolvedSpec, table.spec());
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .addTruncateField("id", 4)
+        .renameField("data_partition", "data_bucket")
+        .commit();
+
+    evolvedSpec = PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .bucket("data", 16, "data_bucket")
+        .bucket("id", 8, "id_bucket")
+        .truncate("id", 4)
+        .build();
+
+    Assert.assertEquals("should match evolved spec", evolvedSpec, table.spec());
+    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .renameField("id_bucket", "id_partition")
+        .commit();
+
+    evolvedSpec = PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .bucket("data", 16, "data_bucket")
+        .bucket("id", 8, "id_partition")
+        .truncate("id", 4)
+        .build();
+    Assert.assertEquals("should match evolved spec", evolvedSpec, table.spec());
+    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+  }
+
+  @Test
+  public void testRemoveField() {
+    table.updateSpec()
+        .removeField("data_bucket")
+        .addBucketField("id", 8)
+        .commit();
+
+    V1Assert.assertEquals("Should soft delete data bucket", PartitionSpec.builderFor(table.schema())
+        .withSpecId(1)
+        .alwaysNull("data", "1000__[removed]")
+        .bucket("id", 8)
+        .build(), table.spec());
+
+    V2Assert.assertEquals("Should hard delete data bucket", PartitionSpec.builderFor(table.schema())
+        .withSpecId(1)
+        .add(1, 1001, "id_bucket", "bucket[8]")
+        .build(), table.spec());
+
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+  }
+
+  @Test
+  public void testRemoveAndAddField() {
+    table.updateSpec()
+        .removeField("data_bucket")
+        .commit();
+
+    Assert.assertEquals("Should hard delete data bucket for both V1 and V2", PartitionSpec.builderFor(table.schema())
+        .withSpecId(1)
+        .build(), table.spec());
+
+    table.updateSpec()
+        .addBucketField("data", 8)
+        .commit();
+
+    V1Assert.assertEquals("Should add a new data bucket", PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .alwaysNull("data", "1000__[removed]")
+        .bucket("data", 8)
+        .build(), table.spec());
+    V2Assert.assertEquals("Should add a new data bucket", PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .add(2, 1001, "data_bucket", "bucket[8]")
+        .build(), table.spec());
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .addBucketField("data", 6)
+        .removeField("data_bucket")
+        .commit();
+
+    V1Assert.assertEquals("Should evolve to a new spec", PartitionSpec.builderFor(table.schema())
+        .withSpecId(3)
+        .alwaysNull("data", "1000__[removed]")
+        .alwaysNull("data", "1001__[removed]")
+        .bucket("data", 6)
+        .build(), table.spec());
+    V2Assert.assertEquals("Should evolve back to the initial spec", PartitionSpec.builderFor(table.schema())
+        .withSpecId(3)
+        .add(2, 1002, "data_bucket", "bucket[6]")
+        .build(), table.spec());
+    Assert.assertEquals("Should allocate a new field id", 1002, table.spec().lastAssignedFieldId());
+  }
+
+  @Test
+  public void testRenameAndRemoveField() {
+    table.updateSpec()
+        .renameField("data_bucket", "data_partition")
+        .removeField("data_bucket")
+        .addBucketField("data", 8)
+        .commit();
+
+    V1Assert.assertEquals("Should remove the renamed field", PartitionSpec.builderFor(table.schema())
+        .withSpecId(1)
+        .alwaysNull("data", "1000__[removed]")
+        .bucket("data", 8)
+        .build(), table.spec());
+    V2Assert.assertEquals("Should remove the renamed field", PartitionSpec.builderFor(table.schema())
+        .withSpecId(1)
+        .add(2, 1001, "data_bucket", "bucket[8]")
+        .build(), table.spec());
+
+    table.updateSpec()
+        .addBucketField("data", 6)
+        .removeField("data_bucket")
+        .commit();
+
+    V1Assert.assertEquals("Should remove and then add a bucket field", PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .alwaysNull("data", "1000__[removed]")
+        .alwaysNull("data", "1001__[removed]")
+        .bucket("data", 6)
+        .build(), table.spec());
+    V2Assert.assertEquals("Should remove and then add a bucket field", PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .add(2, 1002, "data_bucket", "bucket[6]")
+        .build(), table.spec());
+    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+  }
+
+  @Test
+  public void testFieldIdEvolution() {
+    table.updateSpec()
+        .addBucketField("id", 8)
+        .commit();
+
+    PartitionSpec evolvedSpec = PartitionSpec.builderFor(table.schema())
+        .withSpecId(1)
+        .bucket("data", 16)
+        .bucket("id", 8)
+        .build();
+    Assert.assertEquals("Should append a partition field to the spec", evolvedSpec, table.spec());
+    Assert.assertEquals(1001, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .removeField("data_bucket")
+        .addBucketField("data", 8)
+        .commit();
+
+    V1Assert.assertEquals("Should add a new data bucket", PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .alwaysNull("data", "1000__[removed]")
+        .bucket("id", 8)
+        .bucket("data", 8)
+        .build(), table.spec());
+    V2Assert.assertEquals("Should add a new data bucket", PartitionSpec.builderFor(table.schema())
+        .withSpecId(2)
+        .add(1, 1001, "id_bucket", "bucket[8]")
+        .add(2, 1002, "data_bucket", "bucket[8]")
+        .build(), table.spec());
+    Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .removeField("data_bucket")
+        .addBucketField("data", 16, "data_partition")
+        .commit();
+
+    Assert.assertEquals(
+        "Should add back a removed data bucket, truncate the trailing removed fields, and reuse the spec id",
+        PartitionSpec.builderFor(table.schema())
+            .withSpecId(1)
+            .bucket("data", 16, "data_partition")
+            .bucket("id", 8)
+            .build(),
+        table.spec());
+    Assert.assertEquals("Should reuse the field id", 1001, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .removeField("id_bucket")
+        .renameField("data_partition", "data_bucket")
+        .commit();
+
+    Assert.assertEquals(
+        "Should rename an existing data bucket, truncate the trailing removed fields, and reuse the spec id",
+        PartitionSpec.builderFor(table.schema())
+            .withSpecId(0)
+            .add(2, 1000, "data_bucket", "bucket[16]")
+            .build(),
+        table.spec());
+    Assert.assertEquals("Should not add a new field id", 1000, table.spec().lastAssignedFieldId());
+
+    table.updateSpec()
+        .addTruncateField("id", 6)
+        .commit();
+
+    V1Assert.assertEquals("Should add a new data bucket and fill the gaps", PartitionSpec.builderFor(table.schema())
+        .withSpecId(3)
+        .add(2, 1000, "data_bucket", "bucket[16]")
+        .add(1, 1001, "1001__[removed]", "void")
+        .add(2, 1002, "1002__[removed]", "void")
+        .add(1, 1003, "id_trunc", "truncate[6]")
+        .build(), table.spec());
+    V2Assert.assertEquals("Should add a new data bucket", PartitionSpec.builderFor(table.schema())
+        .withSpecId(3)
+        .add(2, 1000, "data_bucket", "bucket[16]")
+        .add(1, 1003, "id_trunc", "truncate[6]")
+        .build(), table.spec());
+    Assert.assertEquals(1003, table.spec().lastAssignedFieldId());
+  }
+}

From 99373a79fbf233ba4b4bf95eec672df96f0d9f7d Mon Sep 17 00:00:00 2001
From: Jun He <jun-he@users.noreply.github.com>
Date: Sun, 12 Jul 2020 19:58:08 -0700
Subject: [PATCH 11/11] address the comments

---
 .../org/apache/iceberg/PartitionSpec.java     |  32 ++--
 .../org/apache/iceberg/transforms/Bucket.java |   5 +
 .../org/apache/iceberg/transforms/Dates.java  |   5 +
 .../apache/iceberg/transforms/Identity.java   |   5 +
 .../apache/iceberg/transforms/Timestamps.java |   5 +
 .../apache/iceberg/transforms/Transform.java  |   9 +
 .../apache/iceberg/transforms/Truncate.java   |   5 +
 .../iceberg/TestPartitionSpecValidation.java  |  71 +++++---
 .../apache/iceberg/PartitionSpecUpdate.java   | 159 ++++++++----------
 .../org/apache/iceberg/TableMetadata.java     |   5 +
 .../iceberg/TestPartitionSpecUpdate.java      |  37 ++--
 .../iceberg/TestTablePartitionSpecUpdate.java |  42 +++++
 12 files changed, 241 insertions(+), 139 deletions(-)

diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
index 010e0b337a75..3ec2735f0cc8 100644
--- a/api/src/main/java/org/apache/iceberg/PartitionSpec.java
+++ b/api/src/main/java/org/apache/iceberg/PartitionSpec.java
@@ -174,7 +174,7 @@ public String partitionToPath(StructLike data) {
   }
 
   /**
-   * Returns true if this spec is equivalent to the other, with field names and partition field ids ignored.
+   * Returns true if this spec is compatible to the other, with field names and partition field ids ignored.
    * That is, if both specs have the same number of fields, field order, source columns, and transforms.
    *
    * @param other another PartitionSpec
@@ -201,6 +201,21 @@ public boolean compatibleWith(PartitionSpec other) {
     return true;
   }
 
+  /**
+   * Returns true if this spec is equivalent to the other.
+   * That is, if both specs have the same number of fields, field order, and partition fields.
+   *
+   * @param other another PartitionSpec
+   * @return true if the specs have the same partition fields.
+   */
+  boolean equivalentTo(PartitionSpec other) {
+    if (equals(other)) {
+      return true;
+    }
+    return Arrays.equals(fields, other.fields);
+  }
+
+
   @Override
   public boolean equals(Object other) {
     if (this == other) {
@@ -347,17 +362,12 @@ private void checkAndAddPartitionName(String name, Integer identitySourceColumnI
     }
 
     private String getDedupKey(PartitionField field) {
-      String transform = field.transform().toString();
-      String type;
-      if ("hour".equalsIgnoreCase(transform) || "day".equalsIgnoreCase(transform) ||
-          "month".equalsIgnoreCase(transform) || "year".equalsIgnoreCase(transform)) {
-        type = "time";
-      } else if (transform.startsWith("bucket[")) {
-        type = "bucket";
+      String transformName = field.transform().getName();
+      if (transformName != null) {
+        return transformName + "(" + field.sourceId() + ")";
       } else {
         return null;
       }
-      return type + "(" + field.sourceId() + ")";
     }
 
     private void checkForRedundantPartitions(PartitionField field) {
@@ -515,10 +525,6 @@ Builder addAll(Iterable<PartitionField> fieldsToAdd) {
       return this;
     }
 
-    PartitionField getLastPartitionField() {
-      return fields.get(fields.size() - 1);
-    }
-
     public PartitionSpec build() {
       PartitionSpec spec = new PartitionSpec(schema, specId, fields, lastAssignedFieldId.get());
       checkCompatibility(spec, schema);
diff --git a/api/src/main/java/org/apache/iceberg/transforms/Bucket.java b/api/src/main/java/org/apache/iceberg/transforms/Bucket.java
index 9ab1fd44a5c9..0458b355c098 100644
--- a/api/src/main/java/org/apache/iceberg/transforms/Bucket.java
+++ b/api/src/main/java/org/apache/iceberg/transforms/Bucket.java
@@ -154,6 +154,11 @@ public Type getResultType(Type sourceType) {
     return Types.IntegerType.get();
   }
 
+  @Override
+  public String getName() {
+    return "bucket";
+  }
+
   private static class BucketInteger extends Bucket<Integer> {
     private BucketInteger(int numBuckets) {
       super(numBuckets);
diff --git a/api/src/main/java/org/apache/iceberg/transforms/Dates.java b/api/src/main/java/org/apache/iceberg/transforms/Dates.java
index d1a1583f6f36..6b389014e307 100644
--- a/api/src/main/java/org/apache/iceberg/transforms/Dates.java
+++ b/api/src/main/java/org/apache/iceberg/transforms/Dates.java
@@ -125,4 +125,9 @@ public String toHumanString(Integer value) {
   public String toString() {
     return name;
   }
+
+  @Override
+  public String getName() {
+    return "date";
+  }
 }
diff --git a/api/src/main/java/org/apache/iceberg/transforms/Identity.java b/api/src/main/java/org/apache/iceberg/transforms/Identity.java
index e53a8f115f9c..53fa1069d87e 100644
--- a/api/src/main/java/org/apache/iceberg/transforms/Identity.java
+++ b/api/src/main/java/org/apache/iceberg/transforms/Identity.java
@@ -128,4 +128,9 @@ public boolean equals(Object o) {
   public int hashCode() {
     return Objects.hashCode(type);
   }
+
+  @Override
+  public String getName() {
+    return "identity";
+  }
 }
diff --git a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java
index 97086211da83..ed3d771e106b 100644
--- a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java
+++ b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java
@@ -129,4 +129,9 @@ public String toHumanString(Integer value) {
   public String toString() {
     return name;
   }
+
+  @Override
+  public String getName() {
+    return "timestamp";
+  }
 }
diff --git a/api/src/main/java/org/apache/iceberg/transforms/Transform.java b/api/src/main/java/org/apache/iceberg/transforms/Transform.java
index ba0fb1b219df..d4f5169c78ba 100644
--- a/api/src/main/java/org/apache/iceberg/transforms/Transform.java
+++ b/api/src/main/java/org/apache/iceberg/transforms/Transform.java
@@ -102,4 +102,13 @@ default boolean isIdentity() {
   default String toHumanString(T value) {
     return String.valueOf(value);
   }
+
+  /**
+   * Returns the transform name
+   *
+   * @return the transform name in String format
+   */
+  default String getName() {
+    return null;
+  }
 }
diff --git a/api/src/main/java/org/apache/iceberg/transforms/Truncate.java b/api/src/main/java/org/apache/iceberg/transforms/Truncate.java
index 39120b372ccc..28b4668e7465 100644
--- a/api/src/main/java/org/apache/iceberg/transforms/Truncate.java
+++ b/api/src/main/java/org/apache/iceberg/transforms/Truncate.java
@@ -63,6 +63,11 @@ public Type getResultType(Type sourceType) {
     return sourceType;
   }
 
+  @Override
+  public String getName() {
+    return "truncate";
+  }
+
   private static class TruncateInteger extends Truncate<Integer> {
     private final int width;
 
diff --git a/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java b/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
index 62771ecd9624..30ff5732ed2f 100644
--- a/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
+++ b/api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
@@ -35,10 +35,51 @@ public class TestPartitionSpecValidation {
   );
 
   @Test
-  public void testMultipleTimestampPartitions() {
-    AssertHelpers.assertThrows("Should not allow year(ts) and year(ts)",
+  public void testPartitionNameCollisions() {
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
         IllegalArgumentException.class, "Cannot use partition name more than once",
         () -> PartitionSpec.builderFor(SCHEMA).year("ts", "year").year("another_ts", "year").build());
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).month("ts", "month").month("another_ts", "month").build());
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).day("ts", "day").day("another_ts", "day").build());
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name)",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).hour("ts", "hour").hour("another_ts", "hour").build());
+
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).year("d", "year").year("another_d", "year").build());
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).month("d", "month").month("another_d", "month").build());
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA).day("d", "day").day("another_d", "day").build());
+
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA)
+            .bucket("id", 8, "bucket")
+            .bucket("s", 16, "bucket").build());
+
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA)
+            .truncate("id", 8, "bucket")
+            .truncate("s", 16, "bucket").build());
+
+    AssertHelpers.assertThrows("Should not allow partition fields with the same name",
+        IllegalArgumentException.class, "Cannot use partition name more than once",
+        () -> PartitionSpec.builderFor(SCHEMA)
+            .identity("id", "identity")
+            .identity("s", "identity").build());
+  }
+
+  @Test
+  public void testMultipleTimestampPartitions() {
     AssertHelpers.assertThrows("Should not allow year(ts) and year(ts)",
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).year("ts").year("ts").build());
@@ -52,9 +93,6 @@ public void testMultipleTimestampPartitions() {
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).year("ts").hour("ts").build());
 
-    AssertHelpers.assertThrows("Should not allow month(ts) and month(ts)",
-        IllegalArgumentException.class, "Cannot use partition name more than once",
-        () -> PartitionSpec.builderFor(SCHEMA).month("ts", "month").month("another_ts", "month").build());
     AssertHelpers.assertThrows("Should not allow month(ts) and month(ts)",
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).month("ts").month("ts").build());
@@ -65,9 +103,6 @@ public void testMultipleTimestampPartitions() {
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).month("ts").hour("ts").build());
 
-    AssertHelpers.assertThrows("Should not allow day(ts) and day(ts)",
-        IllegalArgumentException.class, "Cannot use partition name more than once",
-        () -> PartitionSpec.builderFor(SCHEMA).day("ts", "day").day("another_ts", "day").build());
     AssertHelpers.assertThrows("Should not allow day(ts) and day(ts)",
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).day("ts").day("ts").build());
@@ -75,9 +110,6 @@ public void testMultipleTimestampPartitions() {
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).day("ts").hour("ts").build());
 
-    AssertHelpers.assertThrows("Should not allow hour(ts) and hour(ts)",
-        IllegalArgumentException.class, "Cannot use partition name more than once",
-        () -> PartitionSpec.builderFor(SCHEMA).hour("ts", "hour").hour("another_ts", "hour").build());
     AssertHelpers.assertThrows("Should not allow hour(ts) and hour(ts)",
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).hour("ts").hour("ts").build());
@@ -85,9 +117,6 @@ public void testMultipleTimestampPartitions() {
 
   @Test
   public void testMultipleDatePartitions() {
-    AssertHelpers.assertThrows("Should not allow year(d) and year(d)",
-        IllegalArgumentException.class, "Cannot use partition name more than once",
-        () -> PartitionSpec.builderFor(SCHEMA).year("d", "year").year("another_d", "year").build());
     AssertHelpers.assertThrows("Should not allow year(d) and year(d)",
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).year("d").year("d").build());
@@ -98,9 +127,6 @@ public void testMultipleDatePartitions() {
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).year("d").day("d").build());
 
-    AssertHelpers.assertThrows("Should not allow month(d) and month(d)",
-        IllegalArgumentException.class, "Cannot use partition name more than once",
-        () -> PartitionSpec.builderFor(SCHEMA).month("d", "month").month("another_d", "month").build());
     AssertHelpers.assertThrows("Should not allow month(d) and month(d)",
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).month("d").month("d").build());
@@ -108,13 +134,9 @@ public void testMultipleDatePartitions() {
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).month("d").day("d").build());
 
-    AssertHelpers.assertThrows("Should not allow day(d) and day(d)",
-        IllegalArgumentException.class, "Cannot use partition name more than once",
-        () -> PartitionSpec.builderFor(SCHEMA).day("d", "day").day("another_d", "day").build());
     AssertHelpers.assertThrows("Should not allow day(d) and day(d)",
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).day("d").day("d").build());
-
   }
 
   @Test
@@ -287,12 +309,6 @@ public void testAddPartitionFieldsWithInvalidFieldId() {
 
   @Test
   public void testMultipleBucketPartitions() {
-    AssertHelpers.assertThrows("Should not allow bucket[8](id) and bucket[16](id)",
-        IllegalArgumentException.class, "Cannot use partition name more than once",
-        () -> PartitionSpec.builderFor(SCHEMA)
-            .bucket("id", 8, "bucket")
-            .bucket("s", 16, "bucket").build());
-
     AssertHelpers.assertThrows("Should not allow bucket[8](id) and bucket[16](id)",
         IllegalArgumentException.class, "Cannot add redundant partition",
         () -> PartitionSpec.builderFor(SCHEMA).bucket("id", 8).bucket("id", 16).build());
@@ -302,6 +318,5 @@ public void testMultipleBucketPartitions() {
         () -> PartitionSpec.builderFor(SCHEMA)
             .bucket("id", 8, "id_bucket1")
             .bucket("id", 16, "id_bucket2").build());
-
   }
 }
diff --git a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
index 36c30d9a2eae..69a525924eb3 100644
--- a/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
+++ b/core/src/main/java/org/apache/iceberg/PartitionSpecUpdate.java
@@ -24,7 +24,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.function.Consumer;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import org.apache.iceberg.exceptions.ValidationException;
@@ -33,7 +33,10 @@
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.transforms.Transform;
 import org.apache.iceberg.transforms.Transforms;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
 
 /**
  * PartitionSpec evolution API implementation.
@@ -47,20 +50,17 @@ class PartitionSpecUpdate implements UpdatePartitionSpec {
   private final List<PartitionSpec> specs;
   private final Schema schema;
   private final Map<String, PartitionField> curSpecFields;
-  private final List<Consumer<PartitionSpec.Builder>> newSpecFields = Lists.newArrayList();
-  private final Map<String, PartitionField> newRemovedFields = Maps.newHashMap();
+  private final List<PartitionField> fields = Lists.newArrayList();
+  private final AtomicInteger lastAssignedFieldId = new AtomicInteger(0);
+  private final Map<String, Integer> partitionFieldIdByKey = Maps.newHashMap();
 
   PartitionSpecUpdate(TableOperations ops) {
     this.ops = ops;
     this.base = ops.current();
     this.specs = ImmutableList.<PartitionSpec>builder().addAll(base.specs()).add(base.spec()).build();
     this.schema = base.schema();
-    this.curSpecFields = base.spec().fields().stream().filter(PartitionSpecUpdate::notSoftDeleted).collect(
-        Collectors.toMap(
-            PartitionField::name,
-            Function.identity()
-        )
-    );
+    this.curSpecFields = buildSpecByNameMap(base.spec());
+    init();
   }
 
   /**
@@ -72,115 +72,137 @@ class PartitionSpecUpdate implements UpdatePartitionSpec {
     this.base = null;
     this.specs = partitionSpecs;
     this.schema = partitionSpecs.get(partitionSpecs.size() - 1).schema();
-    this.curSpecFields = partitionSpecs.get(partitionSpecs.size() - 1).fields().stream()
+    this.curSpecFields = buildSpecByNameMap(partitionSpecs.get(partitionSpecs.size() - 1));
+    init();
+  }
+
+  private Map<String, PartitionField> buildSpecByNameMap(PartitionSpec spec) {
+    return spec.fields().stream()
         .filter(PartitionSpecUpdate::notSoftDeleted).collect(
             Collectors.toMap(
                 PartitionField::name,
                 Function.identity()));
   }
 
+  private void init() {
+    for (PartitionSpec spec : specs) {
+      for (PartitionField field : spec.fields()) {
+        if (notSoftDeleted(field)) {
+          partitionFieldIdByKey.put(getKey(field.transform(), field.sourceId()), field.fieldId());
+        }
+      }
+      lastAssignedFieldId.getAndAccumulate(spec.lastAssignedFieldId(), Math::max);
+    }
+  }
+
   @Override
   public void commit() {
     PartitionSpec newSpec = apply(); // V2 ready partition spec
     if (base.formatVersion() == 1) {
       newSpec = fillGapsByNullFields(newSpec);
     }
-    TableMetadata update = base.updatePartitionSpec(newSpec);
-    ops.commit(base, update);
+    TableMetadata updated = base.updatePartitionSpec(newSpec);
+
+    if (updated == base) {
+      // do not commit if the metadata has not changed. For example, this may happen
+      // when the committing partition spec is already current. Note that this check uses identity.
+      return;
+    }
+
+    ops.commit(base, updated);
   }
 
   @Override
   public PartitionSpec apply() {
-    PartitionSpec.Builder specBuilder = PartitionSpec.builderFor(schema).addAll(curSpecFields.values());
-    newSpecFields.forEach(c -> {
-      c.accept(specBuilder);
-      checkIfRemoved(specBuilder.getLastPartitionField());
-    });
-    return freshSpecFieldIds(specBuilder.build());
+    fields.addAll(curSpecFields.values());
+    fields.sort(Comparator.comparingInt(PartitionField::fieldId));
+    return PartitionSpec.builderFor(schema).addAll(fields).build();
+  }
+
+  private UpdatePartitionSpec addFieldWithType(String sourceName,
+                                               String targetName,
+                                               Function<Type, Transform<?, ?>> func) {
+    Types.NestedField sourceColumn = schema.findField(sourceName);
+    Preconditions.checkArgument(sourceColumn != null, "Cannot find source column: %s", sourceName);
+
+    Transform<?, ?> transform = func.apply(sourceColumn.type());
+    Integer assignedFieldId = partitionFieldIdByKey.get(getKey(transform, sourceColumn.fieldId()));
+    if (assignedFieldId == null) {
+      assignedFieldId = lastAssignedFieldId.incrementAndGet();
+    }
+
+    fields.add(new PartitionField(sourceColumn.fieldId(), assignedFieldId, targetName, transform));
+    return this;
   }
 
   @Override
   public UpdatePartitionSpec addIdentityField(String sourceName, String targetName) {
-    newSpecFields.add(builder -> builder.identity(sourceName, targetName));
-    return this;
+    return addFieldWithType(sourceName, targetName, Transforms::identity);
   }
 
   @Override
   public UpdatePartitionSpec addIdentityField(String sourceName) {
-    newSpecFields.add(builder -> builder.identity(sourceName));
-    return this;
+    return addIdentityField(sourceName, sourceName);
   }
 
   @Override
   public UpdatePartitionSpec addYearField(String sourceName, String targetName) {
-    newSpecFields.add(builder -> builder.year(sourceName, targetName));
-    return this;
+    return addFieldWithType(sourceName, targetName, Transforms::year);
   }
 
   @Override
   public UpdatePartitionSpec addYearField(String sourceName) {
-    newSpecFields.add(builder -> builder.year(sourceName));
-    return this;
+    return addYearField(sourceName, sourceName + "_year");
   }
 
   @Override
   public UpdatePartitionSpec addMonthField(String sourceName, String targetName) {
-    newSpecFields.add(builder -> builder.month(sourceName, targetName));
-    return this;
+    return addFieldWithType(sourceName, targetName, Transforms::month);
   }
 
   @Override
   public UpdatePartitionSpec addMonthField(String sourceName) {
-    newSpecFields.add(builder -> builder.month(sourceName));
-    return this;
+    return addMonthField(sourceName, sourceName + "_month");
   }
 
   @Override
   public UpdatePartitionSpec addDayField(String sourceName, String targetName) {
-    newSpecFields.add(builder -> builder.day(sourceName, targetName));
-    return this;
+    return addFieldWithType(sourceName, targetName, Transforms::day);
   }
 
   @Override
   public UpdatePartitionSpec addDayField(String sourceName) {
-    newSpecFields.add(builder -> builder.day(sourceName));
-    return this;
+    return addDayField(sourceName, sourceName + "_day");
   }
 
   @Override
   public UpdatePartitionSpec addHourField(String sourceName, String targetName) {
-    newSpecFields.add(builder -> builder.hour(sourceName, targetName));
-    return this;
+    return addFieldWithType(sourceName, targetName, Transforms::hour);
   }
 
   @Override
   public UpdatePartitionSpec addHourField(String sourceName) {
-    newSpecFields.add(builder -> builder.hour(sourceName));
-    return this;
+    return addHourField(sourceName, sourceName + "_hour");
   }
 
   @Override
   public UpdatePartitionSpec addBucketField(String sourceName, int numBuckets, String targetName) {
-    newSpecFields.add(builder -> builder.bucket(sourceName, numBuckets, targetName));
-    return this;
+    return addFieldWithType(sourceName, targetName, type -> Transforms.bucket(type, numBuckets));
   }
 
   @Override
   public UpdatePartitionSpec addBucketField(String sourceName, int numBuckets) {
-    newSpecFields.add(builder -> builder.bucket(sourceName, numBuckets));
-    return this;
+    return addBucketField(sourceName, numBuckets, sourceName + "_bucket");
   }
 
   @Override
   public UpdatePartitionSpec addTruncateField(String sourceName, int width, String targetName) {
-    newSpecFields.add(builder -> builder.truncate(sourceName, width, targetName));
-    return this;
+    return addFieldWithType(sourceName, targetName, type -> Transforms.truncate(type, width));
   }
 
   @Override
   public UpdatePartitionSpec addTruncateField(String sourceName, int width) {
-    newSpecFields.add(builder -> builder.truncate(sourceName, width));
-    return this;
+    return addTruncateField(sourceName, width, sourceName + "_trunc");
   }
 
   @Override
@@ -200,8 +222,7 @@ public UpdatePartitionSpec renameField(String name, String newName) {
   public UpdatePartitionSpec removeField(String name) {
     Preconditions.checkArgument(curSpecFields.containsKey(name),
         "Cannot find an existing partition field with the name: %s", name);
-    PartitionField field = curSpecFields.remove(name);
-    newRemovedFields.put(getKey(field), field);
+    curSpecFields.remove(name);
     return this;
   }
 
@@ -209,46 +230,8 @@ private static boolean notSoftDeleted(PartitionField field) {
     return !(field.name().endsWith(SOFT_DELETE_POSTFIX) && Transforms.alwaysNull().equals(field.transform()));
   }
 
-  private static String getKey(PartitionField field) {
-    return field.transform() + "(" + field.sourceId() + ")";
-  }
-
-  private void checkIfRemoved(PartitionField addedField) {
-    String key = getKey(addedField);
-    Preconditions.checkArgument(!newRemovedFields.containsKey(key),
-        "Cannot add a partition field (%s) because it is compatible with a previously removed field: %s",
-        addedField, newRemovedFields.get(key));
-  }
-
-  private PartitionSpec freshSpecFieldIds(PartitionSpec partitionSpec) {
-    int lastAssignedFieldId = 0;
-    Map<String, Integer> partitionFieldIdByKey = Maps.newHashMap();
-    for (PartitionSpec spec : specs) {
-      for (PartitionField field : spec.fields()) {
-        if (notSoftDeleted(field)) {
-          partitionFieldIdByKey.put(getKey(field), field.fieldId());
-        }
-      }
-      lastAssignedFieldId = Math.max(lastAssignedFieldId, spec.lastAssignedFieldId());
-    }
-
-    List<PartitionField> partitionFields = Lists.newArrayList();
-    for (PartitionField field : partitionSpec.fields()) {
-      String key = getKey(field);
-      if (!partitionFieldIdByKey.containsKey(key)) {
-        lastAssignedFieldId++;
-      }
-      int assignedFieldId = partitionFieldIdByKey.getOrDefault(key, lastAssignedFieldId);
-
-      partitionFields.add(new PartitionField(
-          field.sourceId(),
-          assignedFieldId,
-          field.name(),
-          field.transform()));
-    }
-    partitionFields.sort(Comparator.comparingInt(PartitionField::fieldId));
-
-    return PartitionSpec.builderFor(schema).withSpecId(partitionSpec.specId()).addAll(partitionFields).build();
+  private static String getKey(Transform<?, ?> transform, int sourceId) {
+    return transform + "(" + sourceId + ")";
   }
 
   private PartitionSpec fillGapsByNullFields(PartitionSpec partitionSpec) {
diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java
index d3014ca31ed7..1fe4f4e4a663 100644
--- a/core/src/main/java/org/apache/iceberg/TableMetadata.java
+++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java
@@ -429,6 +429,11 @@ public TableMetadata updatePartitionSpec(PartitionSpec newPartitionSpec) {
       }
     }
 
+    if (defaultSpecId == newDefaultSpecId && newPartitionSpec.equivalentTo(spec())) {
+      // the new spec is already current and return the current table metadata
+      return this;
+    }
+
     // Always setting default partition spec to the new partition spec
     ImmutableList.Builder<PartitionSpec> builder = ImmutableList.builder();
     for (PartitionSpec spec : specs) {
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
index 34e2060a8da6..bbb6b783cb3f 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitionSpecUpdate.java
@@ -65,26 +65,43 @@ public void testAddAndRemoveField() {
   }
 
   @Test
-  public void testAddRemovedFieldException() {
+  public void testAddRemovedSamePartitionField() {
     specUpdate = new PartitionSpecUpdate(initialSpecs);
-    AssertHelpers.assertThrows(
-        "Should throw IllegalArgumentException if adding back a pending removed field",
-        IllegalArgumentException.class, "Cannot add a partition field",
-        () -> specUpdate
+    Assert.assertEquals(
+        "remove and add operations should cancel each other, equivalent to noop",
+        PartitionSpec.builderFor(SCHEMA)
+            .add(2, 1000, "data_bucket", "bucket[16]")
+            .add(1, 1001, "id_bucket", "bucket[8]")
+            .build(),
+        specUpdate
             .removeField("data_bucket")
             .addBucketField("id", 8)
-            .addBucketField("data", 16, "data_partition")
+            .addBucketField("data", 16)
             .apply());
 
     specUpdate = new PartitionSpecUpdate(initialSpecs);
-    AssertHelpers.assertThrows(
-        "Should throw IllegalArgumentException if adding back a pending removed field",
-        IllegalArgumentException.class, "Cannot add a partition field",
-        () -> specUpdate
+    Assert.assertEquals(
+        "remove and add operations should cancel each other, equivalent to noop",
+        PartitionSpec.builderFor(SCHEMA)
+            .add(2, 1000, "data_bucket", "bucket[16]")
+            .add(1, 1001, "id_bucket", "bucket[8]")
+            .build(),
+        specUpdate
             .addBucketField("id", 8)
             .addBucketField("data", 16)
             .removeField("data_bucket")
             .apply());
+
+    specUpdate = new PartitionSpecUpdate(initialSpecs);
+    Assert.assertEquals(
+        "remove and add operations are equivalent to rename data_bucket to data_partition",
+        PartitionSpec.builderFor(SCHEMA)
+            .add(2, 1000, "data_partition", "bucket[16]")
+            .build(),
+        specUpdate
+            .addBucketField("data", 16, "data_partition")
+            .removeField("data_bucket")
+            .apply());
   }
 
   @Test
diff --git a/core/src/test/java/org/apache/iceberg/TestTablePartitionSpecUpdate.java b/core/src/test/java/org/apache/iceberg/TestTablePartitionSpecUpdate.java
index b3ff37ef66a0..a3a785e46518 100644
--- a/core/src/test/java/org/apache/iceberg/TestTablePartitionSpecUpdate.java
+++ b/core/src/test/java/org/apache/iceberg/TestTablePartitionSpecUpdate.java
@@ -83,6 +83,48 @@ public void testCommitUpdatedSpec() {
     Assert.assertEquals(1002, table.spec().lastAssignedFieldId());
   }
 
+  @Test
+  public void testNoopCommit() {
+    TableMetadata current = table.ops().current();
+    Integer currentVersion = TestTables.metadataVersion("test");
+
+    // no-op commit due to no-op
+    table.updateSpec().commit();
+    TableMetadata updated = table.ops().current();
+    Integer updatedVersion = TestTables.metadataVersion("test");
+    Assert.assertEquals(current, updated);
+    Assert.assertEquals(currentVersion, updatedVersion);
+
+    // no-op commit due to no-op rename
+    table.updateSpec()
+        .renameField("data_bucket", "data_bucket")
+        .commit();
+    updated = table.ops().current();
+    updatedVersion = TestTables.metadataVersion("test");
+    Assert.assertEquals(current, updated);
+    Assert.assertEquals(currentVersion, updatedVersion);
+
+    // no-op commit due to no-ops (remove + add for the same field)
+    table.updateSpec()
+        .removeField("data_bucket")
+        .addBucketField("data", 16)
+        .commit();
+    updated = table.ops().current();
+    updatedVersion = TestTables.metadataVersion("test");
+    Assert.assertEquals(current, updated);
+    Assert.assertEquals(currentVersion, updatedVersion);
+
+    // no-op commit due to no-ops (add + remove for the same field)
+    table.updateSpec()
+        .addBucketField("data", 16)
+        .removeField("data_bucket")
+        .commit();
+    updated = table.ops().current();
+    updatedVersion = TestTables.metadataVersion("test");
+    Assert.assertEquals(current, updated);
+    Assert.assertEquals(currentVersion, updatedVersion);
+  }
+
   @Test
   public void testRenameField() {
     table.updateSpec()