kinolaev commented on code in PR #17196:
URL: https://github.com/apache/iceberg/pull/17196#discussion_r3596434088
##########
core/src/test/java/org/apache/iceberg/avro/TestSchemaConversions.java:
##########
@@ -112,6 +112,53 @@ public void
testAvroToIcebergTimestampTypeWithoutAdjustToUTC() {
assertThat(AvroSchemaUtil.convert(avroType)).isEqualTo(expectedIcebergType);
}
+ @Test
+ public void testTimestampTypesWithLegacyMappingDisabled() {
+ String name = "timestamps";
+
+ Schema ts =
LogicalTypes.localTimestampMicros().addToSchema(Schema.create(Schema.Type.LONG));
+ Schema tsNs =
LogicalTypes.localTimestampNanos().addToSchema(Schema.create(Schema.Type.LONG));
+ Schema tsTz =
LogicalTypes.timestampMicros().addToSchema(Schema.create(Schema.Type.LONG));
+ Schema tsTzNs =
LogicalTypes.timestampNanos().addToSchema(Schema.create(Schema.Type.LONG));
+ Schema avroSchema =
+ record(
+ name,
+ requiredField(0, "ts", ts),
+ requiredField(1, "ts_ns", tsNs),
+ requiredField(2, "ts_tz", tsTz),
+ requiredField(3, "ts_tz_ns", tsTzNs));
+
+ Schema legacyTs =
LogicalTypes.timestampMicros().addToSchema(Schema.create(Schema.Type.LONG));
+ Schema legacyTsNs =
LogicalTypes.timestampNanos().addToSchema(Schema.create(Schema.Type.LONG));
+ Schema legacyTsTz =
LogicalTypes.timestampMicros().addToSchema(Schema.create(Schema.Type.LONG));
+ Schema legacyTsTzNs =
+
LogicalTypes.timestampNanos().addToSchema(Schema.create(Schema.Type.LONG));
+ Schema legacyAvroSchema =
+ record(
+ name,
+ requiredField(0, "ts", addAdjustToUtc(legacyTs, false)),
+ requiredField(1, "ts_ns", addAdjustToUtc(legacyTsNs, false)),
+ requiredField(2, "ts_tz", addAdjustToUtc(legacyTsTz, true)),
+ requiredField(3, "ts_tz_ns", addAdjustToUtc(legacyTsTzNs, true)));
+
+ Types.StructType icebergSchema =
+ Types.StructType.of(
+ required(0, "ts", Types.TimestampType.withoutZone()),
+ required(1, "ts_ns", Types.TimestampNanoType.withoutZone()),
+ required(2, "ts_tz", Types.TimestampType.withZone()),
+ required(3, "ts_tz_ns", Types.TimestampNanoType.withZone()));
+
+ assertThat(AvroSchemaUtil.convert(avroSchema, false))
Review Comment:
I don't agree that the round trip is redundant because Avro timestamps in
`testPrimitiveTypes` and `testTimestampTypesWithLegacyMappingDisabled` have
different schemas.
I've added nano timestamps to `testPrimitiveTypes`. I've also added cases
that are not covered by the round trips for both - legacy and Avro native -
modes to separate tests: `testAvroToIcebergTimestampTypes` and
`testAvroToIcebergTimestampTypesWithLegacyMappingDisabled`. I hope, all
conversions are covered now.
5de86afdedddf2964ff29f2f11c5177589c4aa04
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
