apilloud commented on a change in pull request #13751:
URL: https://github.com/apache/beam/pull/13751#discussion_r558549554
##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/util/RowJson.java
##########
@@ -515,7 +537,11 @@ private void writeValue(JsonGenerator gen, FieldType type,
Object value) throws
writeRow((Row) value, type.getRowSchema(), gen);
break;
case LOGICAL_TYPE:
- writeValue(gen, type.getLogicalType().getBaseType(), value);
+ if
(KNOWN_LOGICAL_TYPE_IDENTIFIERS.contains(type.getLogicalType().getIdentifier()))
{
+ gen.writeString(value.toString()); // ISO 8601 format
Review comment:
This happens to work for the current logical types but I think there is
a risk it will subtly break if the underlying type isn't something that
guarantees toString is ISO 8601. How about this pattern:
```
if
(SqlTypes.DATE.getIdentifier().equals(type.getLogicalType().getIdentifier())) {
gen.writeString(((LocalDate) value).toString()); // ISO 8601 format
}
```
(Can you add the cast to 'DATETIME' above as well?)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
