[iceberg] branch master updated: Core: Allow deleting old partition spec columns in V1 (#7398)

[bl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

blue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/master by this push:
     new 36ad90b876 Core: Allow deleting old partition spec columns in V1 (#7398)
36ad90b876 is described below

commit 36ad90b8765221709506dcbed2717ab258a6f0de
Author: Fokko Driesprong <fo...@apache.org>
AuthorDate: Tue May 16 04:08:38 2023 +0200

    Core: Allow deleting old partition spec columns in V1 (#7398)
    
    Resolves #7386
---
 .../main/java/org/apache/iceberg/TableMetadata.java | 15 ++++++++++-----
 .../java/org/apache/iceberg/TestPartitioning.java   | 21 +++++++++++++++++++++
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java
index f332f84fc8..bd409bdeed 100644
--- a/core/src/main/java/org/apache/iceberg/TableMetadata.java
+++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java
@@ -738,11 +738,16 @@ public class TableMetadata implements Serializable {
     for (PartitionField field : partitionSpec.fields()) {
       // look up the name of the source field in the old schema to get the new schema's id
       String sourceName = partitionSpec.schema().findColumnName(field.sourceId());
-      specBuilder.addField(
-          field.transform().toString(),
-          schema.findField(sourceName).fieldId(),
-          field.fieldId(),
-          field.name());
+
+      final int fieldId;
+      if (sourceName != null) {
+        fieldId = schema.findField(sourceName).fieldId();
+      } else {
+        // In the case of a null sourceName, the column has been deleted.
+        // This only happens in V1 tables where the reference is still around as a void transform
+        fieldId = field.sourceId();
+      }
+      specBuilder.addField(field.transform().toString(), fieldId, field.fieldId(), field.name());
     }
 
     return specBuilder.build().bind(schema);
diff --git a/core/src/test/java/org/apache/iceberg/TestPartitioning.java b/core/src/test/java/org/apache/iceberg/TestPartitioning.java
index 1faf8d2f69..21558be3da 100644
--- a/core/src/test/java/org/apache/iceberg/TestPartitioning.java
+++ b/core/src/test/java/org/apache/iceberg/TestPartitioning.java
@@ -386,4 +386,25 @@ public class TestPartitioning {
         "Conflicting partition fields",
         () -> Partitioning.groupingKeyType(table.schema(), table.specs().values()));
   }
+
+  @Test
+  public void testDeletingPartitionField() {
+    TestTables.TestTable table =
+        TestTables.create(tableDir, "test", SCHEMA, BY_DATA_SPEC, V1_FORMAT_VERSION);
+
+    table.updateSpec().removeField("data").commit();
+
+    table.updateSchema().deleteColumn("data").commit();
+
+    table.updateSpec().addField("id").commit();
+
+    PartitionSpec spec =
+        PartitionSpec.builderFor(SCHEMA)
+            .withSpecId(2)
+            .alwaysNull("data", "data")
+            .identity("id")
+            .build();
+
+    Assert.assertEquals("The spec should be there", spec, table.spec());
+  }
 }