amuletxheart commented on a change in pull request #15485:
URL: https://github.com/apache/beam/pull/15485#discussion_r715944187
##########
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:
@pabloem @TheNeuralBit This part was missing from my earlier attempt as
I did not fully understand the codebase at that point in time. The TableRow
just contained a String in the Java Instant format, and not the native BigQuery
TIMESTAMP format as it was skipping past all the if statements. I apologize for
this slip-up.
As for whether to reject or truncate nanos to micros, I think we should
follow the pattern established by other SQL types and do the truncation
implicitly. Anyway, this commit will be an improvement as the previous SQL
DATETIME used org.joda.time.Instant, which is limited to microseconds
precision. The switch to java.time.Instant will improve it to nanos precision,
and then truncating to micros at the last step when formatting to String for
TableRow to accept.
Regarding @TheNeuralBit suggestion to allow the option for selecting, I
think it will increase the scope of the ticket, which is originally to solve
the Protobuf -> BigQuery pipeline for google.protobuf.Timestamp fields. We can
open another ticket to figure out what options are open to user configuration,
and how best to pass it to the existing control flow.
--
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]