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

Wu Xiang commented on PARQUET-388:
----------------------------------

One more suggestion, let's just be honest to align the type signature of 
{{ProtoRecordConverter}} class definition and {{getCurrentRecord}} method.

I mean, when we use {{ProtoRecordConverter<MyProtoMessage>}} (or 
ProtoReadSupport<MyProtoMessage>), only MyProtoMessage instances would be 
returned from getCurrentRecord.
On the other hand, when we use {{ProtoRecordConverter<MyProtoMessage.Builder>}} 
or ProtoReadSupport<MyProtoMessage.Builder>, only MyProtoMessage.Builder 
instances returned.

In this way, I would feel less confused about ProtoRecordConverter API.

> ProtoRecordConverter might wrongly cast a Message.Builder to Message
> --------------------------------------------------------------------
>
>                 Key: PARQUET-388
>                 URL: https://issues.apache.org/jira/browse/PARQUET-388
>             Project: Parquet
>          Issue Type: Bug
>          Components: parquet-mr
>            Reporter: Wu Xiang
>            Assignee: Reuben Kuhnert
>
> ProtoRecordConverter returns current record as follows:
> {code}
>   public T getCurrentRecord() {
>     if (buildBefore) {
>       return (T) this.reusedBuilder.build();
>     } else {
>       return (T) this.reusedBuilder;
>     }
>   }
> {code}
> However this might fail if T is subclass of Message and buildBefore == false, 
> since it's actually casting a Message.Builder instance to Message type.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to