adriangb opened a new pull request, #10930:
URL: https://github.com/apache/arrow-rs/pull/10930
# Which issue does this PR close?
- Closes #10929.
# Rationale for this change
`GenericColumnWriter::add_data_page` flushes the encoder unconditionally.
Its two neighbouring call sites already avoid calling it with an empty page
(`should_add_data_page` returns `false` at zero buffered values, and
`dict_fallback` / `flush_data_pages` test `num_buffered_values > 0`), but
content-defined chunking does not:
```rust
// Add a page break after each chunk except the last
if i + 1 < num_chunks {
match &mut self.writer {
ArrowColumnWriterImpl::Column(c) => c.add_data_page()?,
ArrowColumnWriterImpl::ByteArray(c) => c.add_data_page()?,
}
}
```
Writing the chunk can already have flushed the page, when the chunk's own
values reach `data_page_size_limit` or `data_page_row_count_limit` exactly at
the chunk boundary. The forced break then flushes a page with nothing buffered.
For a `BOOLEAN` column that panics. `RleValueEncoder` creates its inner
encoder lazily on the first `put` and `flush_buffer` does `.take().expect("RLE
value encoder is not initialized")`, so flushing before any `put` panics.
`BOOLEAN` resolves to `RleValueEncoder` under `WriterVersion::PARQUET_2_0`, or
when `Encoding::RLE` is set explicitly.
For other encodings it does not panic but writes a data page holding zero
values.
This is reachable from the public API:
`WriterPropertiesBuilder::set_content_defined_chunking` plus `ArrowWriter`.
`data_page_size_limit` smaller than `max_chunk_size` is a configuration the
`CdcOptions::max_chunk_size` docs explicitly describe as supported.
# What changes are included in this PR?
`add_data_page` returns `Ok(())` when `page_metrics.num_buffered_values ==
0`, making a forced page break a no-op when there is nothing to write. This
brings it in line with the checks the other call sites already make.
# Are these changes tested?
Yes, two regression tests in `parquet/src/arrow/arrow_writer/mod.rs`, both
driving only the public `ArrowWriter` / `WriterProperties` API. Both fail on
`main`:
- `test_arrow_writer_cdc_boolean_forced_page_break_after_flush` writes a
`BOOLEAN` column with CDC and a small `data_page_size_limit`, and checks the
values round-trip. On `main` it panics with `RLE value encoder is not
initialized`.
- `test_arrow_writer_cdc_writes_no_empty_data_pages` writes an `INT32`
column with CDC and `data_page_row_count_limit`, and asserts no data page
reports zero values. On `main` 121 of 611 pages are empty.
# Are there any user-facing changes?
Yes, both are fixes:
- Writing a `BOOLEAN` column with content-defined chunking no longer panics.
- Files written with content-defined chunking no longer contain zero-value
data pages.
No public API change, and no change to files written without content-defined
chunking.
---
*This PR was written by Claude (Anthropic's AI assistant) working with
@adriangb. The tests were run and the results reported above are actual.*
--
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]
