advancedxy commented on code in PR #956:
URL: https://github.com/apache/iceberg-cpp/pull/956#discussion_r4280796423
##########
src/iceberg/parquet/parquet_reader.cc:
##########
@@ -404,23 +413,28 @@ class ParquetReader::Impl {
Status InitReadContext() {
context_ = std::make_unique<ReadContext>();
- // Row group pruning based on the split
- // TODO(gangwu): add row group filtering based on zone map, bloom filter,
etc.
+ // Row group pruning based on the split and zone map
auto metadata = reader_->parquet_reader()->metadata();
std::vector<int> row_group_indices;
- if (split_.has_value()) {
- for (int i = 0; i < metadata->num_row_groups(); ++i) {
- auto row_group_offset = metadata->RowGroup(i)->file_offset();
- if (row_group_offset >= split_->offset &&
- row_group_offset < split_->offset + split_->length) {
- row_group_indices.push_back(i);
- } else if (row_group_offset >= split_->offset + split_->length) {
+ for (int i = 0; i < metadata->num_row_groups(); ++i) {
+ auto row_group = metadata->RowGroup(i);
+ if (split_.has_value()) {
+ auto row_group_offset = row_group->file_offset();
+ if (std::cmp_greater_equal(row_group_offset, split_->offset +
split_->length)) {
break;
}
+ if (std::cmp_less(row_group_offset, split_->offset)) {
+ continue;
+ }
Review Comment:
how about we simplify the logic a bit: just iterate the row groups, no need
to shortcut when row group already passes the request split length
```cpp
auto row_group_offset = row_group->file_offset();
bool selected = row_group_offset >= split_->offset && row_group_offset <
split_->offset + split_->length;
if (!selected) {
continue;
}
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
