claudevdm commented on code in PR #36892:
URL: https://github.com/apache/beam/pull/36892#discussion_r2582815267
##########
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:
I am keeping it consistent with the current formatters
https://github.com/apache/beam/blob/21109328f725bb9136316455db07507144df719f/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryAvroUtils.java#L170
Formatting it as an iso timestamp will break this convention, is that ok?
##########
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:
I am keeping it consistent with the current format for timestamps
https://github.com/apache/beam/blob/21109328f725bb9136316455db07507144df719f/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryAvroUtils.java#L170
Formatting it as an iso timestamp will break this convention, is that ok?
--
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]