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

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

apilloud commented on a change in pull request #11074:
URL: https://github.com/apache/beam/pull/11074#discussion_r833730138



##########
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:
       There was a past world where the getters supported returning either logical type or base type. This was suppose to be whatever the previous transform provided (or the base type if coming from a coder). The goal was to save a conversion from base type to logical type for values that were just passed along. I believe the only use case was SQL and I removed that in #13930 because it is broken and like you notice here more expensive.
   
   It should be possible to do more simplification such that we always work with logical types instead of base types. Eventually we will want a pass-through optimization again, but it will need to be at a lower level than logical types. (The only expensive type today is String, which isn't a logical 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