voonhous commented on code in PR #19384:
URL: https://github.com/apache/hudi/pull/19384#discussion_r3673426459
##########
hudi-common/src/test/java/org/apache/hudi/common/schema/internal/utils/TestAvroSchemaEvolutionUtils.java:
##########
@@ -840,4 +841,103 @@ public void testCrossZoneTimestampChangeIsRejected() {
SchemaChangeUtils.parseTimestampLogicalTypeOverrides("ts:local-timestamp-millis")).toAvroSchema();
Assertions.assertEquals("local-timestamp-millis",
stillWorks.getField("ts").schema().getLogicalType().getName());
}
+
+ @Test
+ void testLongToUtcTimestampGatedInBothReconcilePaths() {
+ // Bare long to a UTC timestamp is override-gated exactly like the
local-timestamp case: rejected
+ // without a per-field override and applied with one, in both reconcile
paths. The non-reconcile
+ // guard previously skipped this and let it through silently on the
default write path.
+ HoodieSchema tableBareLong =
HoodieSchema.fromAvroSchema(tripAvro(Schema.create(Schema.Type.LONG)));
+ HoodieSchema incomingMicros =
HoodieSchema.fromAvroSchema(tripAvro(LogicalTypes.timestampMicros().addToSchema(Schema.create(Schema.Type.LONG))));
+
+ // No override: rejected in both paths with the exact actionable error.
+ Map<String, Type> noOverride =
SchemaChangeUtils.parseTimestampLogicalTypeOverrides("");
+ String expectedError =
AvroSchemaEvolutionUtils.timestampPrecisionChangeError(
+ "ts", Types.LongType.get(), Types.TimestampType.get()).getMessage();
+ SchemaCompatibilityException reconcileError =
assertThrows(SchemaCompatibilityException.class,
+ () -> AvroSchemaEvolutionUtils.reconcileSchema(incomingMicros,
tableBareLong, false, noOverride));
+ assertEquals(expectedError, reconcileError.getMessage());
+ SchemaCompatibilityException guardError =
assertThrows(SchemaCompatibilityException.class,
+ () ->
AvroSchemaEvolutionUtils.reconcileTimestampLogicalType(incomingMicros,
tableBareLong, noOverride));
+ assertEquals(expectedError, guardError.getMessage());
+
+ // With the override: the promotion is applied in both paths.
+ Schema viaReconcile =
AvroSchemaEvolutionUtils.reconcileSchema(incomingMicros, tableBareLong, false,
+
SchemaChangeUtils.parseTimestampLogicalTypeOverrides("ts:timestamp-micros")).toAvroSchema();
+ assertEquals("timestamp-micros",
viaReconcile.getField("ts").schema().getLogicalType().getName());
+ Schema viaGuard =
AvroSchemaEvolutionUtils.reconcileTimestampLogicalType(incomingMicros,
tableBareLong,
+
SchemaChangeUtils.parseTimestampLogicalTypeOverrides("ts:timestamp-micros")).toAvroSchema();
+ assertEquals("timestamp-micros",
viaGuard.getField("ts").schema().getLogicalType().getName());
+ }
+
+ @Test
+ void testLongToLocalTimestampGatedInBothReconcilePaths() {
Review Comment:
Test nit:
This one guards #19029's behaviour rather than this PR's. I spliced the two
new test files onto `d6ca44ed1279^` (plus only the `private -> public` line so
they compile) and ran them: `TestAvroSchemaEvolutionUtils` gives `Tests run:
22, Failures: 2` and the two failures are
`testLongToUtcTimestampGatedInBothReconcilePaths:859` and
`testNestedLongToTimestampGated:908`. **This test passes against pre-PR code.**
Its `reconcileSchema` legs also restate what
`testReconcileSchemaTimestampPrecisionEvolution` already asserts at `:693-704`
-- bare long rejected with no override, applied with
`ts:local-timestamp-millis` / `ts:local-timestamp-micros`.
The only genuinely uncovered bit is the `reconcileTimestampLogicalType` leg
for the local case. Please delete this test and add those two lines to the
existing block at `:693-704` instead.
--
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]
