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]

Reply via email to