huan233usc commented on code in PR #762:
URL: https://github.com/apache/iceberg-cpp/pull/762#discussion_r3450001246
##########
src/iceberg/test/metadata_serde_test.cc:
##########
@@ -486,6 +486,54 @@ TEST(MetadataSerdeTest, DeserializeV2MissingSortOrder) {
"sort-orders must exist");
}
+TEST(MetadataSerdeTest, DeserializeHistoricalSortOrderWithDroppedField) {
+ nlohmann::json metadata_json = R"({
+ "format-version": 2,
+ "table-uuid": "test-uuid-1234",
+ "location": "s3://bucket/test",
+ "last-sequence-number": 0,
+ "last-updated-ms": 0,
+ "last-column-id": 2,
+ "schemas": [
+ {
+ "type": "struct",
+ "schema-id": 1,
+ "fields": [
+ {"id": 1, "name": "id", "type": "int", "required": true}
+ ]
+ }
+ ],
+ "current-schema-id": 1,
+ "partition-specs": [{"spec-id": 0, "fields": []}],
+ "default-spec-id": 0,
+ "last-partition-id": 999,
+ "sort-orders": [
+ {"order-id": 1, "fields": [
+ {"transform": "identity", "source-id": 1, "direction": "asc",
"null-order": "nulls-first"},
+ {"transform": "identity", "source-id": 2, "direction": "asc",
"null-order": "nulls-first"}
+ ]},
+ {"order-id": 2, "fields": [
+ {"transform": "identity", "source-id": 1, "direction": "asc",
"null-order": "nulls-first"}
+ ]}
+ ],
+ "default-sort-order-id": 2,
+ "properties": {},
+ "current-snapshot-id": null,
+ "refs": {},
+ "snapshots": [],
+ "statistics": [],
+ "partition-statistics": [],
+ "snapshot-log": [],
+ "metadata-log": []
+ })"_json;
+
+ auto metadata = TableMetadataFromJson(metadata_json);
+ ASSERT_THAT(metadata, IsOk());
+ ASSERT_EQ(metadata.value()->sort_orders.size(), 2);
Review Comment:
Can we assert the historical sort order still has two fields, including
`source_id() == 2`? Otherwise this would still pass if the dropped-field sort
term were skipped. A negative case where that order is the default sort order
would also help verify default sort order validation is still strict.
--
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]
