exceptionfactory commented on code in PR #8502:
URL: https://github.com/apache/nifi/pull/8502#discussion_r1546320728
##########
nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/MapRecord.java:
##########
@@ -279,10 +282,27 @@ public Boolean getAsBoolean(final String fieldName) {
}
@Override
- public Date getAsDate(final String fieldName, final String format) {
+ public LocalDate getAsLocalDate(final String fieldName, final String
format) {
final FieldConverter<Object, LocalDate> converter =
StandardFieldConverterRegistry.getRegistry().getFieldConverter(LocalDate.class);
- final LocalDate localDate =
converter.convertField(getValue(fieldName), Optional.ofNullable(format),
fieldName);
- return localDate == null ? null : java.sql.Date.valueOf(localDate);
+ return converter.convertField(getValue(fieldName),
Optional.ofNullable(format), fieldName);
+ }
+
+ @Override
+ public OffsetDateTime getAsOffsetDateTime(final String fieldName, final
String format) {
+ // Support three use-cases:
+ // 1) the datetime string does not contain timezone information,
e.g. `2022-01-01 12:34:56`
+ // 2) the datetime string contains timezone information, e.g.
`Sat, 01 Jan 2022 12:34:56 GMT`
+ // 3) the datetime string contains timezone offset, e.g.
`2022-01-01 12:34:56.789-05:00`
+ try {
+ // First, try to parse the data as if it can be converted to
`LocalDateTime` then set `ZoneOffset.UTC`
+ final FieldConverter<Object, LocalDateTime> localDateTimeConverter
=
StandardFieldConverterRegistry.getRegistry().getFieldConverter(LocalDateTime.class);
+ final LocalDateTime localDateTime =
localDateTimeConverter.convertField(getValue(fieldName),
Optional.ofNullable(format), fieldName);
+ return OffsetDateTime.of(localDateTime, ZoneOffset.UTC);
+ } catch (DateTimeParseException exc) {
+ // Parsing fails if offset is not provided, error: `Unable to
obtain OffsetDateTime from TemporalAccessor`
+ final FieldConverter<Object, OffsetDateTime>
offsetDateTimeConverter =
StandardFieldConverterRegistry.getRegistry().getFieldConverter(OffsetDateTime.class);
+ return offsetDateTimeConverter.convertField(getValue(fieldName),
Optional.ofNullable(format), fieldName);
+ }
Review Comment:
The try-catch approach to parsing is not optimal from a performance
perspective. There is a more general date parsing method that returns a
`TemporalAccessor`, which can be evaluated for various time fields. It looks
like this method might become more complicated in order to support a wider
variety of string formats.
On closer consideration, however, throwing an exception may be more
appropriate. Requesting an `OffsetDateTime` implies that the timezone is
available in source field. Returning an `OffsetDateTime` with `UTC` can create
confusion in calling functions, so this does not seem like the best approach.
--
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]
