lucasfang opened a new pull request, #309: URL: https://github.com/apache/paimon-cpp/pull/309
### Purpose Linked issue: close #308 A page-filtered read resets each leaf with `ColumnReader::ResetLeaf(col_idx, total)`, where `total` is the leaf's whole-row-group size in compressed space, and that one number drives `RecordReader::Reserve(total)`. On a selective predicate that appends a few rows out of a large row group this mis-sizes the Arrow builders both ways: a variable-width (BYTE_ARRAY) leaf reserves its `::arrow::BinaryBuilder` offsets buffer for the entire row group (tens of MiB never written), while the value-data buffer is not reserved at all because `Reserve()` leaves it alone for BYTE_ARRAY readers (`uses_values_` is false), so it doubles once per decoded batch and each doubling copies everything accumulated so far. This PR splits the single `reserve` into `reserve_records` / `reserve_values` / `reserve_value_bytes` and threads them from the format layer into the patched Arrow, so each leaf sizes its builders once for the selection. Output is unchanged — the reservation is only a capacity hint. Changes: - `cmake_modules/arrow.diff`: widen `parquet::arrow::ColumnReader::ResetLeaf` from `(col_idx, reserve)` to `(col_idx, reserve_records, reserve_values, reserve_value_bytes)`, threaded through `LeafReader`, `ExtensionReader`, `ListReader`, and `StructReader`; `LeafReader::ResetLeaf` now calls `Reserve(reserve_records)` plus `ReserveValueBytes(reserve_values, reserve_value_bytes)`. Add the `parquet::RecordReader::ReserveValueBytes(num_values, num_bytes)` virtual (default no-op for fixed-width readers) and a `ByteArrayChunkedRecordReader` override that reserves the builder offsets and data buffer, clamped to `kBinaryMemoryLimit - value_data_length()` to mirror `ArrowBinaryHelper::Prepare`. - `page_filtered_row_group_reader.h` / `.cpp`: `ExecuteSkipReadPattern` gains `reserve_values` / `reserve_value_bytes`; `ReadFilteredField` computes them per leaf — `reserve_values = effective_ranges.RowCount()` and `reserve_value_bytes = min(avg_width * reserve_values, total_uncompressed_size)` from the column chunk, an over-estimate capped by the chunk and guarded by `num_values() > 0 && reserve_values > 0` — and fetches `column_chunk` whenever the selection is non-empty, not only on the page-index path. ### Tests No new test: the change preserves output and the existing `page_filtered_row_group_reader_test.cpp` already covers every path it touches — `StringColumnPageFilter` and `StringColumnMultiBatchStreaming` (multi-batch BYTE_ARRAY selective reads with per-value content assertions, the doubling case `ReserveValueBytes` removes), the nested struct/list/map cases that thread `ResetLeaf` through `StructReader` / `ListReader`, and the dictionary cases. A reservation-size assertion would be brittle and couple to implementation details. Verified against a bundled Arrow re-patched with the updated `arrow.diff` (Release, `-j 96`): - `paimon-parquet-format-test`: 220 tests from 16 suites passed, including all 49 `PageFilteredRowGroupReaderTest` cases. - `paimon-common-test`: 1551 tests from 179 suites passed. ### API and Format No. No header under `include/paimon/` is touched and neither the storage format nor the protocol changes. The only API affected is paimon's own vendored Arrow patch (`cmake_modules/arrow.diff`): the internal `parquet::arrow::ColumnReader::ResetLeaf` signature and the new `parquet::RecordReader::ReserveValueBytes` virtual, neither of which is upstream Arrow's public API. ### Documentation No. This is an internal read-path performance optimization with no user-visible API, configuration, or behavior change. ### Generative AI tooling Generated-by: Qoder -- 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]
