yyanyy commented on a change in pull request #2275:
URL: https://github.com/apache/iceberg/pull/2275#discussion_r658283192
##########
File path: core/src/test/java/org/apache/iceberg/TestTableMetadata.java
##########
@@ -93,12 +93,11 @@ public void testJsonConversion() throws Exception {
new GenericManifestFile(localInput("file:/tmp/manfiest.1.avro"),
SPEC_5.specId())));
long currentSnapshotId = System.currentTimeMillis();
Snapshot currentSnapshot = new BaseSnapshot(
- ops.io(), currentSnapshotId, previousSnapshotId, currentSnapshotId,
null, null, ImmutableList.of(
+ ops.io(), currentSnapshotId, previousSnapshotId, currentSnapshotId,
null, null, 7, ImmutableList.of(
new GenericManifestFile(localInput("file:/tmp/manfiest.2.avro"),
SPEC_5.specId())));
List<HistoryEntry> snapshotLog = ImmutableList.<HistoryEntry>builder()
.add(new SnapshotLogEntry(previousSnapshot.timestampMillis(),
previousSnapshot.snapshotId()))
- .add(new SnapshotLogEntry(currentSnapshot.timestampMillis(),
currentSnapshot.snapshotId()))
Review comment:
I modified it earlier when I put schema id in snapshot entry, and later
took it out by mistake as I thought this line was introduced by me. Will make
sure to double check next time!
##########
File path: api/src/test/java/org/apache/iceberg/TestHelpers.java
##########
@@ -120,6 +120,22 @@ public static void assertSerializedAndLoadedMetadata(Table
expected, Table actua
Assert.assertEquals("History must match", expected.history(),
actual.history());
}
+ public static void assertSameSchemaMap(Map<Integer, Schema> map1,
Map<Integer, Schema> map2) {
+ if (map1.size() != map2.size()) {
+ Assert.fail("Should have same number of schemas in both maps");
+ }
+
+ map1.forEach((schemaId, schema1) -> {
+ Schema schema2 = map2.get(schemaId);
+ Assert.assertNotNull(String.format("Schema ID %s does not exist in map:
%s", schemaId, map2), schema2);
+
+ Assert.assertEquals("Should have matching schema id",
+ schema1.schemaId(), schema2.schemaId());
+ Assert.assertEquals("Should have matching schema struct",
Review comment:
Have to rebase to incorporate that, but sure!
--
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]
