yyanyy commented on a change in pull request #2354:
URL: https://github.com/apache/iceberg/pull/2354#discussion_r605980693
##########
File path: core/src/main/java/org/apache/iceberg/TableMetadata.java
##########
@@ -482,6 +506,7 @@ public TableMetadata updateSchema(Schema newSchema, int
newLastColumnId) {
// rebuild all of the partition specs and sort orders for the new current
schema
List<PartitionSpec> updatedSpecs = Lists.transform(specs, spec ->
updateSpecSchema(newSchema, spec));
List<SortOrder> updatedSortOrders = Lists.transform(sortOrders, order ->
updateSortOrderSchema(newSchema, order));
+ RowKey updatedRowKey = updateRowKeySchema(newSchema, rowKey);
Review comment:
should we fail if schema update drops column defined in the row key?
probably want to add test for that too
##########
File path: core/src/test/java/org/apache/iceberg/TestTableMetadata.java
##########
@@ -646,6 +659,55 @@ public void testUpdateSortOrder() {
NullOrder.NULLS_FIRST,
sortedByX.sortOrder().fields().get(0).nullOrder());
}
+ @Test
+ public void testRowKey() {
+ Schema schema = new Schema(
+ Types.NestedField.required(10, "x", Types.StringType.get())
+ );
+
+ TableMetadata meta = TableMetadata.newTableMetadata(
+ schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of());
+ RowKey rowKey = meta.rowKey();
+ Assert.assertTrue("Row key must be default", rowKey.isNotIdentified());
+ }
+
+ @Test
+ public void testUpdateRowKey() {
+ Schema schema = new Schema(
+ Types.NestedField.required(10, "x", Types.StringType.get()),
+ Types.NestedField.required(11, "y", Types.StringType.get())
+ );
+
+ RowKey id = RowKey.builderFor(schema).addField("x").build();
+
+ TableMetadata identifiedByX = TableMetadata.newTableMetadata(
+ schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), id, null,
ImmutableMap.of());
+ RowKey actualKey = identifiedByX.rowKey();
+ Assert.assertEquals("Row key must have 1 field", 1,
actualKey.identifierFields().size());
+ Assert.assertEquals("Row key must have the expected field",
+ org.apache.commons.compress.utils.Sets.newHashSet(1),
Review comment:
nit: wrong import? Also may worth confirming "x" in schema is indeed
assigned as id 1
--
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]
