lxy-9602 commented on code in PR #166:
URL: https://github.com/apache/paimon-cpp/pull/166#discussion_r3702625941
##########
src/paimon/format/parquet/column_index_filter.h:
##########
Review Comment:
Please adjust comments.
##########
src/paimon/format/parquet/page_filtered_row_group_reader.h:
##########
@@ -56,20 +56,24 @@ class PageFilteredRowGroupReader {
/// and only waits via WhenBuffered (no redundant PreBuffer).
/// @param page_ranges If non-empty, wait via WhenBufferedRanges instead
of WhenBuffered
/// @param max_chunksize Per-batch row cap for the returned reader.
+ /// @param row_group_page_index_reader Reusable page-index reader for the
target row group
/// @param arrow_file_reader The Arrow FileReader for ColumnReader tree
creation
/// @return A RecordBatchReader streaming the filtered rows.
static Result<std::unique_ptr<arrow::RecordBatchReader>>
ReadFilteredRowGroup(
const TargetRowGroup& target_row_group, const std::vector<int32_t>&
column_indices,
const ::arrow::io::CacheOptions& cache_options, bool pre_buffered,
const std::vector<::arrow::io::ReadRange>& page_ranges, int64_t
max_chunksize,
- std::shared_ptr<::arrow::MemoryPool> pool,
::parquet::arrow::FileReader* arrow_file_reader);
+ std::shared_ptr<::arrow::MemoryPool> pool,
+ std::shared_ptr<::parquet::RowGroupPageIndexReader>
row_group_page_index_reader,
+ ::parquet::arrow::FileReader* arrow_file_reader);
Review Comment:
I’m a bit curious whether `row_group_page_index_reader` and
`arrow_file_reader` are both intended to be output parameters. If so, should we
make them consistent and use `shared_ptr` for both before putting them into the
pool?
--
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]
