voonhous commented on code in PR #19403:
URL: https://github.com/apache/hudi/pull/19403#discussion_r3689102856
##########
hudi-spark-datasource/hudi-spark4-common/src/test/java/org/apache/hudi/variant/TestSpark4VariantShreddingProvider.java:
##########
@@ -100,15 +244,111 @@ void objectRoundTrips() throws Exception {
assertRoundTrips("{\"a\":\"x\",\"b\":5}",
HoodieSchema.createVariantShreddedObject(shreddedFields));
}
+ @Test
+ void partialObjectShreddingKeepsExtraFieldsInResidual() throws Exception {
+ // Shredded schema declares {a, b} but the variant provides {a, c}: "a"
shreds into typed_value,
+ // "b" is absent (null value + null typed_value), and the extra "c" lands
in the residual value.
+ Map<String, HoodieSchema> shreddedFields = new LinkedHashMap<>();
+ shreddedFields.put("a", HoodieSchema.create(HoodieSchemaType.STRING));
+ shreddedFields.put("b", HoodieSchema.create(HoodieSchemaType.LONG));
+ HoodieSchema.Variant shredded =
HoodieSchema.createVariantShreddedObject(shreddedFields);
+
+ Variant variant = VariantBuilder.parseJson("{\"a\":\"x\",\"c\":99}",
false);
+ GenericRecord shreddedRecord = shred(variant, shredded);
+
+ // The unmatched field forces a non-null residual value at the top level.
+ assertNotNull(shreddedRecord.get(VARIANT_VALUE_FIELD), "extra field must
be captured in residual value");
+ GenericRecord typedValue = (GenericRecord)
shreddedRecord.get(VARIANT_TYPED_VALUE_FIELD);
+ GenericRecord bField = (GenericRecord) typedValue.get("b");
+ assertNull(bField.get(VARIANT_VALUE_FIELD), "absent field b carries no
residual value");
+ assertNull(bField.get(VARIANT_TYPED_VALUE_FIELD), "absent field b carries
no typed_value");
+
+ assertEquals(variant.toJson(ZoneOffset.UTC), rebuild(shreddedRecord,
shredded).toJson(ZoneOffset.UTC));
+ }
+
@Test
void arrayRoundTrips() throws Exception {
// typed_value for an array is array<{value, typed_value}>: each element
is itself a shredded struct.
HoodieSchema element = HoodieSchema.createRecord("v_array_element",
"org.apache.hudi.test", null, Arrays.asList(
- HoodieSchemaField.of(HoodieSchema.Variant.VARIANT_VALUE_FIELD,
HoodieSchema.createNullable(HoodieSchemaType.BYTES)),
- HoodieSchemaField.of(HoodieSchema.Variant.VARIANT_TYPED_VALUE_FIELD,
HoodieSchema.create(HoodieSchemaType.LONG))));
+ HoodieSchemaField.of(VARIANT_VALUE_FIELD,
HoodieSchema.createNullable(HoodieSchemaType.BYTES)),
+ HoodieSchemaField.of(VARIANT_TYPED_VALUE_FIELD,
HoodieSchema.create(HoodieSchemaType.LONG))));
assertScalarRoundTrips("[1,2,3]", HoodieSchema.createArray(element));
}
+ //
---------------------------------------------------------------------------
+ // Decimal reconstruction from the on-disk (avro-decoded) encodings a
parquet reader produces:
+ // the shred path emits a BigDecimal, but a base file feeds rebuild a
ByteBuffer / GenericFixed.
+ //
---------------------------------------------------------------------------
+
+ @Test
+ void rebuildDecimalFromBytesEncoding() {
+ assertDecimalRebuildsFromEncoding(HoodieSchema.createDecimal(10, 2),
false);
+ }
+
+ @Test
+ void rebuildDecimalFromFixedEncoding() {
+ assertDecimalRebuildsFromEncoding(
+ HoodieSchema.createDecimal("dec_fixed", "org.apache.hudi.test", null,
10, 2, 8), true);
+ }
+
+ private void assertDecimalRebuildsFromEncoding(HoodieSchema decimalType,
boolean fixed) {
Review Comment:
Addressed: parameter dropped; the encoding is derived from
`decimalType.getAvroSchema().getType() == FIXED`.
##########
hudi-spark-datasource/hudi-spark4-common/src/test/java/org/apache/hudi/variant/TestSpark4VariantShreddingProvider.java:
##########
@@ -100,15 +244,111 @@ void objectRoundTrips() throws Exception {
assertRoundTrips("{\"a\":\"x\",\"b\":5}",
HoodieSchema.createVariantShreddedObject(shreddedFields));
}
+ @Test
+ void partialObjectShreddingKeepsExtraFieldsInResidual() throws Exception {
+ // Shredded schema declares {a, b} but the variant provides {a, c}: "a"
shreds into typed_value,
+ // "b" is absent (null value + null typed_value), and the extra "c" lands
in the residual value.
+ Map<String, HoodieSchema> shreddedFields = new LinkedHashMap<>();
+ shreddedFields.put("a", HoodieSchema.create(HoodieSchemaType.STRING));
+ shreddedFields.put("b", HoodieSchema.create(HoodieSchemaType.LONG));
+ HoodieSchema.Variant shredded =
HoodieSchema.createVariantShreddedObject(shreddedFields);
+
+ Variant variant = VariantBuilder.parseJson("{\"a\":\"x\",\"c\":99}",
false);
+ GenericRecord shreddedRecord = shred(variant, shredded);
+
+ // The unmatched field forces a non-null residual value at the top level.
+ assertNotNull(shreddedRecord.get(VARIANT_VALUE_FIELD), "extra field must
be captured in residual value");
+ GenericRecord typedValue = (GenericRecord)
shreddedRecord.get(VARIANT_TYPED_VALUE_FIELD);
+ GenericRecord bField = (GenericRecord) typedValue.get("b");
+ assertNull(bField.get(VARIANT_VALUE_FIELD), "absent field b carries no
residual value");
+ assertNull(bField.get(VARIANT_TYPED_VALUE_FIELD), "absent field b carries
no typed_value");
+
+ assertEquals(variant.toJson(ZoneOffset.UTC), rebuild(shreddedRecord,
shredded).toJson(ZoneOffset.UTC));
+ }
+
@Test
void arrayRoundTrips() throws Exception {
// typed_value for an array is array<{value, typed_value}>: each element
is itself a shredded struct.
HoodieSchema element = HoodieSchema.createRecord("v_array_element",
"org.apache.hudi.test", null, Arrays.asList(
- HoodieSchemaField.of(HoodieSchema.Variant.VARIANT_VALUE_FIELD,
HoodieSchema.createNullable(HoodieSchemaType.BYTES)),
- HoodieSchemaField.of(HoodieSchema.Variant.VARIANT_TYPED_VALUE_FIELD,
HoodieSchema.create(HoodieSchemaType.LONG))));
+ HoodieSchemaField.of(VARIANT_VALUE_FIELD,
HoodieSchema.createNullable(HoodieSchemaType.BYTES)),
+ HoodieSchemaField.of(VARIANT_TYPED_VALUE_FIELD,
HoodieSchema.create(HoodieSchemaType.LONG))));
assertScalarRoundTrips("[1,2,3]", HoodieSchema.createArray(element));
}
+ //
---------------------------------------------------------------------------
+ // Decimal reconstruction from the on-disk (avro-decoded) encodings a
parquet reader produces:
+ // the shred path emits a BigDecimal, but a base file feeds rebuild a
ByteBuffer / GenericFixed.
+ //
---------------------------------------------------------------------------
+
+ @Test
+ void rebuildDecimalFromBytesEncoding() {
+ assertDecimalRebuildsFromEncoding(HoodieSchema.createDecimal(10, 2),
false);
+ }
+
+ @Test
+ void rebuildDecimalFromFixedEncoding() {
+ assertDecimalRebuildsFromEncoding(
+ HoodieSchema.createDecimal("dec_fixed", "org.apache.hudi.test", null,
10, 2, 8), true);
+ }
+
+ private void assertDecimalRebuildsFromEncoding(HoodieSchema decimalType,
boolean fixed) {
+ BigDecimal value = new BigDecimal("123.45");
+ HoodieSchema.Variant shredded =
HoodieSchema.createVariantShredded(decimalType);
+ GenericRecord shreddedRecord = shred(scalar(b -> b.appendDecimal(value)),
shredded);
+
+ Schema tvSchema =
shredded.getAvroSchema().getField(VARIANT_TYPED_VALUE_FIELD).schema();
+ Conversions.DecimalConversion conversion = new
Conversions.DecimalConversion();
+ Object encoded = fixed
+ ? conversion.toFixed(value, tvSchema, tvSchema.getLogicalType())
+ : conversion.toBytes(value, tvSchema, tvSchema.getLogicalType());
+ shreddedRecord.put(VARIANT_TYPED_VALUE_FIELD, encoded);
+
+ Variant original = scalar(b -> b.appendDecimal(value));
+ assertEquals(original.toJson(ZoneOffset.UTC), rebuild(shreddedRecord,
shredded).toJson(ZoneOffset.UTC));
+ }
+
+ @Test
+ void rebuildDecimalRejectsUnexpectedEncoding() {
+ HoodieSchema.Variant shredded =
HoodieSchema.createVariantShredded(HoodieSchema.createDecimal(10, 2));
+ GenericRecord shreddedRecord = shred(scalar(b -> b.appendDecimal(new
BigDecimal("1.00"))), shredded);
+ shreddedRecord.put(VARIANT_TYPED_VALUE_FIELD, "not-a-decimal");
+ assertThrows(IllegalStateException.class,
+ () -> provider.rebuildVariantRecord(shreddedRecord,
shredded.getAvroSchema(), unshreddedSchema));
+ }
+
+ //
---------------------------------------------------------------------------
+ // Null / error guards.
+ //
---------------------------------------------------------------------------
+
+ @Test
+ void shredReturnsNullWhenValueOrMetadataMissing() {
+ HoodieSchema.Variant shredded =
HoodieSchema.createVariantShredded(HoodieSchema.create(HoodieSchemaType.LONG));
+ Variant variant = scalar(b -> b.appendLong(1));
+
+ GenericRecord missingValue = unshredded(variant);
+ missingValue.put(VARIANT_VALUE_FIELD, null);
+ assertNull(provider.shredVariantRecord(missingValue,
shredded.getAvroSchema(), shredded));
+
+ GenericRecord missingMetadata = unshredded(variant);
+ missingMetadata.put(VARIANT_METADATA_FIELD, null);
+ assertNull(provider.shredVariantRecord(missingMetadata,
shredded.getAvroSchema(), shredded));
+ }
+
+ @Test
+ void rebuildReturnsNullForNullRecord() {
Review Comment:
Kept, with a comment marking it as defensive-guard coverage only.
--
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]
