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