ahmedabu98 commented on code in PR #36892:
URL: https://github.com/apache/beam/pull/36892#discussion_r2582945183


##########
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/schemas/utils/AvroUtils.java:
##########
@@ -1186,6 +1196,9 @@ private static org.apache.avro.Schema getFieldSchema(
         } else if (SqlTypes.TIMESTAMP.getIdentifier().equals(identifier)) {
           baseType =
               
LogicalTypes.timestampMicros().addToSchema(org.apache.avro.Schema.create(Type.LONG));
+        } else if (Timestamp.IDENTIFIER.equals(identifier)) {
+          baseType = org.apache.avro.Schema.create(Type.LONG);
+          baseType.addProp("logicalType", TIMESTAMP_NANOS_LOGICAL_TYPE);

Review Comment:
   But it's still possible to use `Timestamp.MICROS` (precision = 6) right? 
Shouldn't that lead to `baseType = LogicalTypes.timestampMicros()` ?



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryAvroUtils.java:
##########
@@ -161,36 +159,73 @@ static Schema getPrimitiveType(TableFieldSchema schema, 
Boolean useAvroLogicalTy
    * Formats BigQuery seconds-since-epoch into String matching JSON export. 
Thread-safe and
    * immutable.
    */
-  private static final DateTimeFormatter DATE_AND_SECONDS_FORMATTER =
-      DateTimeFormat.forPattern("yyyy-MM-dd HH:mm:ss").withZoneUTC();
-
-  @VisibleForTesting
-  static String formatTimestamp(Long timestampMicro) {
-    String dateTime = formatDatetime(timestampMicro);
-    return dateTime + " UTC";
+  private static final java.time.format.DateTimeFormatter DATE_TIME_FORMATTER =
+      java.time.format.DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss")
+          .withZone(java.time.ZoneOffset.UTC);
+
+  /** Enum to define the precision of a timestamp since the epoch. */
+  enum TimestampPrecision {
+    MILLISECONDS,
+    MICROSECONDS,
+    NANOSECONDS;
+
+    /** Converts an epoch value of this precision to an Instant. */
+    java.time.Instant toInstant(long epochValue) {
+      switch (this) {
+        case MILLISECONDS:
+          return java.time.Instant.ofEpochMilli(epochValue);
+        case MICROSECONDS:
+          {
+            long seconds = Math.floorDiv(epochValue, 1_000_000L);
+            long microsOfSecond = Math.floorMod(epochValue, 1_000_000L);
+            return java.time.Instant.ofEpochSecond(seconds, microsOfSecond * 
1_000L);
+          }
+        case NANOSECONDS:
+          {
+            long seconds = Math.floorDiv(epochValue, 1_000_000_000L);
+            long nanosOfSecond = Math.floorMod(epochValue, 1_000_000_000L);
+            return java.time.Instant.ofEpochSecond(seconds, nanosOfSecond);
+          }
+        default:
+          throw new IllegalStateException("Unknown precision: " + this);
+      }
+    }
   }
 
+  /**
+   * Formats an Instant with minimal fractional second precision. Shows 0, 3, 
6, or 9 decimal places
+   * based on actual precision of the value.
+   */
   @VisibleForTesting
-  static String formatDatetime(Long timestampMicro) {
-    // timestampMicro is in "microseconds since epoch" format,
-    // e.g., 1452062291123456L means "2016-01-06 06:38:11.123456 UTC".
-    // Separate into seconds and microseconds.
-    long timestampSec = timestampMicro / 1_000_000;
-    long micros = timestampMicro % 1_000_000;
-    if (micros < 0) {
-      micros += 1_000_000;
-      timestampSec -= 1;
-    }
-    String dayAndTime = DATE_AND_SECONDS_FORMATTER.print(timestampSec * 1000);
-    if (micros == 0) {
-      return dayAndTime;
-    } else if (micros % 1000 == 0) {
-      return String.format("%s.%03d", dayAndTime, micros / 1000);
+  @SuppressWarnings("JavaInstantGetSecondsGetNano")
+  static String formatDatetime(java.time.Instant instant) {
+    String dateTime = DATE_TIME_FORMATTER.format(instant);
+    int nanos = instant.getNano();
+
+    if (nanos == 0) {
+      return dateTime;
+    } else if (nanos % 1_000_000 == 0) {
+      return dateTime + String.format(".%03d", nanos / 1_000_000);
+    } else if (nanos % 1_000 == 0) {
+      return dateTime + String.format(".%06d", nanos / 1_000);
     } else {
-      return String.format("%s.%06d", dayAndTime, micros);
+      return dateTime + String.format(".%09d", nanos);
     }
   }
 
+  @VisibleForTesting
+  static String formatDatetime(long epochValue, TimestampPrecision precision) {
+    return formatDatetime(precision.toInstant(epochValue));
+  }
+
+  static String formatTimestamp(java.time.Instant instant) {
+    return formatDatetime(instant) + " UTC";
+  }
+
+  static String formatTimestamp(long epochValue, TimestampPrecision precision) 
{
+    return formatTimestamp(precision.toInstant(epochValue));
+  }
+

Review Comment:
   not very ideal the way we're doing it, but maybe it's not in scope of this 
PR to do that cleanup. we can do it later



-- 
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]

Reply via email to