wombatu-kun commented on code in PR #19834:
URL: https://github.com/apache/hudi/pull/19834#discussion_r3947366845
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchemaUtils.java:
##########
@@ -574,7 +574,12 @@ private static HoodieSchema
pruneDataSchemaInternal(HoodieSchema dataSchema, Hoo
return
HoodieSchema.createMap(pruneDataSchema(dataSchema.getValueType(),
requiredSchema.getValueType(), Collections.emptySet()));
case UNION:
- throw new IllegalArgumentException("Data schema is a union");
+ // A union is a leaf as far as pruning goes: Avro resolves a branch by
its type, so dropping a
+ // branch changes the column's type instead of narrowing it. Hand back
the data schema unpruned,
+ // which is what the default arm below already does when Spark
projects a single member out of
+ // the member-struct encoding of a union. This also covers a plain
record whose fields happen to
+ // be named member0..memberN, which HoodieSparkSchemaConverters reads
back as a union.
+ return dataSchema;
Review Comment:
The union arm hands back the whole data union, so if Spark asks for a subset
of the member struct the reader is told to emit more inner fields than
`requestedStructType` declares, which is what `appendPartitionAndProject`
projects from. Is a partial member projection reachable here, or does something
upstream keep the requested struct whole?
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchema.java:
##########
@@ -1379,9 +1379,13 @@ public boolean isSchemaNull() {
}
/**
- * If this is a union schema, returns the non-null type. Otherwise, returns
this schema.
+ * Strips the null branch from a nullable union. For {@code ["null", T]} (in
either order) this returns
+ * {@code T}. For a union with two or more non-null branches it returns a
union of just those branches,
Review Comment:
getNonNullType() also returns `this` for a single-branch union like
["string"], and throws out of createUnion for ["null"] on its own, so neither
outcome is covered by "a union of just those branches, or this schema when
there is no null branch to strip". Worth naming both, since this javadoc is the
contract the three new caller-side UNION re-checks lean on.
##########
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) {
Review Comment:
`getNonNullType()` followed by a re-check for UNION is now spelled out three
times - `findNestedField`, `coerceToComparable` and here - and the two new
throw messages paraphrase the one `HoodieSchemaUtils.resolveUnionSchema`
already carries. `HoodieSchemaUtils`'s own javadoc puts questions about a
single schema on `HoodieSchema`, so a predicate there would give all three
sites one definition; follow-up, not a blocker.
--
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]
