You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/02 23:37:03 UTC

[GitHub] [iceberg] rdblue commented on a diff in pull request #4668: Core - Add MetadataUpdateParser implementation for AddPartitionSpec and AddSortOrder

rdblue commented on code in PR #4668:
URL: https://github.com/apache/iceberg/pull/4668#discussion_r863259986


##########
core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java:
##########
@@ -197,4 +314,55 @@ private static void assertEqualsSetDefaultPartitionSpec(
       MetadataUpdate.SetDefaultPartitionSpec expected, MetadataUpdate.SetDefaultPartitionSpec actual) {
     Assertions.assertThat(actual.specId()).isEqualTo(expected.specId());
   }
+
+  private static void assertEqualsAddPartitionSpec(
+      MetadataUpdate.AddPartitionSpec expected, MetadataUpdate.AddPartitionSpec actual) {
+    Assert.assertEquals("Unbound partition specs should have the same spec id",
+        expected.spec().specId(), actual.spec().specId());
+    Assert.assertEquals("Unbound partition specs should have the same number of fields",
+        expected.spec().fields().size(), actual.spec().fields().size());
+
+    // Sort the fields so that we can compare them field by field
+    expected.spec().fields().sort(Comparator.comparing(UnboundPartitionSpec.UnboundPartitionField::transformAsString));
+    actual.spec().fields().sort(Comparator.comparing(UnboundPartitionSpec.UnboundPartitionField::transformAsString));
+    IntStream.range(0, expected.spec().fields().size())
+        .forEachOrdered(i -> {
+          UnboundPartitionSpec.UnboundPartitionField expectedField = expected.spec().fields().get(i);
+          UnboundPartitionSpec.UnboundPartitionField actualField = actual.spec().fields().get(i);

Review Comment:
   I don't think these need to be sorted, but otherwise this looks good.



##########
core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java:
##########
@@ -197,4 +314,55 @@ private static void assertEqualsSetDefaultPartitionSpec(
       MetadataUpdate.SetDefaultPartitionSpec expected, MetadataUpdate.SetDefaultPartitionSpec actual) {
     Assertions.assertThat(actual.specId()).isEqualTo(expected.specId());
   }
+
+  private static void assertEqualsAddPartitionSpec(
+      MetadataUpdate.AddPartitionSpec expected, MetadataUpdate.AddPartitionSpec actual) {
+    Assert.assertEquals("Unbound partition specs should have the same spec id",
+        expected.spec().specId(), actual.spec().specId());
+    Assert.assertEquals("Unbound partition specs should have the same number of fields",
+        expected.spec().fields().size(), actual.spec().fields().size());
+
+    // Sort the fields so that we can compare them field by field
+    expected.spec().fields().sort(Comparator.comparing(UnboundPartitionSpec.UnboundPartitionField::transformAsString));
+    actual.spec().fields().sort(Comparator.comparing(UnboundPartitionSpec.UnboundPartitionField::transformAsString));
+    IntStream.range(0, expected.spec().fields().size())
+        .forEachOrdered(i -> {
+          UnboundPartitionSpec.UnboundPartitionField expectedField = expected.spec().fields().get(i);
+          UnboundPartitionSpec.UnboundPartitionField actualField = actual.spec().fields().get(i);

Review Comment:
   I don't think these need to be sorted, but otherwise this looks good.



-- 
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: issues-unsubscribe@iceberg.apache.org

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