(iceberg) branch 1.10.x updated: Parquet: Backport variant STRING bounds UTF-8 fix (#17397) to 1.10.x (#17589)

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

huaxingao pushed a commit to branch 1.10.x
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/1.10.x by this push:
     new 1285e208f1 Parquet: Backport variant STRING bounds UTF-8 fix (#17397) to 1.10.x (#17589)
1285e208f1 is described below

commit 1285e208f17b49dc8c6fce5ceab3e2b8160bcd4a
Author: vishnu prakash <nv...@gmail.com>
AuthorDate: Sat Sep 12 11:03:37 2026 +0530

    Parquet: Backport variant STRING bounds UTF-8 fix (#17397) to 1.10.x (#17589)
    
    Parquet: Fix variant STRING metrics bounds to use UTF-8 byte order (#17397)
---
 .../apache/iceberg/parquet/ParquetVariantUtil.java | 11 +++--
 .../apache/iceberg/parquet/TestVariantMetrics.java | 53 ++++++++++++++++++++++
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/parquet/src/main/java/org/apache/iceberg/parquet/ParquetVariantUtil.java b/parquet/src/main/java/org/apache/iceberg/parquet/ParquetVariantUtil.java
index a9b088aaf2..9d5dbbba0a 100644
--- a/parquet/src/main/java/org/apache/iceberg/parquet/ParquetVariantUtil.java
+++ b/parquet/src/main/java/org/apache/iceberg/parquet/ParquetVariantUtil.java
@@ -184,10 +184,13 @@ class ParquetVariantUtil {
    */
   @SuppressWarnings("unchecked")
   static <T> Comparator<T> comparator(PhysicalType primitive) {
-    if (primitive == PhysicalType.BINARY) {
-      return (Comparator<T>) Comparators.unsignedBytes();
-    } else {
-      return (Comparator<T>) Comparator.naturalOrder();
+    switch (primitive) {
+      case BINARY:
+        return (Comparator<T>) Comparators.unsignedBytes();
+      case STRING:
+        return (Comparator<T>) Comparators.charSequences();
+      default:
+        return (Comparator<T>) Comparator.naturalOrder();
     }
   }
 
diff --git a/parquet/src/test/java/org/apache/iceberg/parquet/TestVariantMetrics.java b/parquet/src/test/java/org/apache/iceberg/parquet/TestVariantMetrics.java
index c9dcd2b8ee..9cfc0feb41 100644
--- a/parquet/src/test/java/org/apache/iceberg/parquet/TestVariantMetrics.java
+++ b/parquet/src/test/java/org/apache/iceberg/parquet/TestVariantMetrics.java
@@ -18,6 +18,7 @@
  */
 package org.apache.iceberg.parquet;
 
+import static org.apache.iceberg.TableProperties.PARQUET_ROW_GROUP_SIZE_BYTES;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.io.IOException;
@@ -266,6 +267,33 @@ public class TestVariantMetrics {
         .isNull();
   }
 
+  @Test
+  public void testShreddedStringBoundsAcrossRowGroups() throws IOException {
+    // shredded string bounds must use UTF-8 order (Comparators.charSequences), like the scan path;
+    // String.compareTo (UTF-16) sorts a supplementary char below U+E000 and would invert the bounds
+    String belowSurrogate = new String(Character.toChars(0xE000));
+    String supplementary = new String(Character.toChars(0x10000));
+
+    Variant[] rows = new Variant[300];
+    for (int i = 0; i < rows.length; i += 1) {
+      rows[i] = Variant.of(EMPTY, Variants.of(i < 150 ? belowSurrogate : supplementary));
+    }
+
+    // a tiny row-group size forces multiple row groups so cross-chunk bound aggregation runs
+    Metrics metrics =
+        writeParquetWithRowGroupSize(
+            (id, name) -> ParquetVariantUtil.toParquetSchema(Variants.of(belowSurrogate)),
+            "1",
+            rows);
+
+    assertThat(metrics.lowerBounds().get(2))
+        .extracting(b -> Variant.from(b).value().asObject().get(ROOT_FIELD))
+        .isEqualTo(Variants.of(belowSurrogate));
+    assertThat(metrics.upperBounds().get(2))
+        .extracting(b -> Variant.from(b).value().asObject().get(ROOT_FIELD))
+        .isEqualTo(Variants.of(supplementary));
+  }
+
   @Test
   public void testVariantFloatNaN() throws IOException {
     // NaN values are not counted because there is no ID for FieldMetrics
@@ -552,6 +580,31 @@ public class TestVariantMetrics {
     return writer.metrics();
   }
 
+  private Metrics writeParquetWithRowGroupSize(
+      VariantShreddingFunction shredding, String rowGroupSizeBytes, Variant... variants)
+      throws IOException {
+    OutputFile out = new InMemoryOutputFile();
+    GenericRecord record = GenericRecord.create(SCHEMA);
+
+    FileAppender<Record> writer =
+        Parquet.write(out)
+            .schema(SCHEMA)
+            .variantShreddingFunc(shredding)
+            .set(PARQUET_ROW_GROUP_SIZE_BYTES, rowGroupSizeBytes)
+            .createWriterFunc(fileSchema -> InternalWriter.create(SCHEMA.asStruct(), fileSchema))
+            .build();
+
+    try (writer) {
+      for (int id = 0; id < variants.length; id += 1) {
+        record.setField("id", (long) id);
+        record.setField("var", variants[id]);
+        writer.add(record);
+      }
+    }
+
+    return writer.metrics();
+  }
+
   private static VariantValue increment(VariantValue value) {
     VariantPrimitive<?> primitive = value.asPrimitive();
     switch (value.type()) {