[jira] [Commented] (PARQUET-1917) [parquet-proto] default values are stored in oneOf fields that aren't set

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/PARQUET-1917?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17205516#comment-17205516 ] 

ASF GitHub Bot commented on PARQUET-1917:
-----------------------------------------

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


> [parquet-proto] default values are stored in oneOf fields that aren't set
> -------------------------------------------------------------------------
>
>                 Key: PARQUET-1917
>                 URL: https://issues.apache.org/jira/browse/PARQUET-1917
>             Project: Parquet
>          Issue Type: Bug
>    Affects Versions: 1.12.0
>            Reporter: Aaron Blake Niskode-Dossett
>            Priority: Major
>
> SCHEMA
> --------
> {noformat}
> message Person {
>   int32 foo = 1;
>   oneof optional_bar {
>     int32 bar_int = 200;
>     int32 bar_int2 = 201;
>     string bar_string = 300;
>   }
> }{noformat}
>  
> CODE
> --------
> I set values for foo and bar_string
>  
> {noformat}
> for (int i = 0; i < 3; i += 1) {
>                 com.etsy.grpcparquet.Person message = Person.newBuilder()
>                         .setFoo(i)
>                         .setBarString("hello world")
>                         .build();
>                 message.writeDelimitedTo(out);
>             }{noformat}
> And then I write the protobuf file out to parquet.
>  
> RESULT
> -----------
> {noformat}
> $ parquet-tools show example.parquet                                                                                                                        
> +-------+-----------+------------+--------------+
> |   foo |   bar_int |   bar_int2 | bar_string   |
> |-------+-----------+------------+--------------|
> |     0 |         0 |          0 | hello world  |
> |     1 |         0 |          0 | hello world  |
> |     2 |         0 |          0 | hello world  |
> +-------+-----------+------------+--------------+{noformat}
>  
> bar_int and bar_int2 should be EMPTY for all three rows since only bar_string is set in the oneof.  0 is the default value for int, but it should not be stored.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)