exceptionfactory commented on a change in pull request #5014:
URL: https://github.com/apache/nifi/pull/5014#discussion_r673102933
##########
File path:
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/bigquery/PutBigQueryStreaming.java
##########
@@ -191,6 +195,16 @@ public void onTrigger(ProcessContext context,
ProcessSession session) throws Pro
lmapr.add(convertMapRecord(((MapRecord) mapr).toMap()));
}
result.put(key, lmapr);
+ } else if (obj instanceof Timestamp) {
+ LocalDateTime dateTime =
LocalDateTime.ofInstant(Instant.ofEpochMilli(((Timestamp) obj).getTime()),
ZoneOffset.UTC);
+ DateTimeFormatter timestampFormatter =
DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSSSSS");
+ result.put(key, dateTime.format(timestampFormatter));
+ } else if (obj instanceof Time) {
+ LocalDateTime dateTime =
LocalDateTime.ofInstant(Instant.ofEpochMilli(((Time) obj).getTime()),
ZoneOffset.UTC);
+ DateTimeFormatter timeFormatter =
DateTimeFormatter.ofPattern("HH:mm:ss.SSSSSS");
+ result.put(key, dateTime.format(timeFormatter) );
Review comment:
Thanks for the reply, I understand the problem. The `JsonTreeReader` and
other Record Reader implementations perform implicit conversion from the local
system time zone to the GMT time zone when parsing a string to a Timestamp or
Time object. This unfortunately impacts a number of processors, and as you have
found in this class, it ends up requiring setting `ZoneOffset.UTC` when
converting back from a Timestamp or Time to String.
I addressed a similar problem with Date field handling in #5210, so that's
why `java.sql.Date.toString()` works in this class.
Addressing the larger issue with Time and Timestamp in Record Readers needs
to be addressed elsewhere, so for that reason, maintaining the approach you
have implemented seems reasonable for now.
With that background, I would recommend keeping the `DateTimeFormatter`, but
declaring it as a static class variable. Unlike `java.text.SimpleDateFormat`,
`java.time.DateTimeFormatter` is thread-safe, and can be reused, so making it a
static class variable would avoid unnecessary object creation when handling
each record.
It might also be helpful to include a short comment that the ZoneOffset.UTC
time zone is necessary due to implicit time zone conversion in Record Readers.
At least that will be a pointer to address it when evaluating the underlying
issue.
--
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]
