airborne12 commented on code in PR #67180:
URL: https://github.com/apache/doris/pull/67180#discussion_r3890143401
##########
be/src/storage/segment/segment_iterator.cpp:
##########
@@ -834,12 +835,28 @@ Status
SegmentIterator::_get_row_ranges_by_column_conditions() {
if (_index_query_context != nullptr) {
_index_query_context->count_on_index_fastpath =
_count_on_index_fastpath_safe();
_index_query_context->count_on_index_fastpath_hit = false;
+ // Candidate-pushdown handshake: while index conditions are
+ // evaluated, expose the current candidate bitmap so index
+ // queries can restrict themselves to it (two-phase
+ // evaluation). _row_bitmap only shrinks during the applies
+ // below, so restricting to its current state stays correct
+ // for every later conjunct.
+ _refresh_candidate_pushdown();
}
- DEFER({ _capture_count_fastpath_hit(); });
+ DEFER({
+ _capture_count_fastpath_hit();
+ if (_index_query_context != nullptr) {
+ _index_query_context->candidate_rows = nullptr;
+ }
+ });
// Only apply column-level inverted index if we have iterators
if (has_index_in_iterators()) {
RETURN_IF_ERROR(_apply_inverted_index());
}
+ // The column predicates above may have shrunk the bitmap across
+ // the engage threshold; refresh the handshake at this conjunct
+ // boundary so the expression conjuncts below still benefit.
+ _refresh_candidate_pushdown();
Review Comment:
Verified and fixed with TDD rather than accepting the scenario from code
inspection alone.
RED evidence (ASAN): I added three cold-cache tests covering external
scanner row ranges, a per-segment MoW delete bitmap, and condition-derived page
ranges. Before the fix all 3/3 failed at the same assertion: the expression
observed a null candidate even though each late restriction reduced a 100-row
segment to 5 rows (below the 0.3 engage threshold).
The fix moves those stable restrictions onto the same `_row_bitmap` before
inverted-index expression evaluation. The candidate pointer therefore keeps the
same lifetime and observes the monotonically shrinking bitmap. This does not
create a local/partial candidate object; the final scan domain is unchanged,
and external row ranges remain part of the condition-cache digest.
GREEN evidence: the 3 RED tests now pass; 18/18 related ASAN tests pass
(candidate pushdown, phrase, and phrase-prefix); clang-format v16/check-format
pass; and `./build.sh --be -j 24` succeeds under ASAN.
Normalized decision score: severity 5/10 (performance fallback, no wrong
result), production likelihood 7/10 (MoW deletes/splits/page pruning are
common, gated by phrase-family queries and threshold crossing), scenario
confidence 10/10 (three deterministic reproductions). Arithmetic normalization
= (5 + 7 + 10) / 3 = 7.33/10, above the 6/10 repair threshold, so this was
worth fixing.
Fixed in 4057b24f2ac55ec5e6218220781c5fc5d4b8925d.
--
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]
