You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/08/18 12:03:41 UTC

[GitHub] [beam] reubenvanammers commented on a change in pull request #14974: [BEAM-12464] Change ProtoSchemaTranslator beam schema creation to match the order for protobufs containing Oneof fields

reubenvanammers commented on a change in pull request #14974:
URL: https://github.com/apache/beam/pull/14974#discussion_r691170567



##########
File path: sdks/java/extensions/protobuf/src/main/java/org/apache/beam/sdk/extensions/protobuf/ProtoSchemaTranslator.java
##########
@@ -172,17 +173,24 @@ static Schema getSchema(Descriptors.Descriptor descriptor) {
             enumIds.putIfAbsent(fieldDescriptor.getName(), fieldDescriptor.getNumber()) == null);
       }
       FieldType oneOfType = FieldType.logicalType(OneOfType.create(subFields, enumIds));
-      fields.add(Field.of(oneofDescriptor.getName(), oneOfType));
+      oneOfFieldLocationMap.put(
+          oneofDescriptor.getFields().get(0).getNumber(),
+          Field.of(oneofDescriptor.getName(), oneOfType));
     }
 
     for (Descriptors.FieldDescriptor fieldDescriptor : descriptor.getFields()) {
-      if (!oneOfFields.contains(fieldDescriptor.getNumber())) {
+      int fieldDescriptorNumber = fieldDescriptor.getNumber();
+      if (!oneOfFields.contains(fieldDescriptorNumber)) {
         // Store proto field number in metadata.
         FieldType fieldType = beamFieldTypeFromProtoField(fieldDescriptor);
         fields.add(
-            withFieldNumber(
-                    Field.of(fieldDescriptor.getName(), fieldType), fieldDescriptor.getNumber())
+            withFieldNumber(Field.of(fieldDescriptor.getName(), fieldType), fieldDescriptorNumber)
                 .withOptions(getFieldOptions(fieldDescriptor)));
+      } else if (oneOfFieldLocationMap.containsKey(fieldDescriptorNumber)) {
+        Field oneOfField = oneOfFieldLocationMap.get(fieldDescriptorNumber);
+        if (oneOfField != null) {

Review comment:
       Yeah, without this check it complains unfortunately, not sure if theres a neater fix for this. 




-- 
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