aihuaxu commented on code in PR #13284:
URL: https://github.com/apache/iceberg/pull/13284#discussion_r2198328877


##########
api/src/test/java/org/apache/iceberg/variants/TestSerializedObject.java:
##########
@@ -182,66 +182,55 @@ public void testMixedValueTypes() {
     assertThat(actualInner.get("f").asPrimitive().get()).isEqualTo((byte) 3);
   }
 
-  @Test
-  public void testTwoByteOffsets() {
-    // a string larger than 255 bytes to push the value offset size above 1 
byte
-    String randomString = RandomUtil.generateString(300, random);
+  @ParameterizedTest
+  @ValueSource(
+      ints = {
+        300, // a big string larger than 255 bytes to push the value offset 
size above 1 byte to
+        // test TwoByteOffsets
+        70_000 // a really-big string larger than 65535 bytes to push the 
value offset size above 1
+        // byte to test ThreeByteOffsets
+      })
+  public void testMultiByteOffsets(int multiByteOffset) {
+    String randomString = RandomUtil.generateString(multiByteOffset, random);
     SerializedPrimitive bigString = VariantTestUtil.createString(randomString);
 
-    // note that order doesn't matter. fields are sorted by name
-    Map<String, VariantValue> data = ImmutableMap.of("big", bigString, "a", 
I1, "b", I2, "c", I3);
-    ByteBuffer meta = VariantTestUtil.createMetadata(data.keySet(), true /* 
sort names */);
-    ByteBuffer value = VariantTestUtil.createObject(meta, data);
-
-    VariantMetadata metadata = VariantMetadata.from(meta);
-    SerializedObject object = SerializedObject.from(metadata, value, 
value.get(0));
-
-    assertThat(object.type()).isEqualTo(PhysicalType.OBJECT);
-    assertThat(object.numFields()).isEqualTo(4);
-
-    assertThat(object.get("a").type()).isEqualTo(PhysicalType.INT8);
-    assertThat(object.get("a").asPrimitive().get()).isEqualTo((byte) 1);
-    assertThat(object.get("b").type()).isEqualTo(PhysicalType.INT8);
-    assertThat(object.get("b").asPrimitive().get()).isEqualTo((byte) 2);
-    assertThat(object.get("c").type()).isEqualTo(PhysicalType.INT8);
-    assertThat(object.get("c").asPrimitive().get()).isEqualTo((byte) 3);
-    assertThat(object.get("big").type()).isEqualTo(PhysicalType.STRING);
-    assertThat(object.get("big").asPrimitive().get()).isEqualTo(randomString);
-  }
-
-  @Test
-  public void testThreeByteOffsets() {
-    // a string larger than 65535 bytes to push the value offset size above 1 
byte
-    String randomString = RandomUtil.generateString(70_000, random);
-    SerializedPrimitive reallyBigString = 
VariantTestUtil.createString(randomString);
-
     // note that order doesn't matter. fields are sorted by name
     Map<String, VariantValue> data =
-        ImmutableMap.of("really-big", reallyBigString, "a", I1, "b", I2, "c", 
I3);
+        ImmutableMap.of(
+            "small",
+            VariantTestUtil.createShortString("iceberg"),

Review Comment:
   nit: this may not be needed for this test since this test is targeted to 
test out value offset from the big string?



##########
api/src/test/java/org/apache/iceberg/variants/TestSerializedObject.java:
##########
@@ -257,13 +246,40 @@ public void testLargeObject(boolean sortFieldNames) {
     assertThat(object.type()).isEqualTo(PhysicalType.OBJECT);
     assertThat(object.numFields()).isEqualTo(10_000);
 
-    for (Map.Entry<String, SerializedPrimitive> entry : fields.entrySet()) {
+    for (Map.Entry<String, VariantPrimitive<?>> entry : fields.entrySet()) {
       VariantValue fieldValue = object.get(entry.getKey());
-      assertThat(fieldValue.type()).isEqualTo(PhysicalType.STRING);
-      
assertThat(fieldValue.asPrimitive().get()).isEqualTo(entry.getValue().get());
+      assertThat(fieldValue.getClass()).isEqualTo(SerializedPrimitive.class);
+      assertThat(fieldValue.asPrimitive().sizeInBytes()).isEqualTo(5 + 10);

Review Comment:
   I think we should revert this change. This is to test out the object has 
many fields. I don't think we need to check the size here.



-- 
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-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to