sundapeng commented on code in PR #1072:
URL: https://github.com/apache/paimon-rust/pull/1072#discussion_r4216076794
##########
crates/paimon/src/arrow/format/parquet.rs:
##########
@@ -445,7 +445,66 @@ fn parquet_writer_properties(
);
}
}
- Ok(props.build())
+ Ok(parquet_sizing_properties(props, options)?.build())
+}
+
+/// Java's parquet writer sizing keys; `file.block-size` overrides
`parquet.block.size` as in
+/// Java's `ParquetFileFormat`.
+fn parquet_sizing_properties(
+ mut props: parquet::file::properties::WriterPropertiesBuilder,
+ options: &HashMap<String, String>,
+) -> crate::Result<parquet::file::properties::WriterPropertiesBuilder> {
+ const BLOCK_SIZE: &str = "parquet.block.size";
+ const PAGE_SIZE: &str = "parquet.page.size";
+ const DICTIONARY_PAGE_SIZE: &str = "parquet.dictionary.page.size";
+ const PAGE_ROW_COUNT_LIMIT: &str = "parquet.page.row.count.limit";
+ const PAGE_SIZE_ROW_CHECK_MIN: &str = "parquet.page.size.row.check.min";
+ let positive = |key: &str| -> crate::Result<Option<usize>> {
+ options
+ .get(key)
+ .map(|value| {
+ value
+ .trim()
+ .parse::<usize>()
+ .ok()
+ .filter(|parsed| *parsed > 0)
+ .ok_or_else(|| crate::Error::ConfigInvalid {
+ message: format!("Option '{key}' must be a positive
integer, got: {value}"),
+ })
+ })
+ .transpose()
+ };
+ let file_block_size = CoreOptions::new(options)
+ .file_block_size()?
+ .map(|bytes| {
+ usize::try_from(bytes)
+ .ok()
+ .filter(|bytes| *bytes > 0)
+ .ok_or_else(|| crate::Error::ConfigInvalid {
+ message: format!("Option 'file.block-size' must be
positive, got: {bytes}"),
+ })
+ })
+ .transpose()?;
+ if let Some(bytes) = file_block_size.or(positive(BLOCK_SIZE)?) {
+ props = props.set_max_row_group_bytes(Some(bytes));
Review Comment:
Confirmed: an empty row group takes the whole first batch. When a row group
byte limit is set, `ParquetFormatWriter::write` now feeds bounded slices. Each
slice holds at most `limit / in-memory bytes per row` rows; in-memory size is
not smaller than the encoded size, so parquet-rs can check and flush between
slices and merges small slices into one group itself. Fixed in 5b0fecf.
`test_parquet_block_size_bounds_row_groups_of_one_large_batch` writes one
65,536-row batch through `TableWrite` for an append table and a primary-key
table, checks that every row group stays within the limit, then commits and
reads all rows back. Before the change it produced a single row group.
##########
crates/paimon/src/arrow/format/parquet.rs:
##########
@@ -445,7 +445,66 @@ fn parquet_writer_properties(
);
}
}
- Ok(props.build())
+ Ok(parquet_sizing_properties(props, options)?.build())
+}
+
+/// Java's parquet writer sizing keys; `file.block-size` overrides
`parquet.block.size` as in
+/// Java's `ParquetFileFormat`.
+fn parquet_sizing_properties(
+ mut props: parquet::file::properties::WriterPropertiesBuilder,
+ options: &HashMap<String, String>,
+) -> crate::Result<parquet::file::properties::WriterPropertiesBuilder> {
+ const BLOCK_SIZE: &str = "parquet.block.size";
+ const PAGE_SIZE: &str = "parquet.page.size";
+ const DICTIONARY_PAGE_SIZE: &str = "parquet.dictionary.page.size";
+ const PAGE_ROW_COUNT_LIMIT: &str = "parquet.page.row.count.limit";
+ const PAGE_SIZE_ROW_CHECK_MIN: &str = "parquet.page.size.row.check.min";
+ let positive = |key: &str| -> crate::Result<Option<usize>> {
+ options
+ .get(key)
+ .map(|value| {
+ value
+ .trim()
+ .parse::<usize>()
+ .ok()
+ .filter(|parsed| *parsed > 0)
+ .ok_or_else(|| crate::Error::ConfigInvalid {
+ message: format!("Option '{key}' must be a positive
integer, got: {value}"),
+ })
+ })
+ .transpose()
+ };
+ let file_block_size = CoreOptions::new(options)
+ .file_block_size()?
+ .map(|bytes| {
+ usize::try_from(bytes)
+ .ok()
+ .filter(|bytes| *bytes > 0)
+ .ok_or_else(|| crate::Error::ConfigInvalid {
+ message: format!("Option 'file.block-size' must be
positive, got: {bytes}"),
+ })
+ })
+ .transpose()?;
+ if let Some(bytes) = file_block_size.or(positive(BLOCK_SIZE)?) {
Review Comment:
Fixed in 5b0fecf: `parquet.block.size` is parsed only when `file.block-size`
is absent. The `TableWrite` regression above sets `file.block-size=16 kb`
together with a malformed, shadowed `parquet.block.size=128mb`, and writes,
commits and reads; before the change the write failed with `ConfigInvalid`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
