airborne12 commented on code in PR #68758:
URL: https://github.com/apache/doris/pull/68758#discussion_r4254990205
##########
be/src/exprs/function/search/function_search.cpp:
##########
@@ -355,12 +402,8 @@ Status
FunctionSearch::evaluate_inverted_index_with_search_param_unguarded(
// so score() queries must execute scorers to populate
CollectionSimilarity.
const bool enable_scoring =
index_query_context != nullptr &&
index_query_context->collection_similarity != nullptr;
- // Also bypass the DSL cache when any referenced field is served by an
SNII reader.
auto* dsl_cache =
- enable_cache && !enable_scoring &&
-
!referenced_fields_contain_snii_reader(search_param.root, iterators)
- ? InvertedIndexQueryCache::instance()
- : nullptr;
+ enable_cache && !enable_scoring ?
InvertedIndexQueryCache::instance() : nullptr;
Review Comment:
Confirmed and fixed in a48a4b6b6b3. The DSL cache signature now ends with
the effective `inverted_index_max_expansions`, read through the same
`max_expansions()` helper the expansion uses (50 without a runtime state), so a
result is reused only under the limit it was computed with.
`FunctionSearchTest.TestSearchDslCacheKeysExpansionsByTheirLimit` runs `fo*`
on a cached segment holding `foa` and `fob` with limits 1, 2 and 1 and expects
one, two and one rows. Before the fix the second run returned the cached single
row.
##########
build-support/check-header-deps.py:
##########
@@ -579,6 +608,18 @@ def load_includes():
return includes
+def hub_headers(hub, includes):
Review Comment:
Fixed in 06be24721cd. The self-test now injects `#include <CLucene.h>` into
`query/spi/index_source.h`, a header covered only through the
`storage/index/inverted/query/` directory entry, and expects exit 1 with
`query/spi/index_source.h must not include <CLucene.h>`. The directory-covered
layering rule was already exercised by the existing `query/spi` injection; the
directory angle ban was not.
Writing this case showed that the directory banned `<CLucene/...>` but not
the umbrella `<CLucene.h>`: the injection passed the gate. The ban now covers
it. `build-support/tests/test-build-hygiene-header-deps.sh` fails without the
new ban and passes with it, and `check-build-hygiene.sh` passes.
##########
be/src/storage/index/inverted/query/engine/boolean_query/operator_boolean_weight.h:
##########
@@ -149,127 +192,156 @@ class OperatorBooleanWeight : public Weight {
return std::dynamic_pointer_cast<DoNothingCombiner>(_score_combiner)
!= nullptr;
}
- struct EvalResult {
- roaring::Roaring true_bitmap;
- roaring::Roaring null_bitmap;
- };
-
- static EvalResult collect_eval_result(ScorerPtr scorer, const
NullBitmapResolver* resolver) {
- EvalResult result;
- if (!scorer) {
- return result;
- }
-
- uint32_t doc = scorer->doc();
- if (doc == TERMINATED) {
- doc = scorer->advance();
- }
- while (doc != TERMINATED) {
- result.true_bitmap.add(doc);
- doc = scorer->advance();
- }
-
- if (scorer->has_null_bitmap(resolver)) {
- const auto* bitmap = scorer->get_null_bitmap(resolver);
- if (bitmap != nullptr) {
- result.null_bitmap = *bitmap;
+ // Groups term clauses by source for batched reads, unscored intersections
or scored unions.
+ // Remaining clauses use their individual scorers.
+ std::vector<ListedTerms> listed_terms(const QueryExecutionContext&
context, bool scoring) {
+ const bool any_source = (_type == OperatorType::OP_AND && !scoring) ||
+ (_type == OperatorType::OP_OR && scoring);
Review Comment:
Confirmed and fixed in d43168868be. A TopK SEARCH now marks its execution
context (`QueryExecutionContext::top_k`). In that case a scored OR lists its
terms only on sources that batch their reads; on the others the terms stream
through the per-term union again, so a `LIMIT 1` no longer holds every matching
row and score. Full collection keeps the listing.
`ListedTermsTest.ATopKDisjunctionStreamsTheTermsOfAStreamingSource` uses a
source that does not batch reads: full collection opens one posting at a time,
while the TopK path must keep both postings open together (streaming) and
produce the same scores.
`FunctionSearchTest.TestSearchTopKStreamsAScoredDisjunction` runs `body:foo OR
body:bar` through SEARCH with scoring and LIMIT 1 on such a source and expects
the best row with both postings streamed. Both failed before the fix, which
listed the terms one at a time.
--
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]