wombatu-kun commented on code in PR #19834:
URL: https://github.com/apache/hudi/pull/19834#discussion_r3939058118
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchemaRepair.java:
##########
@@ -246,7 +246,7 @@ public static boolean hasTimestampMillisField(HoodieSchema
tableSchema) {
return hasTimestampMillisField(tableSchema.getValueType());
case UNION:
- return hasTimestampMillisField(tableSchema.getNonNullType());
+ return
tableSchema.getTypes().stream().anyMatch(HoodieSchemaRepair::hasTimestampMillisField);
Review Comment:
HoodieFileGroupReaderBasedFileFormat calls HoodieSchemaUtils.pruneDataSchema
on every scan right after this check, and pruneDataSchema still throws "Data
schema is a union" for the same field, so such a table is not readable from
Spark yet. Worth pulling that site into this fix, or is it deliberately out of
scope?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/stats/ValueType.java:
##########
@@ -294,7 +294,12 @@ public static ValueType fromSchema(HoodieSchema schema) {
case UUID:
return ValueType.UUID;
case UNION:
- return fromSchema(schema.getNonNullType());
+ HoodieSchema nonNullSchema = schema.getNonNullType();
+ if (nonNullSchema.getType() == HoodieSchemaType.UNION) {
+ throw new IllegalArgumentException("Unsupported union type " + schema
Review Comment:
The old failure here was a StackOverflowError, which is an Error; an
IllegalArgumentException is caught by
HoodieTableMetadataUtil.readColumnRangeMetadataFrom's catch (Exception)
fallback, so a log file with such a column now silently yields no column stats
for any of its columns instead of failing the job. Is that silent fallback the
intended failure mode?
##########
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:
coerceToComparable returns null for every other type it cannot coerce -
ENUM, MAP, NULL, RECORD, ARRAY and VARIANT all fall through to null - and this
arm throws instead, which drops the whole file's stats rather than just this
column's. Was the throw deliberate, or should the union join that list?
--
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]
