github-actions[bot] commented on code in PR #65496:
URL: https://github.com/apache/doris/pull/65496#discussion_r3566208490
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -624,8 +629,10 @@ void TableReader::_finalize_reader_condition_cache() {
_condition_cache_ctx = nullptr;
return;
}
- segment_v2::ConditionCache::instance()->insert(_condition_cache_key,
-
std::move(_condition_cache));
+ DORIS_CHECK(_condition_cache_ctx->num_granules <=
_condition_cache->size());
+ _condition_cache->resize(_condition_cache_ctx->num_granules);
+ segment_v2::ConditionCache::instance()->insert(
+ _condition_cache_key, std::move(_condition_cache),
_condition_cache_ctx->base_granule);
Review Comment:
`_current_reader_reached_eof` is not enough to prove this MISS bitmap is
complete. If `FileScannerV2::try_stop()` flips `io_ctx->should_stop` while the
physical reader is in `get_block()`, both Parquet and ORC can convert that stop
into `Status::OK` with `eof=true`; TableReader then sets
`_current_reader_reached_eof = true` and reaches this insert even though unread
granules in the MISS bitmap are still false.
A later HIT can treat those false bits as authoritative and skip rows.
Please distinguish stop/cancel EOF from real physical EOF before publishing the
condition-cache entry, or have the readers return a stop status that leaves
`_current_reader_reached_eof` false.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -889,14 +889,28 @@ void
OrcReader::set_condition_cache_context(std::shared_ptr<ConditionCacheContex
DORIS_CHECK(_state != nullptr);
_state->condition_cache_ctx = std::move(ctx);
if (_state->condition_cache_ctx != nullptr &&
- _state->condition_cache_ctx->filter_result != nullptr) {
+ _state->condition_cache_ctx->filter_result != nullptr &&
+ !_state->condition_cache_ctx->is_hit) {
_state->condition_cache_ctx->base_granule = static_cast<int64_t>(
_state->row_reader_range_first_row /
ConditionCacheContext::GRANULE_SIZE);
+ const auto end_granule =
+ (_state->row_reader_range_first_row +
_state->row_reader_range_rows +
+ ConditionCacheContext::GRANULE_SIZE - 1) /
+ ConditionCacheContext::GRANULE_SIZE;
+ DORIS_CHECK(end_granule >
static_cast<uint64_t>(_state->condition_cache_ctx->base_granule));
+ _state->condition_cache_ctx->num_granules =
Review Comment:
`set_condition_cache_context()` still freezes the MISS bitmap span from only
the row reader that is open at this point, but ORC SARG pruning can keep
multiple non-adjacent stripe ranges. `open()` applies only
`selected_stripe_ranges[0]` before TableReader initializes the cache context;
later `_advance_to_next_stripe_range()` creates row readers for the remaining
ranges without extending `num_granules`.
For a completed MISS that reads stripe 0 and then a later non-adjacent
selected range, marks for the later range are outside this first-range bitmap
span or get trimmed before insert. HIT-side filtering is conservative outside
the stored span, so this should not lose rows, but completed MISS scans cannot
cache/prune those later ranges. Please derive the MISS
`base_granule`/`num_granules` from the full selected ORC span, or extend the
context as each selected range is advanced, and cover a non-adjacent
selected-stripe case in the test.
--
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]
