[GitHub] [parquet-mr] dossett commented on a change in pull request #820: PARQUET-1917 Don't write values for oneOf fields that aren't set

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

dossett commented on a change in pull request #820:
URL: https://github.com/apache/parquet-mr/pull/820#discussion_r498250403



##########
File path: parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoWriteSupport.java
##########
@@ -337,6 +337,12 @@ private void writeAllFields(MessageOrBuilder pb) {
         List<FieldDescriptor> fieldDescriptors = messageDescriptor.getFields();
         for (FieldDescriptor fieldDescriptor : fieldDescriptors) {
           FieldDescriptor.Type type = fieldDescriptor.getType();
+
+          //For a field in a oneOf that isn't set don't write anything
+          if (fieldDescriptor.getContainingOneof() != null && !pb.hasField(fieldDescriptor)) {
+            continue;
+          }

Review comment:
       @belugabehr That's a great question.  Everything seems ok when I tested it out locally. I wrote some data out to a parquet file with this change, read the data back into protobuf messages, and then wrote the data back to a second parquet file.  The two parquet files were identical, so everything round tripped successfully.  
   
   I am struggling a bit to write unit tests for this case, but that's probably me getting up the parquet API learning curve.




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

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