Merge pull request #33014 from vespa-engine/toregge/detect-if-query-o…

…perators-need-ranking

Detect if query operators need ranking.

searchcore/src/tests/proton/matching/query_test.cpp

@@ -1152,6 +1152,53 @@ TEST(QueryTest, global_filter_is_calculated_and_handled)
     }
 }
 
+bool query_needs_ranking(const std::string& stack_dump)
+{
+    Query query;
+    query.buildTree(stack_dump, "", ViewResolver(), plain_index_env);
+    return query.needs_ranking();
+
+}
+
+TEST(QueryTest, normal_term_doesnt_need_ranking)
+{
+    QueryBuilder<ProtonNodeTypes> builder;
+    builder.addStringTerm("xyz", "f1", 1, Weight(1));
+    EXPECT_FALSE(query_needs_ranking(StackDumpCreator::create(*builder.build())));
+}
+
+TEST(QueryTest, weak_and_term_needs_ranking)
+{
+    QueryBuilder<ProtonNodeTypes> builder;
+    builder.addWeakAnd(1, 10, "f1");
+    builder.addStringTerm("xyz", "f1", 1, Weight(1));
+    EXPECT_TRUE(query_needs_ranking(StackDumpCreator::create(*builder.build())));
+}
+
+TEST(QueryTest, weighted_set_term_needs_ranking)
+{
+    QueryBuilder<ProtonNodeTypes> builder;
+    auto& ws = builder.addWeightedSetTerm(1, "f1", 1, Weight(1));
+    ws.addTerm("xyz", Weight(1));
+    EXPECT_TRUE(query_needs_ranking(StackDumpCreator::create(*builder.build())));
+}
+
+TEST(QueryTest, dot_product_term_needs_ranking)
+{
+    QueryBuilder<ProtonNodeTypes> builder;
+    auto& dp = builder.addDotProduct(1, "f1", 1, Weight(1));
+    dp.addTerm("xyz", Weight(1));
+    EXPECT_TRUE(query_needs_ranking(StackDumpCreator::create(*builder.build())));
+}
+
+TEST(QueryTest, wand_term_needs_ranking)
+{
+    QueryBuilder<ProtonNodeTypes> builder;
+    auto& wand = builder.addWandTerm(1, "f1", 1, Weight(1), 10, 0, 1.0);
+    wand.addTerm("xyz", Weight(1));
+    EXPECT_TRUE(query_needs_ranking(StackDumpCreator::create(*builder.build())));
+}
+
 }  // namespace
 }  // namespace proton::matching
 

searchcore/src/vespa/searchcore/proton/matching/matcher.cpp

@@ -80,8 +80,11 @@ numThreads(size_t hits, size_t minHits) {
 
 bool
 willNeedRanking(const SearchRequest & request, const GroupingContext & groupingContext,
-                std::optional<search::feature_t> first_phase_rank_score_drop_limit)
+                std::optional<search::feature_t> first_phase_rank_score_drop_limit, bool query_needs_ranking)
 {
+    if (query_needs_ranking) {
+        return true;
+    }
     return (groupingContext.needRanking() || (request.maxhits != 0))
            && (request.sortSpec.empty() ||
                (request.sortSpec.find("[rank]") != std::string::npos) ||
@@ -281,7 +284,7 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl
         MatchParams params(searchContext.getDocIdLimit(), heapSize, arraySize, first_phase_rank_score_drop_limit,
                            second_phase_rank_score_drop_limit,
                            request.offset, request.maxhits, !_rankSetup->getSecondPhaseRank().empty(),
-                           willNeedRanking(request, groupingContext, first_phase_rank_score_drop_limit));
+                           willNeedRanking(request, groupingContext, first_phase_rank_score_drop_limit, mtf->query().needs_ranking()));
 
         ResultProcessor rp(attrContext, metaStore, sessionMgr, groupingContext, sessionId,
                            request.sortSpec, params.offset, params.hits);

searchcore/src/vespa/searchcore/proton/matching/query.cpp

@@ -12,6 +12,7 @@
 #include <vespa/searchlib/common/geo_location_spec.h>
 #include <vespa/searchlib/engine/trace.h>
 #include <vespa/searchlib/parsequery/stackdumpiterator.h>
+#include <vespa/searchlib/query/tree/templatetermvisitor.h>
 #include <vespa/searchlib/queryeval/intermediate_blueprints.h>
 #include <vespa/vespalib/util/issue.h>
 #include <vespa/vespalib/util/thread_bundle.h>
@@ -32,6 +33,7 @@ using search::fef::MatchDataLayout;
 using search::query::LocationTerm;
 using search::query::Node;
 using search::query::QueryTreeCreator;
+using search::query::TemplateTermVisitor;
 using search::query::Weight;
 using search::queryeval::AndBlueprint;
 using search::queryeval::AndNotBlueprint;
@@ -141,6 +143,27 @@ void exchange_location_nodes(const string &location_str,
     }
 }
 
+/*
+ * WeakAnd, WeightedSetTerm, DotProduct and WandTerm query operators need ranking since
+ * doUnpack is used to updated threshold during query evaluation.
+ */
+class NeedsRankingVisitor : public TemplateTermVisitor<NeedsRankingVisitor, ProtonNodeTypes>
+{
+    bool _needs_ranking;
+public:
+    NeedsRankingVisitor()
+        : TemplateTermVisitor<NeedsRankingVisitor, ProtonNodeTypes>(),
+          _needs_ranking(false)
+    {
+    }
+    template <class TermNode> void visitTerm(TermNode&) { }
+    void visit(ProtonNodeTypes::WeakAnd&) override { _needs_ranking = true; }
+    void visitTerm(ProtonNodeTypes::WeightedSetTerm&) { _needs_ranking = true; }
+    void visitTerm(ProtonNodeTypes::DotProduct&) { _needs_ranking = true; }
+    void visitTerm(ProtonNodeTypes::WandTerm&) { _needs_ranking = true; }
+    bool needs_ranking() const noexcept { return _needs_ranking; }
+};
+
 }  // namespace
 
 Query::Query() = default;
@@ -160,6 +183,9 @@ Query::buildTree(std::string_view stack, const string &location,
         _query_tree = UnpackingIteratorsOptimizer::optimize(std::move(_query_tree), bool(_whiteListBlueprint), always_mark_phrase_expensive);
         ResolveViewVisitor resolve_visitor(resolver, indexEnv);
         _query_tree->accept(resolve_visitor);
+        NeedsRankingVisitor need_ranking_visitor;
+        _query_tree->accept(need_ranking_visitor);
+        _needs_ranking = need_ranking_visitor.needs_ranking();
         return true;
     } else {
         Issue::report("invalid query");

searchcore/src/vespa/searchcore/proton/matching/query.h

@@ -32,6 +32,7 @@ class Query
     Blueprint::UP                _blueprint;
     Blueprint::UP                _whiteListBlueprint;
     std::vector<GeoLocationSpec> _locations;
+    bool                         _needs_ranking = false;
 
 public:
     /** Convenience typedef. */
@@ -148,6 +149,7 @@ class Query
      */
     Blueprint::HitEstimate estimate() const;
     const Blueprint * peekRoot() const { return _blueprint.get(); }
+    bool needs_ranking() const noexcept { return _needs_ranking; }
 };
 
 }