You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by GitBox <gi...@apache.org> on 2022/06/13 08:12:33 UTC

[GitHub] [parquet-mr] guillaume-fetter commented on a diff in pull request #963: PARQUET-1020 Add DynamicMessage writing support

guillaume-fetter commented on code in PR #963:
URL: https://github.com/apache/parquet-mr/pull/963#discussion_r895442880


##########
parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoWriteSupport.java:
##########
@@ -115,27 +120,32 @@ public void prepareForWrite(RecordConsumer recordConsumer) {
   public WriteContext init(Configuration configuration) {
 
     // if no protobuf descriptor was given in constructor, load descriptor from configuration (set with setProtobufClass)
-    if (protoMessage == null) {
-      Class<? extends Message> pbClass = configuration.getClass(PB_CLASS_WRITE, null, Message.class);
-      if (pbClass != null) {
-        protoMessage = pbClass;
-      } else {
-        String msg = "Protocol buffer class not specified.";
-        String hint = " Please use method ProtoParquetOutputFormat.setProtobufClass(...) or other similar method.";
-        throw new BadConfigurationException(msg + hint);
+    if (descriptor == null) {
+      if (protoMessage == null) {
+        Class<? extends Message> pbClass = configuration.getClass(PB_CLASS_WRITE, null, Message.class);
+        if (pbClass != null) {
+          protoMessage = pbClass;
+        } else {
+          String msg = "Protocol buffer class or descriptor not specified.";
+          String hint = " Please use method ProtoParquetOutputFormat.setProtobufClass(...) or other similar method.";
+          throw new BadConfigurationException(msg + hint);
+        }
       }
+      descriptor = Protobufs.getMessageDescriptor(protoMessage);
+    } else {
+      //Assume no specific Message extending class, so use DynamicMessage
+      protoMessage = DynamicMessage.class;

Review Comment:
   Yes I agree. In the end I set it just for the sake of having it set, but you are right it will be more confusing than useful.



-- 
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: dev-unsubscribe@parquet.apache.org

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