airborne12 commented on code in PR #67180:
URL: https://github.com/apache/doris/pull/67180#discussion_r3889695025
##########
be/src/storage/index/inverted/query/phrase_query.cpp:
##########
@@ -53,6 +53,14 @@ void PhraseQuery::add(const InvertedIndexQueryInfo&
query_info) {
init_ordered_sloppy_phrase_matcher(query_info, is_similarity);
}
+ // Two-phase evaluation with a pushed-down candidate set: the candidate
+ // bitmap joins the leapfrog intersection (restricting doc-list walking and
+ // position verification to candidates) but never a matcher's postings, so
+ // phrase semantics stay with the real term iterators.
+ if (_context->candidate_rows != nullptr) {
+
_iterators.emplace_back(std::make_shared<RoaringDocIdIterator>(_context->candidate_rows));
Review Comment:
Fixed in `f82c48c054d` with a RED→GREEN SegmentIterator test using the real
`VirtualSlotRef -> NOT -> AND` expression tree and nullable inverted-index
bitmaps.
RED on the previous head reproduced both observable failures for candidate
row 0 (`NOT(NULL AND FALSE)`): the FALSE child was not evaluated because the
candidate-restricted TRUE bitmap triggered the AND shortcut, and row 0 was
absent from the final TRUE bitmap. The fix resolves a top-level
`VirtualSlotRef` to its underlying expression before the compound-root
suppression decision. The identical test is GREEN after the fix, and the full
related ASAN filter is 15/15 GREEN.
Normalized triage (arithmetic mean): severity 10/10 (silent wrong result),
scenario confidence 10/10 (deterministic RED), production likelihood 3.5/10
(requires the VirtualSlotRef-wrapped nullable compound shape) => **7.83/10**,
above the 6/10 fix threshold.
##########
be/src/storage/index/inverted/query/phrase_query.cpp:
##########
@@ -53,6 +53,14 @@ void PhraseQuery::add(const InvertedIndexQueryInfo&
query_info) {
init_ordered_sloppy_phrase_matcher(query_info, is_similarity);
}
+ // Two-phase evaluation with a pushed-down candidate set: the candidate
+ // bitmap joins the leapfrog intersection (restricting doc-list walking and
+ // position verification to candidates) but never a matcher's postings, so
+ // phrase semantics stay with the real term iterators.
+ if (_context->candidate_rows != nullptr) {
+
_iterators.emplace_back(std::make_shared<RoaringDocIdIterator>(_context->candidate_rows));
Review Comment:
Fixed in `f82c48c054d`. `_norm_source` is now type-constrained to
`TermPositionsIterator`, so candidate and `UnionTermIterator` remain leapfrog
approximations but can never be selected as the per-document norm owner.
RED was reproduced with a real on-disk CLucene phrase-prefix index: `common`
has df=5, the `rare*` expansion union has df=2, and scoring threw `[E3]
UnionTermIterator does not support scoring`. The final test runs both without a
candidate and with candidate `{0}`; both are GREEN and collect scores after the
fix. The no-candidate RED proves this specific union edge predates candidate
pushdown rather than being introduced by the optimization, but it is still
production-reachable and this PR touched the norm-source invariant, so I fixed
it here.
Normalized triage (arithmetic mean): severity 8.5/10 (scoring query
failure), scenario confidence 10/10 (real-index RED), production likelihood
5/10 (requires scoring multi-term phrase-prefix plus df ordering) =>
**7.83/10**, above the 6/10 fix threshold.
--
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]
