voonhous commented on code in PR #19834:
URL: https://github.com/apache/hudi/pull/19834#discussion_r3940233499
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1382,7 +1382,12 @@ public static Comparable<?>
coerceToComparable(HoodieSchema schema, Object val)
switch (schemaType) {
case UNION:
// TODO we need to handle unions in general case as well
- return coerceToComparable(schema.getNonNullType(), val);
+ HoodieSchema nonNullSchema = schema.getNonNullType();
+ if (nonNullSchema.getType() == HoodieSchemaType.UNION) {
+ throw new HoodieNotSupportedException("Unsupported union type " +
schema
Review Comment:
Neither, in the end -- the column should not reach `coerceToComparable` at
all.
`isColumnTypeSupported`'s deny lists never named `UNION`, which is why a
union column got indexed in the first place. `RECORD`, `MAP` and `ARRAY` are
all filtered there and never reach this switch, which is why they can afford to
be `null` arms. `UNION` now joins them in the V1 and V2 lists, so the column is
skipped up front and the rest of the file keeps its stats.
That is safe for ordinary columns: `isColumnTypeSupported` calls
`getNonNullType()` at entry, so `["null", T]` is already stripped to `T` and
only a union with two or more non-null branches is denied.
The throw stays as a guard for a direct caller. Returning `null` here would
be indistinguishable from a genuinely null min/max.
--
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]
