wombatu-kun commented on code in PR #19582:
URL: https://github.com/apache/hudi/pull/19582#discussion_r3763907540
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/HoodieMergeHelper.java:
##########
@@ -93,8 +94,15 @@ public void runMerge(HoodieTable<?, ?, ?, ?> table,
// Check whether the writer schema is simply a projection of the file's
one, ie
// - Its field-set is a proper subset (of the reader schema)
// - There's no schema evolution transformation necessary
+ // Shredded variant columns are aligned to the writer side first: their
footer-derived file
+ // schema surfaces a plain {metadata, value, typed_value} record where the
writer schema has a
+ // variant, which can never pass the strict check even though the reader
reconstructs such
+ // columns when handed a variant-bearing requested schema. Without the
alignment recordSchema
+ // degenerates to the footer schema, reconstruction cannot anchor on a
variant requested
+ // column, and the rewrite below silently drops typed_value (#19567).
boolean isPureProjection = schemaEvolutionTransformerOpt.isEmpty()
- && HoodieSchemaCompatibility.isStrictProjectionOf(readerSchema,
writerSchema);
+ && HoodieSchemaCompatibility.isStrictProjectionOf(
+ VariantSchemaUtils.alignShreddedVariants(readerSchema,
writerSchema), writerSchema);
Review Comment:
The Summary and Risk Level sections still describe detection only, with no
mention that runMerge now flips isPureProjection for shredded variant base
files and skips the writer-schema rewrite - a change on the code path every COW
merge shares. Could you fold that and the residual from the commit message (a
schema-evolution transformer or an added column still takes the lossy rewrite)
into the PR body?
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/schema/TestVariantDataType.scala:
##########
@@ -448,6 +450,77 @@ class TestVariantDataType extends HoodieSparkSqlTestBase {
})
}
+ test("Test COW small-file merge preserves shredded VARIANT values") {
Review Comment:
This has no unshredded twin, so nothing pins that an ordinary variant table
- the layout written without the force-shredding config - still round-trips
through the same bin-pack now that alignShreddedVariants runs on every
runMerge. Could you add one the way the clustering pair above is split?
--
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]
