[GitHub] [beam] mosche commented on a change in pull request #11074: Store logical type values in Row instead of base values

[GitBox <gi...@apache.org>]

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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org