voonhous commented on code in PR #19126:
URL: https://github.com/apache/hudi/pull/19126#discussion_r3629977584
##########
hudi-common/src/test/java/org/apache/hudi/avro/TestAvroRecordContext.java:
##########
@@ -94,11 +97,53 @@ void testGetFieldValueNested() {
@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, ""));
}
+ private static final Schema MAP_AND_ARRAY_SCHEMA = new Schema.Parser().parse(
Review Comment:
nit: `MAP_AND_ARRAY_SCHEMA` sits between two tests; could you move it to the
top of the class with the other constants?
Unrelated: the reworked comment in `testGetFieldValueErrorCases` is a nice
touch, makes the intentional throw -> null change easy to follow.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
