[GitHub] [beam] bashir2 commented on a change in pull request #14227: [BEAM-11969] Adds an option for setting row-group size in ParquetIO

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

bashir2 commented on a change in pull request #14227:
URL: https://github.com/apache/beam/pull/14227#discussion_r595232087



##########
File path: sdks/java/io/parquet/src/main/java/org/apache/beam/sdk/io/parquet/ParquetIO.java
##########
@@ -1054,6 +1054,7 @@ public static Sink sink(Schema schema) {
     return new AutoValue_ParquetIO_Sink.Builder()
         .setJsonSchema(schema.toString())
         .setCompressionCodec(CompressionCodecName.SNAPPY)
+        .setRowGroupSize(0)

Review comment:
       There is already a default in `ParquetWriter.Builder` for `rowGroupSize` ([here](https://github.com/apache/parquet-mr/blob/0efbfaa9a7b02bbd2a5912313ca77a9550da8ea1/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java#L357)) if it is not explicitly set in the builder. So I decided to interpret 0 as fallback to that default (see line 1128 below). Do you feel that we need to introduce another default here?

##########
File path: sdks/java/io/parquet/src/main/java/org/apache/beam/sdk/io/parquet/ParquetIO.java
##########
@@ -1097,6 +1102,12 @@ public Sink withConfiguration(Configuration configuration) {
       return toBuilder().setConfiguration(new SerializableConfiguration(configuration)).build();
     }
 
+    /** Specify row-group size; if not set, a default will be used by the underlying writer. */
+    public Sink withRowGroupSize(int rowGroupSize) {
+      checkArgument(rowGroupSize > 0, "rowGroupSize should be positive");

Review comment:
       Done; also added a note about the zero default value in the comment.




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