szehon-ho commented on code in PR #4560:
URL: https://github.com/apache/iceberg/pull/4560#discussion_r856679592


##########
spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTablesWithPartitionEvolution.java:
##########
@@ -212,7 +214,6 @@ public void testFilesMetadataTableFilter() throws 
ParseException {
     sql("CREATE TABLE %s (id bigint NOT NULL, category string, data string) 
USING iceberg " +
         "TBLPROPERTIES ('commit.manifest-merge.enabled' 'false')", tableName);
     initTable();
-

Review Comment:
   Removed whitespace



##########
core/src/main/java/org/apache/iceberg/PartitionsTable.java:
##########
@@ -140,20 +195,13 @@ private class PartitionsScan extends StaticTableScan {
   }
 
   static class PartitionMap {
-    private final Map<StructLikeWrapper, Partition> partitions = 
Maps.newHashMap();
-    private final Types.StructType type;
-    private final StructLikeWrapper reused;
-
-    PartitionMap(Types.StructType type) {
-      this.type = type;
-      this.reused = StructLikeWrapper.forType(type);
-    }
+    private final Map<StructLike, Partition> partitions = Maps.newHashMap();
 
     Partition get(StructLike key) {
-      Partition partition = partitions.get(reused.set(key));
+      Partition partition = partitions.get(key);
       if (partition == null) {
         partition = new Partition(key);
-        partitions.put(StructLikeWrapper.forType(type).set(key), partition);

Review Comment:
   Changed to map of PartitionData (I feel, it should have been that way in the 
beginning)



##########
spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTablesWithPartitionEvolution.java:
##########
@@ -378,10 +378,177 @@ public void testEntriesMetadataTable() throws 
ParseException {
           tableType);
     }
   }
+  @Test
+  public void testPartitionsTableAddRemoveFields() throws ParseException {
+    sql("CREATE TABLE %s (id bigint NOT NULL, category string, data string) 
USING iceberg " +
+        "TBLPROPERTIES ('commit.manifest-merge.enabled' 'false')", tableName);
+    initTable();
+    sql("INSERT INTO TABLE %s VALUES (1, 'c1', 'd1')", tableName);
+    sql("INSERT INTO TABLE %s VALUES (2, 'c2', 'd2')", tableName);
+
+    // verify the metadata tables while the current spec is still unpartitioned
+    Dataset<Row> df = loadMetadataTable(PARTITIONS);
+    Assert.assertTrue("Partition must be skipped", 
df.schema().getFieldIndex("partition").isEmpty());
+
+    Table table = validationCatalog.loadTable(tableIdent);
+
+    table.updateSpec()

Review Comment:
   Will just keep it for now then, matches existing tests



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to