voonhous commented on code in PR #19620:
URL: https://github.com/apache/hudi/pull/19620#discussion_r3793815750
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/avro/TestHoodieAvroWriteSupportShredding.java:
##########
@@ -68,4 +72,43 @@ void forcedShreddingDdlTreatsDecimalParensAsOneField() {
.collect(Collectors.toList());
assertEquals(Arrays.asList("a", "b", "c"), shreddedFieldNames);
}
+
+ /**
+ * Disabling shredding over an already-shredded schema - the
clustering/compaction case
+ * {@link HoodieAvroWriteSupport#generateEffectiveSchema} calls out - has to
strip typed_value
+ * without tripping Avro's "Field already used". Rebuilding a record while
reusing a
+ * {@code Schema.Field} still bound to the source record throws, so any
table with a shredded
+ * variant AND at least one other column failed here. #18938 fixed exactly
that defect in the
+ * sibling HoodieVariantReconstruction and left this twin behind. Nested
variants must be
+ * stripped too, since the row writer shreds at any depth.
Review Comment:
Reworded in 4da5636 to carry the same scoping as the other two comments:
applyForcedShreddingSchema is top-level only, so the nested shredded schema
this strip sees comes from a clustering/compaction schema read back from a file
the ROW path (or another engine) wrote, not from this path's own hook.
##########
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) {
+ // The shredding spec lets a writer omit `value` when every row is typed,
and
+ // HoodieSchema.Variant.determineIfShredded calls anything with a
typed_value shredded. Shape
+ // detection is the only detector that runs on real files, since the
footer strips the logical
+ // type, so requiring `value` here dropped such a column's payload
silently - #19567 again by
+ // another shape. Hudi's own writer always emits three fields, so this is
about files written
+ // elsewhere.
+ HoodieSchema twoFieldShredded = HoodieSchema.createRecord("v",
"org.apache.hudi.test", null, Arrays.asList(
+ HoodieSchemaField.of("metadata",
HoodieSchema.create(HoodieSchemaType.BYTES)),
+ HoodieSchemaField.of("typed_value",
HoodieSchema.createNullable(HoodieSchemaType.INT))));
+ HoodieSchema fileSchema = recordWithIdAndVariant(twoFieldShredded);
+ HoodieSchema requestedSchema =
recordWithIdAndVariant(HoodieSchema.createVariant());
+ HoodieStorage storage = storageWithReadingShredded(tmp, true);
+
storage.getConf().set(HoodieStorageConfig.PARQUET_VARIANT_SHREDDING_PROVIDER_CLASS.key(),
+ TestVariantShreddingProvider.class.getName());
+
+ HoodieVariantReconstruction reconstruction =
HoodieVariantReconstruction.create(
+ fileSchema, requestedSchema, storage);
+ assertNotNull(reconstruction, "A shredded group with no value column must
still engage");
+
+ GenericRecord shredded = new GenericData.Record(
+
reconstruction.intermediateSchema().getField("v").get().schema().getNonNullType().toAvroSchema());
+ shredded.put("metadata", ByteBuffer.wrap(new byte[] {1}));
+ shredded.put("typed_value", 42);
+ GenericRecord input = new
GenericData.Record(reconstruction.intermediateSchema().toAvroSchema());
+ input.put("id", "record-1");
+ input.put("v", shredded);
+
+ GenericRecord variant = (GenericRecord)
reconstruction.reconstruct(input).get(1);
+ assertEquals(ByteBuffer.wrap(new byte[] {42}), variant.get("value"));
+ }
+
+ @Test
+ void ignoresFourFieldStructThatMerelyCarriesTypedValue(@TempDir Path tmp) {
Review Comment:
Right, the four-field case never gets past the count guard, so the no-value
arm was mutation-blind. Added ignoresThreeFieldStructWhoseThirdFieldIsNotValue
with {metadata, typed_value, extra} in 4da5636: it survives the count guard and
pins that arm, so mutating fieldCount == 2 to true now goes red (create would
throw the reading-disabled exception instead of returning null).
--
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]
