This is an automated email from the ASF dual-hosted git repository.
wgtmac pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/parquet-java.git
The following commit(s) were added to refs/heads/master by this push:
new f0030bd36 GH-3625: Compute dataSize correctly when dealing with
duplicate keys (#3626)
f0030bd36 is described below
commit f0030bd360ff6c7fd1a2bc8f56d3016b1e5549b6
Author: Eduard Tudenhoefner <[email protected]>
AuthorDate: Tue Jun 30 17:05:47 2026 +0200
GH-3625: Compute dataSize correctly when dealing with duplicate keys (#3626)
---
.../org/apache/parquet/variant/VariantBuilder.java | 9 ++-
.../parquet/variant/TestVariantObjectBuilder.java | 66 ++++++++++++++++++++++
2 files changed, 73 insertions(+), 2 deletions(-)
diff --git
a/parquet-variant/src/main/java/org/apache/parquet/variant/VariantBuilder.java
b/parquet-variant/src/main/java/org/apache/parquet/variant/VariantBuilder.java
index 5b0ae146f..c692d3119 100644
---
a/parquet-variant/src/main/java/org/apache/parquet/variant/VariantBuilder.java
+++
b/parquet-variant/src/main/java/org/apache/parquet/variant/VariantBuilder.java
@@ -498,7 +498,6 @@ public class VariantBuilder {
int numFields = fields.size();
Collections.sort(fields);
int maxId = numFields == 0 ? 0 : fields.get(0).id;
- int dataSize = numFields == 0 ? 0 : fields.get(0).valueSize;
int distinctPos = 0;
// Maintain a list of distinct keys in-place.
@@ -513,7 +512,6 @@ public class VariantBuilder {
// Found a distinct key. Add the field to the list.
distinctPos++;
fields.set(distinctPos, fields.get(i));
- dataSize += fields.get(i).valueSize;
}
}
@@ -523,6 +521,13 @@ public class VariantBuilder {
fields.subList(numFields, fields.size()).clear();
}
+ // Compute the data size from the retained fields. This must happen after
deduplication, since a
+ // duplicate key keeps the last-written value, whose size may differ from
the first occurrence.
+ int dataSize = 0;
+ for (int i = 0; i < numFields; ++i) {
+ dataSize += fields.get(i).valueSize;
+ }
+
boolean largeSize = numFields > VariantUtil.U8_MAX;
int sizeBytes = largeSize ? VariantUtil.U32_SIZE : 1;
int idSize = getMinIntegerSize(maxId);
diff --git
a/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantObjectBuilder.java
b/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantObjectBuilder.java
index 56e8d1e3c..e07b55466 100644
---
a/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantObjectBuilder.java
+++
b/parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantObjectBuilder.java
@@ -302,6 +302,72 @@ public class TestVariantObjectBuilder {
Assert.assertEquals(1, v.getFieldByKey("duplicate").getLong());
}
+ @Test
+ public void testDuplicateKeysKeptValueLarger() {
+ // The retained (last-written) value is larger than the first occurrence.
The data size must be
+ // computed from the retained value, otherwise the encoded object is
truncated/corrupt.
+ VariantBuilder b = new VariantBuilder();
+ VariantObjectBuilder objBuilder = b.startObject();
+ objBuilder.appendKey("duplicate");
+ objBuilder.appendInt(1); // 5 bytes
+ objBuilder.appendKey("duplicate");
+ objBuilder.appendString("hello"); // 6 bytes
+ b.endObject();
+ VariantTestUtil.testVariant(b.build(), v -> {
+ VariantTestUtil.checkType(v, VariantUtil.OBJECT, Variant.Type.OBJECT);
+ Assert.assertEquals(1, v.numObjectElements());
+ Variant variant = v.getFieldByKey("duplicate");
+ VariantTestUtil.checkType(variant, VariantUtil.SHORT_STR,
Variant.Type.STRING);
+ Assert.assertEquals("hello", variant.getString());
+ });
+ }
+
+ @Test
+ public void testDuplicateKeysKeptValueSmaller() {
+ // The retained (last-written) value is smaller than the first occurrence.
The data size must be
+ // computed from the retained value, otherwise the encoded object reserves
stale trailing bytes.
+ VariantBuilder b = new VariantBuilder();
+ VariantObjectBuilder objBuilder = b.startObject();
+ objBuilder.appendKey("duplicate");
+ objBuilder.appendString("hello"); // 6 bytes
+ objBuilder.appendKey("duplicate");
+ objBuilder.appendInt(1); // 5 bytes
+ b.endObject();
+ VariantTestUtil.testVariant(b.build(), v -> {
+ VariantTestUtil.checkType(v, VariantUtil.OBJECT, Variant.Type.OBJECT);
+ Assert.assertEquals(1, v.numObjectElements());
+ Variant variant = v.getFieldByKey("duplicate");
+ VariantTestUtil.checkType(variant, VariantUtil.PRIMITIVE,
Variant.Type.INT);
+ Assert.assertEquals(1, variant.getInt());
+ });
+ }
+
+ @Test
+ public void testDuplicateKeysDifferentSizesAcrossMultipleKeys() {
+ // Exercises deduplication across several keys where the retained values
differ in size from the
+ // first occurrence, ensuring offsets remain consistent for every field.
+ VariantBuilder b = new VariantBuilder();
+ VariantObjectBuilder objBuilder = b.startObject();
+ objBuilder.appendKey("a");
+ objBuilder.appendInt(1); // 5 bytes
+ objBuilder.appendKey("b");
+ objBuilder.appendBoolean(true); // 1 byte
+ objBuilder.appendKey("a");
+ objBuilder.appendString("a-final"); // larger, retained for "a"
+ objBuilder.appendKey("b");
+ objBuilder.appendLong(123456789L); // 9 bytes, retained for "b"
+ objBuilder.appendKey("c");
+ objBuilder.appendString("c-only");
+ b.endObject();
+ VariantTestUtil.testVariant(b.build(), v -> {
+ VariantTestUtil.checkType(v, VariantUtil.OBJECT, Variant.Type.OBJECT);
+ Assert.assertEquals(3, v.numObjectElements());
+ Assert.assertEquals("a-final", v.getFieldByKey("a").getString());
+ Assert.assertEquals(123456789L, v.getFieldByKey("b").getLong());
+ Assert.assertEquals("c-only", v.getFieldByKey("c").getString());
+ });
+ }
+
@Test
public void testSortingKeys() {
VariantBuilder b = new VariantBuilder();
