Merge pull request #28723 from vespa-engine/balder/lift-out-single-leaf-iterators-from-ws

Lift out single iterators if they are leafs and tfmd is not needed.

4 files changed, 73 insertions, 36 deletions

diff --git a/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp b/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp
index de5fc1a0c05..bcde2f22a07 100644
--- a/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp
+++ b/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp
@@ -147,23 +147,31 @@ struct MockFixture {
 
 } // namespace <unnamed>
 
-void run_simple(bool field_is_filter, bool term_is_not_needed)
+void run_simple(bool field_is_filter, bool term_is_not_needed, bool singleTerm)
 {
     FakeSearchable index;
     setupFakeSearchable(index);
     FakeResult expect;
     if (field_is_filter || term_is_not_needed) {
-        expect.doc(3)
-            .doc(5)
-            .doc(7);
+        expect.doc(3);
+        if ( ! singleTerm) {
+            expect.doc(5)
+                  .doc(7);
+        }
     } else {
-        expect.doc(3).elem(0).weight(30).pos(0)
-            .doc(5).elem(0).weight(50).pos(0)
-            .doc(7).elem(0).weight(70).pos(0);
+        expect.doc(3).elem(0).weight(30).pos(0);
+        if (!singleTerm) {
+            expect.doc(5).elem(0).weight(50).pos(0)
+                  .doc(7).elem(0).weight(70).pos(0);
+        }
     }
     WS ws;
-    ws.add("7", 70).add("5", 50).add("3", 30).add("100", 1000)
-      .set_field_is_filter(field_is_filter)
+    if (singleTerm) {
+        ws.add("3", 30);
+    } else {
+        ws.add("7", 70).add("5", 50).add("3", 30).add("100", 1000);
+    }
+    ws.set_field_is_filter(field_is_filter)
       .set_term_is_not_needed(term_is_not_needed);
 
     EXPECT_TRUE(ws.isGenericSearch(index, "field", true));
@@ -178,15 +186,35 @@ void run_simple(bool field_is_filter, bool term_is_not_needed)
 }
 
 TEST("testSimple") {
-    TEST_DO(run_simple(false, false));
+    TEST_DO(run_simple(false, false, false));
 }
 
 TEST("testSimple filter field") {
-    TEST_DO(run_simple(true, false));
+    TEST_DO(run_simple(true, false, false));
 }
 
 TEST("testSimple unranked") {
-    TEST_DO(run_simple(false, true));
+    TEST_DO(run_simple(false, true, false));
+}
+
+TEST("testSimple unranked filter filed") {
+    TEST_DO(run_simple(true, true, false));
+}
+
+TEST("testSimple single") {
+    TEST_DO(run_simple(false, false, true));
+}
+
+TEST("testSimple single filter field") {
+    TEST_DO(run_simple(true, false, true));
+}
+
+TEST("testSimple single unranked") {
+    TEST_DO(run_simple(false, true, true));
+}
+
+TEST("testSimple single unranked filter field") {
+    TEST_DO(run_simple(true, true, true));
 }
 
 void run_multi(bool field_is_filter, bool term_is_not_needed)
diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.h b/searchlib/src/vespa/searchlib/queryeval/blueprint.h
index 8d230b6ec01..3dd20d73373 100644
--- a/searchlib/src/vespa/searchlib/queryeval/blueprint.h
+++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.h
@@ -26,6 +26,7 @@ namespace search::queryeval {
 class SearchIterator;
 class ExecuteInfo;
 class MatchingElementsSearch;
+class LeafBlueprint;
 
 /**
  * A Blueprint is an intermediate representation of a search. More
@@ -251,6 +252,7 @@ public:
     virtual bool isEquiv() const { return false; }
     virtual bool isWhiteList() const { return false; }
     virtual bool isIntermediate() const { return false; }
+    virtual LeafBlueprint * asLeaf() noexcept { return nullptr; }
     virtual bool isAnd() const { return false; }
     virtual bool isAndNot() const { return false; }
     virtual bool isOr() const { return false; }
@@ -396,6 +398,7 @@ public:
     void fetchPostings(const ExecuteInfo &execInfo) override;
     void freeze() final;
     SearchIteratorUP createSearch(fef::MatchData &md, bool strict) const override;
+    LeafBlueprint * asLeaf() noexcept final { return this; }
 
     virtual bool getRange(vespalib::string & from, vespalib::string & to) const;
     virtual SearchIteratorUP createLeafSearch(const fef::TermFieldMatchDataArray &tfmda, bool strict) const = 0;
diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp
index 8e7bd185f85..3c32f715484 100644
--- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp
@@ -91,7 +91,7 @@ Blueprint::HitEstimate
 AndNotBlueprint::combine(const std::vector<HitEstimate> &data) const
 {
     if (data.empty()) {
-        return HitEstimate();
+        return {};
     }
     return data[0];
 }
@@ -99,7 +99,7 @@ AndNotBlueprint::combine(const std::vector<HitEstimate> &data) const
 FieldSpecBaseList
 AndNotBlueprint::exposeFields() const
 {
-    return FieldSpecBaseList();
+    return {};
 }
 
 void
@@ -132,7 +132,7 @@ AndNotBlueprint::get_replacement()
     if (childCnt() == 1) {
         return removeChild(0);
     }
-    return Blueprint::UP();
+    return {};
 }
 
 void
@@ -187,7 +187,7 @@ AndBlueprint::combine(const std::vector<HitEstimate> &data) const
 FieldSpecBaseList
 AndBlueprint::exposeFields() const
 {
-    return FieldSpecBaseList();
+    return {};
 }
 
 void
@@ -213,7 +213,7 @@ AndBlueprint::get_replacement()
     if (childCnt() == 1) {
         return removeChild(0);
     }
-    return Blueprint::UP();
+    return {};
 }
 
 void
@@ -304,7 +304,7 @@ OrBlueprint::get_replacement()
     if (childCnt() == 1) {
         return removeChild(0);
     }
-    return Blueprint::UP();
+    return {};
 }
 
 void
@@ -361,7 +361,7 @@ WeakAndBlueprint::combine(const std::vector<HitEstimate> &data) const
 FieldSpecBaseList
 WeakAndBlueprint::exposeFields() const
 {
-    return FieldSpecBaseList();
+    return {};
 }
 
 void
@@ -391,9 +391,9 @@ WeakAndBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches,
     assert(_weights.size() == childCnt());
     for (size_t i = 0; i < sub_searches.size(); ++i) {
         // TODO: pass ownership with unique_ptr
-        terms.push_back(wand::Term(sub_searches[i].release(),
-                                   _weights[i],
-                                   getChild(i).getState().estimate().estHits));
+        terms.emplace_back(sub_searches[i].release(),
+                           _weights[i],
+                           getChild(i).getState().estimate().estHits);
     }
     return WeakAndSearch::create(terms, _n, strict);
 }
@@ -415,7 +415,7 @@ NearBlueprint::combine(const std::vector<HitEstimate> &data) const
 FieldSpecBaseList
 NearBlueprint::exposeFields() const
 {
-    return FieldSpecBaseList();
+    return {};
 }
 
 void
@@ -468,7 +468,7 @@ ONearBlueprint::combine(const std::vector<HitEstimate> &data) const
 FieldSpecBaseList
 ONearBlueprint::exposeFields() const
 {
-    return FieldSpecBaseList();
+    return {};
 }
 
 void
@@ -519,7 +519,7 @@ Blueprint::HitEstimate
 RankBlueprint::combine(const std::vector<HitEstimate> &data) const
 {
     if (data.empty()) {
-        return HitEstimate();
+        return {};
     }
     return data[0];
 }
@@ -527,7 +527,7 @@ RankBlueprint::combine(const std::vector<HitEstimate> &data) const
 FieldSpecBaseList
 RankBlueprint::exposeFields() const
 {
-    return FieldSpecBaseList();
+    return {};
 }
 
 void
@@ -547,7 +547,7 @@ RankBlueprint::get_replacement()
     if (childCnt() == 1) {
         return removeChild(0);
     }
-    return Blueprint::UP();
+    return {};
 }
 
 void
@@ -581,7 +581,7 @@ RankBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches,
             }
         }
         if (require_unpack.size() == 1) {
-            return SearchIterator::UP(std::move(require_unpack[0]));
+            return std::move(require_unpack[0]);
         } else {
             return RankSearch::create(std::move(require_unpack), strict);
         }
@@ -629,7 +629,7 @@ SourceBlenderBlueprint::inheritStrict(size_t) const
 class FindSource : public Blueprint::IPredicate
 {
 public:
-    FindSource(uint32_t sourceId) : _sourceId(sourceId) { }
+    explicit FindSource(uint32_t sourceId) noexcept : _sourceId(sourceId) { }
     bool check(const Blueprint & bp) const override { return bp.getSourceId() == _sourceId; }
 private:
     uint32_t _sourceId;
@@ -655,12 +655,10 @@ SourceBlenderBlueprint::createIntermediateSearch(MultiSearch::Children sub_searc
     assert(sub_searches.size() == childCnt());
     for (size_t i = 0; i < sub_searches.size(); ++i) {
         // TODO: pass ownership with unique_ptr
-        children.push_back(SourceBlenderSearch::Child(sub_searches[i].release(),
-                                                      getChild(i).getSourceId()));
+        children.emplace_back(sub_searches[i].release(), getChild(i).getSourceId());
         assert(children.back().sourceId != 0xffffffff);
     }
-    return SourceBlenderSearch::create(_selector.createIterator(),
-                                       children, strict);
+    return SourceBlenderSearch::create(_selector.createIterator(), children, strict);
 }
 
 SearchIterator::UP
diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp
index 55009e714b9..aec4aa63b6b 100644
--- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp
@@ -96,11 +96,19 @@ SearchIterator::UP
 WeightedSetTermBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &tfmda, bool) const
 {
     assert(tfmda.size() == 1);
+    if ((_terms.size() == 1) && tfmda[0]->isNotNeeded()) {
+        if (LeafBlueprint * leaf = _terms[0]->asLeaf(); leaf != nullptr) {
+            // Always returnin a strict iterator independently of what was required,
+            // as that is what we do with all the children when there are more.
+            return leaf->createLeafSearch(tfmda, true);
+        }
+    }
     fef::MatchData::UP md = _layout.createMatchData();
-    std::vector<SearchIterator*> children(_terms.size());
-    for (size_t i = 0; i < _terms.size(); ++i) {
+    std::vector<SearchIterator*> children;
+    children.reserve(_terms.size());
+    for (const auto & _term : _terms) {
         // TODO: pass ownership with unique_ptr
-        children[i] = _terms[i]->createSearch(*md, true).release();
+        children.push_back(_term->createSearch(*md, true).release());
     }
     return WeightedSetTermSearch::create(children, *tfmda[0], _children_field.isFilter(), _weights, std::move(md));
 }