This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new eaf21ed4d4 fix: Reject 0 for `write_batch_size`,
`data_page_row_count_limit` (#10660)
eaf21ed4d4 is described below
commit eaf21ed4d4740c019148f436e89cd928963777a0
Author: Neil Conway <[email protected]>
AuthorDate: Wed Aug 12 21:22:06 2026 -0400
fix: Reject 0 for `write_batch_size`, `data_page_row_count_limit` (#10660)
# Which issue does this PR close?
- Closes #10659
# Rationale for this change
Zero is not a sensible value for either parameter, and allowing a zero
value results in an infinite loop when writing out chunks in
`GenericColumnWriter::write_batch_internal()`.
# What changes are included in this PR?
* `set_write_batch_size` and `set_data_page_row_count_limit` now panic
if they are passed zeroes. This is consistent with how other similar
functions handle invalid input (e.g., `set_max_row_group_row_count`,
`set_max_row_group_bytes`), although it could potentially be improved
(e.g., return `Result`).
* Add unit tests
* Add assert to `GenericColumnWriter::write_batch_internal()` that we've
selected a non-zero `base_batch_size`.
# Are these changes tested?
Yes, and new tests added.
# Are there any user-facing changes?
No, aside from corner-cases where the user was passing a zero to either
of these functions and then not triggering the buggy behavior.
---
parquet/src/column/writer/mod.rs | 3 +++
parquet/src/file/properties.rs | 20 ++++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/parquet/src/column/writer/mod.rs b/parquet/src/column/writer/mod.rs
index c98469cf7c..52daecf75f 100644
--- a/parquet/src/column/writer/mod.rs
+++ b/parquet/src/column/writer/mod.rs
@@ -593,6 +593,7 @@ impl<'a, E: ColumnValueEncoder> GenericColumnWriter<'a, E> {
&& !matches!(rep_levels, LevelDataRef::Materialized(_));
let has_levels = !matches!(def_levels, LevelDataRef::Absent)
|| !matches!(rep_levels, LevelDataRef::Absent);
+
// When both level vectors are compact (Uniform or Absent), there is no
// materialized slice to split and the per-mini-batch work is O(1), so
we
// can safely use a much larger batch size.
@@ -601,6 +602,8 @@ impl<'a, E: ColumnValueEncoder> GenericColumnWriter<'a, E> {
} else {
self.props.write_batch_size()
};
+ debug_assert!(base_batch_size > 0);
+
let chunker = ByteBudgetChunker::new(&self.descr, &self.props,
base_batch_size);
while levels_offset < num_levels {
let mut end_offset = num_levels.min(levels_offset +
base_batch_size);
diff --git a/parquet/src/file/properties.rs b/parquet/src/file/properties.rs
index ac6adedd57..78b1ee9950 100644
--- a/parquet/src/file/properties.rs
+++ b/parquet/src/file/properties.rs
@@ -706,7 +706,11 @@ impl WriterPropertiesBuilder {
///
/// Note: this is a best effort limit based on value of
/// [`set_write_batch_size`](Self::set_write_batch_size).
+ ///
+ /// # Panics
+ /// If the value is `0`.
pub fn set_data_page_row_count_limit(mut self, value: usize) -> Self {
+ assert_ne!(value, 0, "Cannot have a 0 data page row count limit");
self.data_page_row_count_limit = value;
self
}
@@ -720,7 +724,11 @@ impl WriterPropertiesBuilder {
/// [`set_data_page_row_count_limit`](Self::set_data_page_row_count_limit)
/// are checked between batches, and thus the write batch size value acts
as an
/// upper-bound on the enforcement granularity of other limits.
+ ///
+ /// # Panics
+ /// If the value is `0`.
pub fn set_write_batch_size(mut self, value: usize) -> Self {
+ assert_ne!(value, 0, "Cannot have a 0 write batch size");
self.write_batch_size = value;
self
}
@@ -2090,6 +2098,18 @@ mod tests {
let _ = WriterProperties::builder().set_max_row_group_bytes(Some(0));
}
+ #[test]
+ #[should_panic(expected = "Cannot have a 0 write batch size")]
+ fn test_writer_properties_panic_on_zero_write_batch_size() {
+ let _ = WriterProperties::builder().set_write_batch_size(0);
+ }
+
+ #[test]
+ #[should_panic(expected = "Cannot have a 0 data page row count limit")]
+ fn test_writer_properties_panic_on_zero_data_page_row_count_limit() {
+ let _ = WriterProperties::builder().set_data_page_row_count_limit(0);
+ }
+
#[test]
fn test_writer_properties_bloom_filter_ndv_fpp_set() {
assert_eq!(
