You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "pritampan (via GitHub)" <gi...@apache.org> on 2026/09/10 16:40:57 UTC

[PR] Parquet: parse row-group size as long [iceberg]

pritampan opened a new pull request, #18055:
URL: https://github.com/apache/iceberg/pull/18055

   Closes #18054
   
   ## What
   
   Parse `write.parquet.row-group-size-bytes` and `write.delete.parquet.row-group-size-bytes` with `propertyAsLong` and thread row-group size as `long` through the private write path.
   
   `PARQUET_ROW_GROUP_SIZE_BYTES_DEFAULT` stays `int` (128 MiB). Larger sizes are set via the string table property. `Parquet.concat(..., int)` is unchanged.
   
   Iceberg's `ParquetWriter` already takes `long rowGroupSize`; this only unblocks the table-property parse.
   
   ## Test
   
   - [x] `./gradlew :iceberg-parquet:test --tests org.apache.iceberg.parquet.TestParquet.testRowGroupSizeLargerThanIntegerMax --tests org.apache.iceberg.parquet.TestParquet.testRowGroupSizeConfigurable --tests org.apache.iceberg.parquet.TestParquet.testRowGroupSizeConfigurableWithWriter`
   - [x] `./gradlew :iceberg-parquet:spotlessCheck`
   
   AI-assisted: used a coding agent to port the parse change and unit test onto current `main`.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] Parquet: parse row-group size as long [iceberg]

Posted by "pritampan (via GitHub)" <gi...@apache.org>.
pritampan commented on code in PR #18055:
URL: https://github.com/apache/iceberg/pull/18055#discussion_r3983773318


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquet.java:
##########
@@ -105,6 +105,27 @@ public void testRowGroupSizeConfigurable() throws IOException {
     }
   }
 
+  @Test
+  public void testRowGroupSizeLargerThanIntegerMax() throws IOException {
+    Schema schema = new Schema(optional(1, "intCol", IntegerType.get()));
+    org.apache.avro.Schema avroSchema = AvroSchemaUtil.convert(schema.asStruct());
+    GenericData.Record record = new GenericData.Record(avroSchema);
+    record.put("intCol", 1);
+
+    // Values above Integer.MAX_VALUE used to fail in PropertyUtil.propertyAsInt.
+    File file = createTempFile(temp);
+    write(
+        file,
+        schema,
+        ImmutableMap.of(PARQUET_ROW_GROUP_SIZE_BYTES, Long.toString(4L * 1024 * 1024 * 1024)),

Review Comment:
   added the test as well



-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] Parquet: parse row-group size as long [iceberg]

Posted by "pritampan (via GitHub)" <gi...@apache.org>.
pritampan commented on PR #18055:
URL: https://github.com/apache/iceberg/pull/18055#issuecomment-5661298598

   > @pritampan this looks correct, but it seems like this was less of practical issue and more of a theoretical issue, right? I'm assuming you didn't run into this issue because you're trying to make 2GB+ row groups?
   
   Thank you @danielcweeks for merging the PR, Yes, context is I am working on automated process to tune Parquet file to improve efficiency in terms of READ/ WRITE performance and storage, while doing that setting 2 GiB RG failed the process, hence created the pr. 


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] Parquet: parse row-group size as long [iceberg]

Posted by "pritampan (via GitHub)" <gi...@apache.org>.
pritampan commented on PR #18055:
URL: https://github.com/apache/iceberg/pull/18055#issuecomment-5625838198

   Thanks for the review. Renamed the new test and added coverage for `write.delete.parquet.row-group-size-bytes`.
   
   Agreed on the operational side: 512 MiB–1 GiB is a more practical row-group target. This change only lifts the `Integer.MAX_VALUE` parse ceiling; the default stays 128 MiB.
   
   Related work: I have been experimenting with Parquet layout on my own, and row-group size is one of the levers. I sometimes try a somewhat larger group than the default; hitting the `int` parse limit is what motivated this fix.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] Parquet: parse row-group size as long [iceberg]

Posted by "pritampan (via GitHub)" <gi...@apache.org>.
pritampan commented on code in PR #18055:
URL: https://github.com/apache/iceberg/pull/18055#discussion_r3983768455


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquet.java:
##########
@@ -105,6 +105,27 @@ public void testRowGroupSizeConfigurable() throws IOException {
     }
   }
 
+  @Test
+  public void testRowGroupSizeLargerThanIntegerMax() throws IOException {

Review Comment:
   thanks, Renamed the new test



-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] Parquet: parse row-group size as long [iceberg]

Posted by "yangshangqing95 (via GitHub)" <gi...@apache.org>.
yangshangqing95 commented on code in PR #18055:
URL: https://github.com/apache/iceberg/pull/18055#discussion_r3983428405


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquet.java:
##########
@@ -105,6 +105,27 @@ public void testRowGroupSizeConfigurable() throws IOException {
     }
   }
 
+  @Test
+  public void testRowGroupSizeLargerThanIntegerMax() throws IOException {

Review Comment:
   Nit, non-blocking:
   based on the latest develop guidance, new added test should without the `test` prefix.
   void `rowGroupSizeLargerThanIntegerMax()`



-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] Parquet: parse row-group size as long [iceberg]

Posted by "yangshangqing95 (via GitHub)" <gi...@apache.org>.
yangshangqing95 commented on code in PR #18055:
URL: https://github.com/apache/iceberg/pull/18055#discussion_r3983510206


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquet.java:
##########
@@ -105,6 +105,27 @@ public void testRowGroupSizeConfigurable() throws IOException {
     }
   }
 
+  @Test
+  public void testRowGroupSizeLargerThanIntegerMax() throws IOException {
+    Schema schema = new Schema(optional(1, "intCol", IntegerType.get()));
+    org.apache.avro.Schema avroSchema = AvroSchemaUtil.convert(schema.asStruct());
+    GenericData.Record record = new GenericData.Record(avroSchema);
+    record.put("intCol", 1);
+
+    // Values above Integer.MAX_VALUE used to fail in PropertyUtil.propertyAsInt.
+    File file = createTempFile(temp);
+    write(
+        file,
+        schema,
+        ImmutableMap.of(PARQUET_ROW_GROUP_SIZE_BYTES, Long.toString(4L * 1024 * 1024 * 1024)),

Review Comment:
   Nit, non-blocking:
   can we also add a test about
   `DELETE_PARQUET_ROW_GROUP_SIZE_BYTES` ?



-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] Parquet: parse row-group size as long [iceberg]

Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks merged PR #18055:
URL: https://github.com/apache/iceberg/pull/18055


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org