mosche commented on code in PR #17372:
URL: https://github.com/apache/beam/pull/17372#discussion_r856132501
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/values/RowWithGetters.java:
##########
@@ -128,6 +129,14 @@ private <T> T getValue(FieldType type, Object fieldValue,
@Nullable Integer cach
cachedMaps.computeIfAbsent(
cacheKey, i -> getMapValue(type.getMapKeyType(),
type.getMapValueType(), map))
: (T) getMapValue(type.getMapKeyType(), type.getMapValueType(), map);
+ } else if (type.getTypeName().equals(TypeName.DATETIME)) {
+ if (fieldValue instanceof java.time.LocalDate) {
+ Instant instant = java.time.Instant.parse(fieldValue +
"T00:00:00.00Z");
+ return (T) org.joda.time.Instant.ofEpochMilli(instant.toEpochMilli());
+ } else if (fieldValue instanceof java.time.Instant) {
+ return (T) org.joda.time.Instant.ofEpochMilli(((Instant)
fieldValue).toEpochMilli());
+ }
+ return (T) fieldValue;
Review Comment:
@aromanenko-dev I think @TheNeuralBit is right. Based on
[benchmarks](https://github.com/apache/beam/pull/17172#issuecomment-1085981280)
I've done just recently, branching in `RowWithGetters` doesn't perform well. In
https://github.com/apache/beam/pull/17172 I'm suggesting to push all of the
current code down into the getters.
The `GetterBasedSchemaProvider`s (except the Avro one) only support Joda
[ReadableInstant](https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/StaticSchemaInference.java#L144-L145)
(type is the method return type or field type) as `DATETIME`. Attempting to
use java time would most likely fail during schema generation ([or generate a
row schema with nested internal
fields](https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/StaticSchemaInference.java#L166))
For Avro there's already a [conversion
layer](https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroUtils.java#L278-L294)
in place you could leverage for that. For `DATETIME` it's using these
converters:
-
[ConvertValueForGetter.convertDateTime](https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java#L792)
-
[ConvertValueForSetter.convertDateTime](https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java#L1160)
--
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]
