[Enhancement] Filter row group using runtime filter before prepare next row group in FileReader #54868
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: trueeyu <lxhhust350@qq.com>
![starrocks-cr[bot]](https://avatars.githubusercontent.com/in/404334?s=60&v=4){kind=small}
| RETURN_IF_ERROR(cur_row_group->prepare()); | ||
| } | ||
| break; | ||
| } while (true); | ||
|
|
||
| return Status::OK(); | ||
| } |
There was a problem hiding this comment.
The most risky bug in this code is:
An infinite loop may occur when _row_group_size is 0 or _cur_row_group_idx exceeds _row_group_size.
You can modify the code like this:
Status FileReader::get_next(ChunkPtr* chunk) {
while (_cur_row_group_idx < _row_group_size) { // Add condition check here
const auto& cur_row_group = _row_group_readers[_cur_row_group_idx];
auto ret = _update_rf_and_filter_group(cur_row_group);
if (ret.ok() && ret.value()) {
_group_reader_param.stats->parquet_filtered_row_groups += 1;
_row_group_readers[_cur_row_group_idx] = nullptr; // Ensure the current reader is nullified after filtering
_cur_row_group_idx++;
continue;
} else if (ret.status().is_end_of_file()) {
_group_reader_param.stats->parquet_filtered_row_groups +=
(_row_group_size - _cur_row_group_idx);
_row_group_readers.assign(_row_group_readers.size(), nullptr);
_cur_row_group_idx = _row_group_size;
break;
} else {
// do nothing, ignore the error code
}
RETURN_IF_ERROR(cur_row_group->prepare());
break;
}
return Status::OK();
}There was a problem hiding this comment.
This is not one good suggestion
dirtysalt
reviewed
Jan 9, 2025
| @@ -321,6 +321,8 @@ struct HdfsScannerContext { | |||
|
|
|||
| int64_t connector_max_split_size = 0; | |||
|
|
|||
| OlapRuntimeScanRangePruner* rf_scan_range_pruner = nullptr; | |||
There was a problem hiding this comment.
we definitely have to remove "Olap"
There was a problem hiding this comment.
Good suggestion, i will remove it next pr.
dirtysalt
previously approved these changes
Jan 9, 2025
meegoo
previously approved these changes
Jan 10, 2025
Signed-off-by: trueeyu <lxhhust350@qq.com>
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]✅ pass : 29 / 34 (85.29%) file detail
|
dirtysalt
approved these changes
Jan 10, 2025
satanson
approved these changes
Jan 13, 2025
wyb
approved these changes
Jan 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why I'm doing:
If one
FileReaderscan multi row groups, if runtime filter is updated, theFilterReaderwill use the out of date runtime filter to filter next row group, so the filtering effect is not good.What I'm doing:
Try to filter row group using the newest runtime filter before prepare next row group.
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: