mosche commented on a change in pull request #11074:
URL: https://github.com/apache/beam/pull/11074#discussion_r818731944
##########
File path:
sdks/java/core/src/main/java/org/apache/beam/sdk/values/RowWithGetters.java
##########
@@ -130,10 +130,11 @@ private Iterable getIterableValue(FieldType elementType,
Object fieldValue) {
OneOfType oneOfType = type.getLogicalType(OneOfType.class);
OneOfType.Value oneOfValue = (OneOfType.Value) fieldValue;
Object convertedOneOfField =
- getValue(oneOfValue.getFieldType(), oneOfValue.getValue(), null);
- return (T)
- oneOfType.toBaseType(
- oneOfType.createValue(oneOfValue.getCaseType(),
convertedOneOfField));
+ getValue(oneOfType.getFieldType(oneOfValue),
oneOfValue.getValue(), null);
+ return (T) oneOfType.createValue(oneOfValue.getCaseType(),
convertedOneOfField);
+ } else if (type.getTypeName().isLogicalType()) {
+ // Getters are assumed to return the base type.
+ return (T) type.getLogicalType().toInputType(fieldValue);
Review comment:
@reuvenlax Wondering about handling of logical types here: I don't think
it matters too much as I couldn't find any usage of logical types in the
various `GetterBasedSchemaProvider`s. But I ran into this when testing various
approaches.
When using a logical type with `RowWithGetters`, `getterTarget` will contain
a corresponding field of input type. The getter will have to convert that field
value to base type to match this assumption, just to convert it back to input
type here. And vice versa for the setter [here](
https://github.com/apache/beam/pull/11074/files#diff-d5227cf44e137826953b43dd20f6af2ae579fb1de6e89394db57f0eb76b8d0c7R148).
Wouldn't it be more performant to expect getters to return the input type?
--
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]
