voonhous commented on code in PR #19810:
URL: https://github.com/apache/hudi/pull/19810#discussion_r3923989192
##########
hudi-common/src/test/java/org/apache/hudi/common/schema/TestHoodieSchemaCompatibility.java:
##########
@@ -701,6 +703,120 @@ public void testIsSchemaCompatibleWithTypePromotion() {
assertFalse(HoodieSchemaCompatibility.isSchemaCompatible(longS, intS,
true, true));
}
+ /**
+ * Sibling of {@link #testIsSchemaCompatibleWithTypePromotion()} covering
the rest of the reader/writer type
+ * table: the primitive widening cases shared with {@link
HoodieSchemaTypePromotion}, plus the two
+ * logical-type-over-primitive rules (TIMESTAMP over LONG, UUID over STRING)
that are compatibility-only.
+ *
+ * <p>All pairs are asserted through the 4-arg {@code
isSchemaCompatible(prev = writer, new = reader, true, true)}.</p>
+ */
+ @Test
+ public void testIsSchemaCompatibleWithLogicalTypesAndWidening() {
+ // Logical type over its backing primitive: accepted for reader/writer
compatibility.
+ assertCompatible(HoodieSchema.createTimestampMillis(),
HoodieSchema.create(HoodieSchemaType.LONG));
+ assertCompatible(HoodieSchema.createUUID(),
HoodieSchema.create(HoodieSchemaType.STRING));
+
+ // ... but only in that direction, and only over the matching primitive.
+ assertIncompatible(HoodieSchema.create(HoodieSchemaType.LONG),
HoodieSchema.createTimestampMillis());
+ assertIncompatible(HoodieSchema.createTimestampMillis(),
HoodieSchema.create(HoodieSchemaType.INT));
+ // DATE has no such rule at all, even though it is backed by INT.
+ assertIncompatible(HoodieSchema.createDate(),
HoodieSchema.create(HoodieSchemaType.INT));
+
+ // Primitive widening, delegated to HoodieSchemaTypePromotion.
+ assertCompatible(HoodieSchema.create(HoodieSchemaType.DOUBLE),
HoodieSchema.create(HoodieSchemaType.FLOAT));
+ assertIncompatible(HoodieSchema.create(HoodieSchemaType.FLOAT),
HoodieSchema.create(HoodieSchemaType.DOUBLE));
+ assertCompatible(HoodieSchema.create(HoodieSchemaType.STRING),
HoodieSchema.create(HoodieSchemaType.BYTES));
+ assertCompatible(HoodieSchema.create(HoodieSchemaType.BYTES),
HoodieSchema.create(HoodieSchemaType.STRING));
+ assertCompatible(HoodieSchema.create(HoodieSchemaType.STRING),
HoodieSchema.create(HoodieSchemaType.INT));
+ }
+
+ @Test
+ public void testAreSchemasCompatibleReaderIsFirstArgument() {
+ HoodieSchema longRecord =
singleFieldRecord(HoodieSchema.create(HoodieSchemaType.LONG));
+ HoodieSchema intRecord =
singleFieldRecord(HoodieSchema.create(HoodieSchemaType.INT));
+
+ // A long reader can read int data ...
+ assertTrue(HoodieSchemaCompatibility.areSchemasCompatible(longRecord,
intRecord));
+ // ... but not the other way round, which pins the reader as the FIRST
argument.
+ assertFalse(HoodieSchemaCompatibility.areSchemasCompatible(intRecord,
longRecord));
+ }
+
+ @Test
+ public void testLookupWriterFieldDirectMatch() {
+ HoodieSchemaField readerField = readerFieldWithAlias();
+ HoodieSchema writerSchema =
HoodieSchema.parse("{\"type\":\"record\",\"name\":\"W\",\"fields\":["
+ + "{\"name\":\"a\",\"type\":\"int\"}]}");
+
+ HoodieSchemaField writerField =
HoodieSchemaCompatibility.lookupWriterField(writerSchema, readerField);
+ assertEquals("a", writerField.name());
+ }
+
+ @Test
+ public void testLookupWriterFieldAliasMatch() {
+ HoodieSchemaField readerField = readerFieldWithAlias();
+ HoodieSchema writerSchema =
HoodieSchema.parse("{\"type\":\"record\",\"name\":\"W\",\"fields\":["
+ + "{\"name\":\"old_a\",\"type\":\"int\"}]}");
+
+ HoodieSchemaField writerField =
HoodieSchemaCompatibility.lookupWriterField(writerSchema, readerField);
+ assertEquals("old_a", writerField.name());
+ }
+
+ @Test
+ public void testLookupWriterFieldAmbiguousMatchThrows() {
+ HoodieSchemaField readerField = readerFieldWithAlias();
+ HoodieSchema writerSchema =
HoodieSchema.parse("{\"type\":\"record\",\"name\":\"W\",\"fields\":["
+ + "{\"name\":\"a\",\"type\":\"int\"},"
+ + "{\"name\":\"old_a\",\"type\":\"int\"}]}");
+
+ assertThrows(HoodieSchemaException.class,
+ () -> HoodieSchemaCompatibility.lookupWriterField(writerSchema,
readerField));
+ }
+
+ @Test
+ public void testLookupWriterFieldNoMatchReturnsNull() {
+ HoodieSchemaField readerField = readerFieldWithAlias();
+ HoodieSchema writerSchema =
HoodieSchema.parse("{\"type\":\"record\",\"name\":\"W\",\"fields\":["
+ + "{\"name\":\"unrelated\",\"type\":\"int\"}]}");
+
+ assertNull(HoodieSchemaCompatibility.lookupWriterField(writerSchema,
readerField));
+ }
+
+ @Test
+ public void testLookupWriterFieldRejectsNonRecordWriterSchema() {
+ HoodieSchemaField readerField = readerFieldWithAlias();
+ HoodieSchema notARecord = HoodieSchema.create(HoodieSchemaType.STRING);
+
+ assertThrows(IllegalArgumentException.class,
+ () -> HoodieSchemaCompatibility.lookupWriterField(notARecord,
readerField));
+ }
+
+ /**
+ * Reader field {@code a}, aliased {@code old_a}. Aliases have no builder on
HoodieSchemaField, so the
+ * reader record is parsed from JSON.
+ */
+ private static HoodieSchemaField readerFieldWithAlias() {
+ HoodieSchema readerSchema =
HoodieSchema.parse("{\"type\":\"record\",\"name\":\"R\",\"fields\":["
+ + "{\"name\":\"a\",\"type\":\"int\",\"aliases\":[\"old_a\"]}]}");
+ return readerSchema.getField("a").get();
+ }
+
+ private static void assertCompatible(HoodieSchema readerFieldSchema,
HoodieSchema writerFieldSchema) {
+ assertTrue(HoodieSchemaCompatibility.isSchemaCompatible(
+ singleFieldRecord(writerFieldSchema),
singleFieldRecord(readerFieldSchema), true, true),
+ "reader " + readerFieldSchema + " should read writer " +
writerFieldSchema);
+ }
+
+ private static void assertIncompatible(HoodieSchema readerFieldSchema,
HoodieSchema writerFieldSchema) {
+ assertFalse(HoodieSchemaCompatibility.isSchemaCompatible(
+ singleFieldRecord(writerFieldSchema),
singleFieldRecord(readerFieldSchema), true, true),
+ "reader " + readerFieldSchema + " should not read writer " +
writerFieldSchema);
+ }
+
+ private static HoodieSchema singleFieldRecord(HoodieSchema fieldSchema) {
Review Comment:
Done in 4f9886f810fc: the helper is gone and the four call sites use
`HoodieSchemaTestUtils.createRecord("R", HoodieSchemaField.of("f", ...))`.
##########
hudi-common/src/test/java/org/apache/hudi/common/schema/TestHoodieSchemaTypePromotion.java:
##########
@@ -86,6 +86,11 @@ public void testUnrelatedTypesNotPromotable() {
assertFalse(HoodieSchemaTypePromotion.canPromote(HoodieSchemaType.BOOLEAN,
HoodieSchemaType.INT));
assertFalse(HoodieSchemaTypePromotion.canPromote(HoodieSchemaType.INT,
HoodieSchemaType.BOOLEAN));
assertFalse(HoodieSchemaTypePromotion.canPromote(HoodieSchemaType.LONG,
HoodieSchemaType.STRING));
+ // Logical-type-over-primitive pairs are reader/writer compatibility rules
only (see
+ // HoodieSchemaCompatibilityChecker); they must never be reported as
compatible projections.
+
assertFalse(HoodieSchemaTypePromotion.canPromote(HoodieSchemaType.TIMESTAMP,
HoodieSchemaType.LONG));
+ assertFalse(HoodieSchemaTypePromotion.canPromote(HoodieSchemaType.UUID,
HoodieSchemaType.STRING));
+ assertFalse(HoodieSchemaTypePromotion.canPromote(HoodieSchemaType.DATE,
HoodieSchemaType.INT));
Review Comment:
Added in 4f9886f810fc.
--
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]
