adriangb commented on PR #11223: URL: https://github.com/apache/arrow-rs/pull/11223#issuecomment-5848909459
Automated follow-up (Claude Opus 5.5) to the [review above](https://github.com/apache/arrow-rs/pull/11223#issuecomment-5848201780). The branch is now at `c6ba96e018`. | # | Finding | Result | |---|---|---| | 1 | `batch_size == 0` panics in `Batch` mode | Fixed. `build` returns an error for `Batch` with `batch_size == 0` when the file has rows. `with_batch_size` clamps to the file row count, so an empty file still builds and finishes. `RowGroup` does not change. Tests: `batch_size_zero_is_an_error`, `batch_size_zero_without_rows`. | | 2 | Decode is quadratic in pushed buffers | Fixed in a new first commit, `perf(parquet): keep PushBuffers sorted by offset`. Buffers stay sorted by start offset. Lookups use binary search, and `release_ranges` splices each merged run in place, once per decode step. Push ahead, one buffer per page (debug build): 4000 buffers 269 ms → 18 ms, 8000 1.03 s → 39 ms, 16000 4.08 s → 91 ms. This is now the same cost as push on demand. | | 3 | Round trips per window with predicates | Documented. With a `RowFilter`, a caller that pushes only what `NeedsData` requests pays one round trip per predicate plus one for the output, per window. The docs tell callers on high-latency storage to push ahead. | | 4 | Two inexact doc sentences | Fixed: the emit rule (more than `batch_size` rows ready, or the row group is filtered) and the release rule (the column chunks that the row group reads). | | 5 | Test gap | Added `HETEROGENEOUS_FILE` (28/18/140/35/28/28 pages per column, row groups 700/700/400), `page_boundaries_differ_per_column`, and `randomized_equivalence_heterogeneous_pages` (200 seeds, about 1.7 s in debug), with list, struct and null-returning predicates. The timing and round-count tests are not included: they depend on timing or internals. | | 6 | Move the filtered state machine into `impl Filtered` | Not done in this PR. It is a large move of reviewed code. We can do it in a follow-up if reviewers want it. | Checks: fmt and `clippy --all-targets --all-features -D warnings` on every commit, `cargo test -p parquet --features arrow,async` (0 failures), and `cargo doc` with `-D warnings`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
