TheNeuralBit commented on a change in pull request #15485:
URL: https://github.com/apache/beam/pull/15485#discussion_r718894348
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryUtils.java
##########
@@ -551,9 +548,9 @@ public static TableRow toTableRow(Row row) {
return toTableRow((Row) fieldValue);
case DATETIME:
- return ((Instant) fieldValue)
- .toDateTime(DateTimeZone.UTC)
- .toString(BIGQUERY_TIMESTAMP_PRINTER);
+ org.joda.time.Instant jodaInstant = (org.joda.time.Instant) fieldValue;
+ java.time.Instant javaInstant =
java.time.Instant.ofEpochMilli(jodaInstant.getMillis());
+ return BIGQUERY_TIMESTAMP_PRINTER.format(javaInstant);
Review comment:
It's a little messy that we'd need to pass the option as an argument in
`toTableRow`, but I think to do anything cleaner would take a major
re-architecting. We might create another type like ConversionOptions for this.
I have an important note regarding this statement though:
> This should reject, i.e. error out, before any rows are processed, i.e.
before formatFunction is triggered.
It shouldn't error out universally. Instead it should raise an error _if the
timestamp requires nanosecond precision_. That is, the timestamp 0.123456000
would be processed fine, since we can truncate it to microsecond precision
without losing any information. However, the timestamp 0.123456789 would yield
an error at execution time.
Similar to this logic from the read side:
https://github.com/apache/beam/blob/c8568eb372e2e432ce0a9998fa2864c090ddf44a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryUtils.java#L762-L765
--
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]
