[GitHub] [iceberg] thomaschow commented on a change in pull request #2204: Allow adding of deleted columns

[GitBox <gi...@apache.org>]

thomaschow commented on a change in pull request #2204:
URL: https://github.com/apache/iceberg/pull/2204#discussion_r571460404



##########
File path: core/src/test/java/org/apache/iceberg/TestSchemaUpdate.java
##########
@@ -501,6 +501,58 @@ public void testAddAlreadyExists() {
     );
   }
 
+    @Test
+    public void testDeleteThenAdd() {
+        Schema schema = new Schema(required(1, "id", Types.IntegerType.get()));
+        Schema expected = new Schema(optional(2, "id", Types.IntegerType.get()));
+
+        Schema updated = new SchemaUpdate(schema, 1)
+                .deleteColumn("id")
+                .addColumn("id", optional(2, "id", Types.IntegerType.get()).type())
+                .apply();
+
+        Assert.assertEquals("Should match with added fields", expected.asStruct(), updated.asStruct());
+
+        Schema expectedNested = new Schema(
+                required(1, "id", Types.IntegerType.get()),
+                optional(2, "data", Types.StringType.get()),
+                optional(3, "preferences", Types.StructType.of(
+                        optional(9, "feature2", Types.BooleanType.get()),
+                        optional(24, "feature1", Types.BooleanType.get())
+                ), "struct of named boolean options"),
+                required(4, "locations", Types.MapType.ofRequired(10, 11,
+                        Types.StructType.of(
+                                required(20, "address", Types.StringType.get()),
+                                required(21, "city", Types.StringType.get()),
+                                required(22, "state", Types.StringType.get()),
+                                required(23, "zip", Types.IntegerType.get())
+                        ),
+                        Types.StructType.of(
+                                required(12, "lat", Types.FloatType.get()),
+                                required(13, "long", Types.FloatType.get())
+                        )), "map of address to coordinate"),
+                optional(5, "points", Types.ListType.ofOptional(14,
+                        Types.StructType.of(
+                                required(15, "x", Types.LongType.get()),
+                                required(16, "y", Types.LongType.get())
+                        )), "2-D cartesian points"),
+                required(6, "doubles", Types.ListType.ofRequired(17,
+                        Types.DoubleType.get()
+                )),
+                optional(7, "properties", Types.MapType.ofOptional(18, 19,
+                        Types.StringType.get(),
+                        Types.StringType.get()
+                ), "string map of properties")
+        );
+
+        Schema updatedNested = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID)
+                .deleteColumn("preferences.feature1")
+                .addColumn("preferences", "feature1", Types.BooleanType.get())
+                .apply();

Review comment:
       this test shows that the field id _is_ updated, and I would agree with your perspective.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org