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

voonhous pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git


The following commit(s) were added to refs/heads/master by this push:
     new f25e0799c5b7 fix: Handle map/array-nested leaf columns in column stats 
collection during MOR log-append (#19126)
f25e0799c5b7 is described below

commit f25e0799c5b73e8b77d777910dab78859406b1c5
Author: Vinish Reddy <[email protected]>
AuthorDate: Thu Jul 23 10:30:27 2026 +0530

    fix: Handle map/array-nested leaf columns in column stats collection during 
MOR log-append (#19126)
    
    * fix: Handle map/array nested leaves in Avro column-stats value navigation
    
    Collecting column stats for a primitive nested inside a MAP or ARRAY
    (e.g. `my_map.key_value.value`, `my_array.list.element`) crashed the MOR
    inline log-append path at table version 9:
    
        IllegalStateException: Cannot get field from schema type: MAP
          at HoodieSchema.getField
          at AvroRecordContext.getFieldValueFromIndexedRecord
          at HoodieAvroIndexedRecord.getColumnValueAsJava
          at HoodieTableMetadataUtil.collectColumnRangeFieldValueV2
          at HoodieInlineLogAppendHandle.collectColumnStats
    
    #17694 taught the schema-side navigator (HoodieSchema.getNestedField) and
    the base-file (Parquet) path to resolve the Parquet-style `.key_value.key`,
    `.key_value.value` and `.list.element` synthetic accessors, so such leaves
    pass isColumnTypeSupported. The record value-side navigator
    (AvroRecordContext.getFieldValueFromIndexedRecord), used by the column-stats
    V2 collection path (collectColumnRangeFieldValueV2 -> getColumnValueAsJava),
    was never updated: it assumes every path segment is a RECORD field and calls
    HoodieSchema.getField on the MAP/ARRAY schema, which throws.
    
    Make the value navigator return null when a path segment cannot be resolved
    as a plain RECORD field (a MAP/ARRAY intermediate or a null intermediate),
    instead of throwing. This mirrors HoodieAvroUtils.getNestedFieldVal, which 
is
    why the V1 path at table version 6 already tolerated these paths. A 
map/array
    leaf is multi-valued per record and has no single value to fold into a
    min/max, so returning null (no stats from the record path) is correct;
    statistics for such leaves are still collected from the base-file (Parquet)
    footer path added in #17694.
    
    Also guard the intermediate downcast so a non-record value (java Map/List)
    degrades to null rather than throwing ClassCastException.
    
    * Rename test constant COMPLEX_SCHEMA to MAP_AND_ARRAY_SCHEMA for clarity
    
    * Move the schema constants to the top of TestAvroRecordContext
    
    ---------
    
    Co-authored-by: voon <[email protected]>
---
 .../apache/hudi/common/avro/AvroRecordContext.java | 10 +++-
 .../hudi/common/avro/TestAvroRecordContext.java    | 67 ++++++++++++++++++----
 2 files changed, 65 insertions(+), 12 deletions(-)

diff --git 
a/hudi-common/src/main/java/org/apache/hudi/common/avro/AvroRecordContext.java 
b/hudi-common/src/main/java/org/apache/hudi/common/avro/AvroRecordContext.java
index 30ddf7f789a9..e8b6da238c47 100644
--- 
a/hudi-common/src/main/java/org/apache/hudi/common/avro/AvroRecordContext.java
+++ 
b/hudi-common/src/main/java/org/apache/hudi/common/avro/AvroRecordContext.java
@@ -79,6 +79,14 @@ public class AvroRecordContext extends 
RecordContext<IndexedRecord> {
     String[] path = fieldName.split("\\.");
     for (int i = 0; i < path.length; i++) {
       currentSchema = currentSchema.getNonNullType();
+      // Value navigation here can only descend through RECORD fields. 
Column-stats field paths
+      // that traverse a MAP (".key_value.key" / ".key_value.value") or ARRAY 
(".list.element")
+      // synthetic accessor, or that hit a null intermediate value, cannot be 
resolved to a single
+      // value and yield null instead of throwing. This mirrors 
HoodieAvroUtils.getNestedFieldVal;
+      // statistics for such nested leaves are still collected from the 
base-file (Parquet) path.
+      if (currentRecord == null || !currentSchema.hasFields()) {
+        return null;
+      }
       Option<HoodieSchemaField> fieldOpt = currentSchema.getField(path[i]);
       if (fieldOpt.isEmpty()) {
         return null;
@@ -89,7 +97,7 @@ public class AvroRecordContext extends 
RecordContext<IndexedRecord> {
         return value;
       }
       currentSchema = field.schema();
-      currentRecord = (IndexedRecord) value;
+      currentRecord = value instanceof IndexedRecord ? (IndexedRecord) value : 
null;
     }
     return null;
   }
diff --git 
a/hudi-common/src/test/java/org/apache/hudi/common/avro/TestAvroRecordContext.java
 
b/hudi-common/src/test/java/org/apache/hudi/common/avro/TestAvroRecordContext.java
index 3a905a248254..d1835dfbf7ae 100644
--- 
a/hudi-common/src/test/java/org/apache/hudi/common/avro/TestAvroRecordContext.java
+++ 
b/hudi-common/src/test/java/org/apache/hudi/common/avro/TestAvroRecordContext.java
@@ -28,6 +28,9 @@ import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.stream.Stream;
 
 import static 
org.apache.hudi.common.avro.AvroRecordContext.getFieldValueFromIndexedRecord;
@@ -37,6 +40,23 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class TestAvroRecordContext {
 
+  private static final Schema RECORD_SCHEMA = new Schema.Parser().parse(
+      "{\"type\":\"record\",\"name\":\"top\",\"fields\":["
+          + "{\"name\":\"id\",\"type\":\"int\"},"
+          + 
"{\"name\":\"name\",\"type\":[\"null\",\"string\"],\"default\":null},"
+          + 
"{\"name\":\"address\",\"type\":[\"null\",{\"type\":\"record\",\"name\":\"address\",\"fields\":["
+          + "{\"name\":\"city\",\"type\":\"string\"},"
+          + 
"{\"name\":\"zip\",\"type\":[\"null\",\"int\"],\"default\":null}]}],\"default\":null},"
+          + 
"{\"name\":\"multi\",\"type\":[\"null\",\"string\",\"int\"],\"default\":null}]}");
+
+  private static final Schema MAP_AND_ARRAY_SCHEMA = new Schema.Parser().parse(
+      "{\"type\":\"record\",\"name\":\"complex\",\"fields\":["
+          + "{\"name\":\"id\",\"type\":\"int\"},"
+          + 
"{\"name\":\"str_map\",\"type\":[\"null\",{\"type\":\"map\",\"values\":\"string\"}],\"default\":null},"
+          + 
"{\"name\":\"int_array\",\"type\":[\"null\",{\"type\":\"array\",\"items\":\"int\"}],\"default\":null},"
+          + 
"{\"name\":\"rec_map\",\"type\":[\"null\",{\"type\":\"map\",\"values\":{\"type\":\"record\","
+          + 
"\"name\":\"inner\",\"fields\":[{\"name\":\"x\",\"type\":\"int\"}]}}],\"default\":null}]}");
+
   private static Stream<Arguments> testConvertValueToEngineType() {
     return Stream.of(
         Arguments.of(1L, 1L),
@@ -52,15 +72,6 @@ class TestAvroRecordContext {
     assertEquals(expected, actual);
   }
 
-  private static final Schema RECORD_SCHEMA = new Schema.Parser().parse(
-      "{\"type\":\"record\",\"name\":\"top\",\"fields\":["
-          + "{\"name\":\"id\",\"type\":\"int\"},"
-          + 
"{\"name\":\"name\",\"type\":[\"null\",\"string\"],\"default\":null},"
-          + 
"{\"name\":\"address\",\"type\":[\"null\",{\"type\":\"record\",\"name\":\"address\",\"fields\":["
-          + "{\"name\":\"city\",\"type\":\"string\"},"
-          + 
"{\"name\":\"zip\",\"type\":[\"null\",\"int\"],\"default\":null}]}],\"default\":null},"
-          + 
"{\"name\":\"multi\",\"type\":[\"null\",\"string\",\"int\"],\"default\":null}]}");
-
   private static GenericRecord buildRecord() {
     GenericRecord address = new 
GenericData.Record(RECORD_SCHEMA.getField("address").schema().getTypes().get(1));
     address.put("city", new Utf8("sf"));
@@ -94,11 +105,45 @@ class TestAvroRecordContext {
   @Test
   void testGetFieldValueErrorCases() {
     GenericRecord record = buildRecord();
-    // a union that is not [null, T] does not support field lookups
-    assertThrows(IllegalStateException.class, () -> 
getFieldValueFromIndexedRecord(record, "multi.sub"));
+    // a union that is not [null, T] cannot be navigated into; instead of 
throwing, the value
+    // navigator returns null so callers (e.g. column-stats collection) 
degrade gracefully,
+    // matching HoodieAvroUtils.getNestedFieldVal
+    assertNull(getFieldValueFromIndexedRecord(record, "multi.sub"));
+    // an empty field name is still rejected up front
     assertThrows(IllegalArgumentException.class, () -> 
getFieldValueFromIndexedRecord(record, ""));
   }
 
+  @Test
+  void testGetFieldValueMapAndArrayLeavesReturnNull() {
+    GenericRecord record = new GenericData.Record(MAP_AND_ARRAY_SCHEMA);
+    record.put("id", 7);
+    Map<Utf8, Utf8> strMap = new HashMap<>();
+    strMap.put(new Utf8("a"), new Utf8("v1"));
+    record.put("str_map", strMap);
+    record.put("int_array", Arrays.asList(3, 1, 2));
+    // rec_map left null
+
+    // top-level scalar still resolves normally
+    assertEquals(7, getFieldValueFromIndexedRecord(record, "id"));
+
+    // Parquet-style synthetic accessors that traverse a MAP 
(".key_value.key/value") or an
+    // ARRAY (".list.element") cannot be resolved to a single value and must 
return null instead
+    // of throwing. Regression: these previously threw
+    // IllegalStateException "Cannot get field from schema type: MAP" during 
MOR log-append
+    // column-stats collection. Such nested leaves still get statistics from 
the base-file path.
+    assertNull(getFieldValueFromIndexedRecord(record, 
"str_map.key_value.key"));
+    assertNull(getFieldValueFromIndexedRecord(record, 
"str_map.key_value.value"));
+    assertNull(getFieldValueFromIndexedRecord(record, 
"int_array.list.element"));
+    // a deep path descending through a MAP into a record field also degrades 
to null
+    assertNull(getFieldValueFromIndexedRecord(record, 
"rec_map.key_value.value.x"));
+
+    // and null when the complex field itself is absent/null
+    GenericRecord empty = new GenericData.Record(MAP_AND_ARRAY_SCHEMA);
+    empty.put("id", 0);
+    assertNull(getFieldValueFromIndexedRecord(empty, 
"str_map.key_value.value"));
+    assertNull(getFieldValueFromIndexedRecord(empty, 
"int_array.list.element"));
+  }
+
   @Test
   void testGetFieldValueAcrossEqualSchemaInstances() {
     // records from different files carry equal but distinct schema instances; 
both must intern to

Reply via email to