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 2021/03/31 15:21:06 UTC

[GitHub] [parquet-mr] dossett commented on a change in pull request #813: PARQUET-1903: Improve Parquet Protobuf Usability

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



##########
File path: parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoParquetWriter.java
##########
@@ -82,42 +84,70 @@ public ProtoParquetWriter(Path file, Class<? extends Message> protoMessage) thro
     this(file, protoMessage, CompressionCodecName.UNCOMPRESSED,
             DEFAULT_BLOCK_SIZE, DEFAULT_PAGE_SIZE);
   }
-  
-  public static <T> Builder<T> builder(Path file) {
-	    return new Builder<T>(file);
-	}
-
-	public static <T> Builder<T> builder(OutputFile file) {
-	    return new Builder<T>(file);
-	}
-	
-	private static <T extends MessageOrBuilder> WriteSupport<T> writeSupport(Class<? extends Message> protoMessage) {
-		return new ProtoWriteSupport<T>(protoMessage);
-	}
-	  
-	public static class Builder<T> extends ParquetWriter.Builder<T, Builder<T>> {
-		  
-		Class<? extends Message> protoMessage = null;
-
-		private Builder(Path file) {
-			super(file);
-		}
-
-		private Builder(OutputFile file) {
-		    super(file);
-		}
-
-		protected Builder<T> self() {
-		    return this;
-		}
-		
-		public Builder<T> withMessage(Class<? extends Message> protoMessage){
-			this.protoMessage = protoMessage;
-			return this;
-		}
-
-		protected WriteSupport<T> getWriteSupport(Configuration conf) {
-		    return (WriteSupport<T>) ProtoParquetWriter.writeSupport(protoMessage);
-		}    
-	}
+
+  public static <T extends MessageOrBuilder> Builder<T> builder(Path file) {

Review comment:
       @gszadovszky If your concern is that a client could have been successfully using a type parameter that didn't extend MessageOrBuilder I am almost certain that's not the case.  MessageOrBuilder is one of the highest protobuf abstractions and I can't imagine a running use of this that was outside that interface.




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