exceptionfactory commented on a change in pull request #5014:
URL: https://github.com/apache/nifi/pull/5014#discussion_r672623248
##########
File path:
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/bigquery/PutBigQueryStreaming.java
##########
@@ -18,10 +18,14 @@
package org.apache.nifi.processors.gcp.bigquery;
import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.sql.Date;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.time.Instant;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.*;
Review comment:
Asterisk imports should be replaced with explicit classes.
##########
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:
The [Time
type](https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#time_type)
definition indicates that the value is "independent of a specific date and
timezone", so converting the value with `ZoneOffset.UTC` does not seem to match
the type definition. The
[java.time.LocalTime.toString()](https://docs.oracle.com/javase/8/docs/api/java/time/LocalTime.html#toString)
method supports string formatting based on the precision of the original
value, so it looks like this could be simplified to using
`java.sql.Time.toLocalTime().toString()`. That approach would also avoid the
time zone conversion.
```suggestion
final LocalTime localTime = ((Time) obj).toLocalTime();
result.put(key, localTime.toString());
```
##########
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));
Review comment:
Similar to the `java.sql.Date` and `java.sql.Time` fields,
`java.sql.Timestamp` does not necessarily indicate a particular time zone. In
this case, the [Datetime
type](https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#datetime_type)
seems to be a closer approximation of `java.sql.Timestamp`. Converting
`java.sql.Timestamp` to `java.time.LocalDateTime` would allow using the
`toString()` method for formatting while also avoiding the time zone conversion.
```suggestion
final LocalDateTime localDateTime = ((Timestamp)
obj).toLocalDateTime();
result.put(key, localDateTime.toString());
```
--
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]
