kbendick commented on a change in pull request #2204:
URL: https://github.com/apache/iceberg/pull/2204#discussion_r569876843
##########
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:
Here, when we add a column `preferences.feature1` back, I would
personally expect for the field id to be updated. My thinking for this is that
the new `preferences.feature1` might not represent the same thing as the old
`preferences.feature1`.
I recognize that might not be the expected behavior for everybody else, but
I am curious what others think. To me, deleting a column and then adding a new
column with the same name indicates that this is a new column. What does
everybody else think?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
