wombatu-kun commented on code in PR #19620:
URL: https://github.com/apache/hudi/pull/19620#discussion_r3780557806
##########
hudi-common/src/main/java/org/apache/hudi/common/avro/VariantSchemaUtils.java:
##########
@@ -166,30 +175,44 @@ public static HoodieSchema
toShreddedReadSchema(HoodieSchema requestedSchema, Ho
/**
* Walks {@code base} against its matching {@code other} fields by name,
replacing every shredded
- * variant position with the other side's schema. Recurses through records,
array elements and map
- * values, since the row writer shreds variants at any depth
- * ({@code HoodieRowParquetWriteSupport.processNestedDataType}). {@code
baseIsFile} says which of
- * the two is the file side, which is what {@link #isShreddedVariantTarget}
needs to anchor
- * detection. Returns {@code base} when nothing matches.
+ * variant position with the other side's schema. {@code baseIsFile} says
which of the two is the
+ * file side, which is what {@link #isShreddedVariantTarget} needs to anchor
detection. Returns
+ * {@code base} when nothing matches.
+ *
+ * <p>The walk recurses through records, array elements and map values
because the row writer
+ * shreds at any depth its write schema asks it to
+ * ({@code HoodieRowParquetWriteSupport.processNestedDataType}). Note what
can actually produce
+ * such a file today: the forced-shredding test hook is top-level only in
BOTH write supports
Review Comment:
`generateShreddedSchema` re-reads the forced-shredding DDL on every
recursive entry from `processNestedDataType`, and `makeWriter` hands a nested
shredded struct a real shredded writer, so `struct<v variant>` plus
`hoodie.parquet.variant.force.shredding.schema.for.test` does reach depth
through the row writer. Worth scoping the claim to
`HoodieAvroWriteSupport.applyForcedShreddingSchema` here and in its two copies
(`HoodieVariantReconstruction.buildRebuilder` and the nested reconstruction
test).
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/io/storage/hadoop/TestHoodieVariantReconstruction.java:
##########
@@ -186,6 +186,53 @@ void
returnsNullForFooterDerivedPlainUnshreddedShape(@TempDir Path tmp) {
storageWithReadingShredded(tmp, false)));
}
+ @Test
+ void engagesOnTwoFieldShreddedShapeWithNoValueColumn(@TempDir Path tmp) {
Review Comment:
`TestVariantShreddingProvider` ignores its `shreddedSchema` argument and
reads `metadata`/`typed_value` by name, so this pins detection only - nothing
here exercises `Spark4VariantShreddingProvider.buildVariantSchema` with the
`variantIdx = -1` that a value-less group produces. Worth a value-less case in
`TestHoodieVariantReconstructionRoundTrip`, which is the one test that runs the
real provider through reconstruction.
--
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]
