nssalian commented on code in PR #17726:
URL: https://github.com/apache/iceberg/pull/17726#discussion_r3839002071


##########
core/src/test/java/org/apache/iceberg/variants/TestShreddedObject.java:
##########
@@ -65,6 +65,25 @@ public void testShreddedFields() {
     assertThat(object.get("c").asPrimitive().get()).isEqualTo(new 
BigDecimal("12.21"));
   }
 
+  @Test
+  public void testShreddedFieldOrderFollowsUtf8ByteOrder() {
+    // U+FFFF is 3 UTF-8 bytes (EF BF BF), U+10000 is 4 (F0 90 80 80); they 
order oppositely in
+    // UTF-16
+    String threeByteName = new String(Character.toChars(0xFFFF));
+    String fourByteName = new String(Character.toChars(0x10000));
+    VariantMetadata metadata = Variants.metadata(threeByteName, fourByteName);
+    Map<String, VariantValue> fields =
+        ImmutableMap.of(threeByteName, Variants.of(1), fourByteName, 
Variants.of(2));
+    ShreddedObject object = createShreddedObject(metadata, fields);
+
+    VariantValue value = roundTripMinimalBuffer(object, metadata);
+
+    assertThat(value).isInstanceOf(SerializedObject.class);
+    SerializedObject actual = (SerializedObject) value;
+    assertThat(actual.get(threeByteName).asPrimitive().get()).isEqualTo(1);
+    assertThat(actual.get(fourByteName).asPrimitive().get()).isEqualTo(2);

Review Comment:
   I did see the `TestVariantMetadataFieldOrdering` fail on revert in my local 
tests but this one is missing the comparator/order assertion. Let me add your 
suggestion.



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

Reply via email to