Abacn commented on code in PR #23969:
URL: https://github.com/apache/beam/pull/23969#discussion_r1037795510
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroUtils.java:
##########
@@ -772,9 +810,11 @@ private static Schema.FieldType
toFieldType(TypeWithNullability type) {
if (logicalType instanceof LogicalTypes.Decimal) {
fieldType = FieldType.DECIMAL;
} else if (logicalType instanceof LogicalTypes.TimestampMillis) {
- // TODO: There is a desire to move Beam schema DATETIME to a micros
representation. When
- // this is done, this logical type needs to be changed.
+ // TODO(https://github.com/apache/beam/issues/19215) DATETIME
primitive will be removed when
+ // fully migrates to java.time lib from joda-time
fieldType = FieldType.DATETIME;
+ } else if (logicalType instanceof LogicalTypes.TimestampMicros) {
Review Comment:
This piece apparently can introduce breaking change. Previously
TimestampMicros Avro Logical Type will return `FieldType.INT64` and now it
returns `FieldType.logicalType(SqlTypes.TIMESTAMP)`. For modules not yet
support this Beam logical type this will fail existing user code.
--
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]
