[GitHub] [iceberg] wypoon commented on a change in pull request #2096: Core: add schema id and schemas to table metadata

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

wypoon commented on a change in pull request #2096:
URL: https://github.com/apache/iceberg/pull/2096#discussion_r589966136



##########
File path: core/src/test/java/org/apache/iceberg/TestTableMetadata.java
##########
@@ -223,6 +237,7 @@ public static String toJsonWithoutSpecList(TableMetadata metadata) {
       generator.writeNumberField(LAST_UPDATED_MILLIS, metadata.lastUpdatedMillis());
       generator.writeNumberField(LAST_COLUMN_ID, metadata.lastColumnId());
 
+      // mimic an old writer by writing only schema and not the current ID or schema list
       generator.writeFieldName(SCHEMA);
       SchemaParser.toJson(metadata.schema(), generator);

Review comment:
       I think that to properly test backward compatibility, the schema in this `schema` field should not have a `schema-id`, so this should be `SchemaParser.toJson(metadata.schema().asStruct(), generator);`

##########
File path: core/src/test/java/org/apache/iceberg/TestTableMetadata.java
##########
@@ -100,8 +101,14 @@ public void testJsonConversion() throws Exception {
         .add(new SnapshotLogEntry(currentSnapshot.timestampMillis(), currentSnapshot.snapshotId()))
         .build();
 
+    Schema schema = new Schema(6,
+        Types.NestedField.required(10, "x", Types.StringType.get()));
+
     TableMetadata expected = new TableMetadata(null, 2, UUID.randomUUID().toString(), TEST_LOCATION,
-        SEQ_NO, System.currentTimeMillis(), 3, TEST_SCHEMA, 5, ImmutableList.of(SPEC_5), SPEC_5.lastAssignedFieldId(),
+        SEQ_NO, System.currentTimeMillis(), 3,
+        7, ImmutableList.of(TEST_SCHEMA, schema),
+        5, ImmutableList.of(SPEC_5), SPEC_5.lastAssignedFieldId(),
+

Review comment:
       Nit: extraneous blank line.




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