[GitHub] [beam] Abacn commented on a diff in pull request #23969: Support SqlTypes Date and Timestamp (MicrosInstant) in AvroUtils

[GitBox <gi...@apache.org>]

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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org