Drop separate doom in phrase search. Rely on proper split/delay solving the doom issue at a higher level.

11 files changed, 36 insertions, 103 deletions

diff --git a/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp b/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp
index 4877072eacd..13202a062c7 100644
--- a/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp
+++ b/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp
@@ -48,9 +48,6 @@ class Test : public vespalib::TestApp {
     void requireThatTermsCanBeEvaluatedInPriorityOrder();
     void requireThatBlueprintExposesFieldWithEstimate();
     void requireThatBlueprintForcesPositionDataOnChildren();
-    void requireThatIteratorHonorsFutureDoom();
-    void requireThatIteratorHonorsDoom();
-    void requireThatDoomIsPropagated();
 
 public:
     int Main() override;
@@ -79,9 +76,6 @@ Test::Main()
     TEST_DO(requireThatPhrasesAreUnpacked(true, false, true));
     TEST_DO(requireThatBlueprintExposesFieldWithEstimate());
     TEST_DO(requireThatBlueprintForcesPositionDataOnChildren());
-    TEST_DO(requireThatIteratorHonorsFutureDoom());
-    TEST_DO(requireThatIteratorHonorsDoom());
-    TEST_DO(requireThatDoomIsPropagated());
 
     TEST_DONE();
 }
@@ -187,7 +181,7 @@ PhraseSearchTest::PhraseSearchTest(bool expiredDoom)
     : _requestContext(nullptr, expiredDoom ? vespalib::steady_time(): vespalib::steady_time::max()),
       _index(),
       _phrase_fs(field, fieldId, phrase_handle),
-      _phrase(_phrase_fs, _requestContext, false),
+      _phrase(_phrase_fs, false),
       _children(),
       _md(MatchData::makeTestInstance(100, 10)),
       _order(),
@@ -207,51 +201,6 @@ void Test::requireThatIteratorFindsSimplePhrase(bool useBlueprint) {
     EXPECT_TRUE(!search->seek(doc_no_match));
 }
 
-void Test::requireThatIteratorHonorsFutureDoom() {
-    PhraseSearchTest test;
-    test.addTerm("foo", 0).addTerm("bar", 1);
-
-    test.fetchPostings(false);
-    vespalib::TestClock clock;
-    vespalib::Doom futureDoom(clock.clock(), vespalib::steady_time::max());
-    unique_ptr<SearchIterator> search(test.createSearch(false));
-    static_cast<SimplePhraseSearch &>(*search).setDoom(&futureDoom);
-    EXPECT_TRUE(!search->seek(1u));
-    EXPECT_TRUE(search->seek(doc_match));
-    EXPECT_TRUE(!search->seek(doc_no_match));
-}
-
-void Test::requireThatIteratorHonorsDoom() {
-    PhraseSearchTest test;
-    test.addTerm("foo", 0).addTerm("bar", 1);
-
-    test.fetchPostings(false);
-    vespalib::TestClock clock;
-    vespalib::Doom futureDoom(clock.clock(), vespalib::steady_time());
-    unique_ptr<SearchIterator> search(test.createSearch(false));
-    static_cast<SimplePhraseSearch &>(*search).setDoom(&futureDoom);
-    EXPECT_TRUE(!search->seek(1u));
-    EXPECT_EQUAL(search->beginId(), search->getDocId());
-    EXPECT_TRUE(!search->seek(doc_match));
-    EXPECT_TRUE(search->isAtEnd());
-    EXPECT_TRUE(!search->seek(doc_no_match));
-    EXPECT_TRUE(search->isAtEnd());
-}
-
-void Test::requireThatDoomIsPropagated() {
-    PhraseSearchTest test(true);
-    test.addTerm("foo", 0).addTerm("bar", 1);
-
-    test.fetchPostings(true);
-    unique_ptr<SearchIterator> search(test.createSearch(true));
-    EXPECT_TRUE(!search->seek(1u));
-    EXPECT_EQUAL(search->beginId(), search->getDocId());
-    EXPECT_TRUE(!search->seek(doc_match));
-    EXPECT_TRUE(search->isAtEnd());
-    EXPECT_TRUE(!search->seek(doc_no_match));
-    EXPECT_TRUE(search->isAtEnd());
-}
-
 void Test::requireThatIteratorFindsLongPhrase(bool useBlueprint) {
     PhraseSearchTest test;
     test.addTerm("foo", 0).addTerm("bar", 0).addTerm("baz", 0)
@@ -326,9 +275,8 @@ void Test::requireThatTermsCanBeEvaluatedInPriorityOrder() {
 void
 Test::requireThatBlueprintExposesFieldWithEstimate()
 {
-    FakeRequestContext requestContext;
     FieldSpec f("foo", 1, 1);
-    SimplePhraseBlueprint phrase(f, requestContext, false);
+    SimplePhraseBlueprint phrase(f, false);
     ASSERT_TRUE(phrase.getState().numFields() == 1);
     EXPECT_EQUAL(f.getFieldId(), phrase.getState().field(0).getFieldId());
     EXPECT_EQUAL(f.getHandle(), phrase.getState().field(0).getHandle());
@@ -352,9 +300,8 @@ Test::requireThatBlueprintExposesFieldWithEstimate()
 void
 Test::requireThatBlueprintForcesPositionDataOnChildren()
 {
-    FakeRequestContext requestContext;
     FieldSpec f("foo", 1, 1, true);
-    SimplePhraseBlueprint phrase(f, requestContext, false);
+    SimplePhraseBlueprint phrase(f, false);
     EXPECT_TRUE(f.isFilter());
     EXPECT_TRUE(!phrase.getNextChildField(f).isFilter());
 }
diff --git a/searchlib/src/vespa/searchlib/queryeval/andsearch.h b/searchlib/src/vespa/searchlib/queryeval/andsearch.h
index 85df54c81d8..a3e7c074cf1 100644
--- a/searchlib/src/vespa/searchlib/queryeval/andsearch.h
+++ b/searchlib/src/vespa/searchlib/queryeval/andsearch.h
@@ -23,7 +23,7 @@ public:
     AndSearch & estimate(uint32_t est) { _estimate = est; return *this; }
     uint32_t estimate() const { return _estimate; }
 protected:
-    AndSearch(Children children);
+    explicit AndSearch(Children children);
     void doUnpack(uint32_t docid) override;
     UP andWith(UP filter, uint32_t estimate) override;
     UP offerFilterToChildren(UP filter, uint32_t estimate);
diff --git a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp
index 5b8757411bd..d179515be6c 100644
--- a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp
@@ -31,7 +31,7 @@ CreateBlueprintVisitorHelper::getResult()
 
 void
 CreateBlueprintVisitorHelper::visitPhrase(query::Phrase &n) {
-    auto phrase = std::make_unique<SimplePhraseBlueprint>(_field, _requestContext, n.is_expensive());
+    auto phrase = std::make_unique<SimplePhraseBlueprint>(_field, n.is_expensive());
     for (const query::Node * child : n.getChildren()) {
         FieldSpecList fields;
         fields.add(phrase->getNextChildField(_field));
diff --git a/searchlib/src/vespa/searchlib/queryeval/irequestcontext.h b/searchlib/src/vespa/searchlib/queryeval/irequestcontext.h
index 5aa5db081ab..52992a52103 100644
--- a/searchlib/src/vespa/searchlib/queryeval/irequestcontext.h
+++ b/searchlib/src/vespa/searchlib/queryeval/irequestcontext.h
@@ -17,7 +17,7 @@ namespace search::queryeval {
 class IRequestContext
 {
 public:
-    virtual ~IRequestContext() { }
+    virtual ~IRequestContext() = default;
 
     /**
      * Provides the time of soft doom for the query. Now it is time to start cleaning up and return what you have.
diff --git a/searchlib/src/vespa/searchlib/queryeval/multisearch.cpp b/searchlib/src/vespa/searchlib/queryeval/multisearch.cpp
index 5c932b3aeb8..16f4012f0e7 100644
--- a/searchlib/src/vespa/searchlib/queryeval/multisearch.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/multisearch.cpp
@@ -42,9 +42,8 @@ MultiSearch::MultiSearch(Children children)
 {
 }
 
-MultiSearch::~MultiSearch()
-{
-}
+MultiSearch::MultiSearch() = default;
+MultiSearch::~MultiSearch() = default;
 
 void
 MultiSearch::initRange(uint32_t beginid, uint32_t endid)
diff --git a/searchlib/src/vespa/searchlib/queryeval/multisearch.h b/searchlib/src/vespa/searchlib/queryeval/multisearch.h
index 73c31d243db..9216391b85d 100644
--- a/searchlib/src/vespa/searchlib/queryeval/multisearch.h
+++ b/searchlib/src/vespa/searchlib/queryeval/multisearch.h
@@ -32,8 +32,8 @@ public:
      * @param children the search objects we are and'ing
      *        this object takes ownership of the children.
      **/
-    MultiSearch(Children children);
-    virtual ~MultiSearch() override;
+    explicit MultiSearch(Children children);
+    ~MultiSearch() override;
     const Children & getChildren() const { return _children; }
     virtual bool isAnd() const { return false; }
     virtual bool isAndNot() const { return false; }
@@ -42,7 +42,7 @@ public:
     virtual bool needUnpack(size_t index) const { (void) index; return true; }
     void initRange(uint32_t beginId, uint32_t endId) override;
 protected:
-    MultiSearch() {}
+    MultiSearch();
     void doUnpack(uint32_t docid) override;
     void visitMembers(vespalib::ObjectVisitor &visitor) const override;
 private:
diff --git a/searchlib/src/vespa/searchlib/queryeval/searchable.h b/searchlib/src/vespa/searchlib/queryeval/searchable.h
index 6202f5d1f0d..2467cfe4142 100644
--- a/searchlib/src/vespa/searchlib/queryeval/searchable.h
+++ b/searchlib/src/vespa/searchlib/queryeval/searchable.h
@@ -36,7 +36,7 @@ protected:
 public:
     typedef std::shared_ptr<Searchable> SP;
 
-    Searchable() {}
+    Searchable() = default;
 
     /**
      * Create a blueprint searching a set of fields. The default
@@ -51,7 +51,7 @@ public:
     virtual Blueprint::UP createBlueprint(const IRequestContext & requestContext,
                                           const FieldSpecList &fields,
                                           const search::query::Node &term);
-    virtual ~Searchable() {}
+    virtual ~Searchable() = default;
 };
 
 }
diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp
index 0025fd5fe03..35d1f1692d4 100644
--- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp
@@ -11,9 +11,8 @@
 
 namespace search::queryeval {
 
-SimplePhraseBlueprint::SimplePhraseBlueprint(const FieldSpec &field, const IRequestContext & requestContext, bool expensive)
+SimplePhraseBlueprint::SimplePhraseBlueprint(const FieldSpec &field, bool expensive)
     : ComplexLeafBlueprint(field),
-      _doom(requestContext.getDoom()),
       _field(field),
       _estimate(),
       _layout(),
@@ -79,11 +78,8 @@ SimplePhraseBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &tfmd
         eval_order.push_back(child.second);
     }
     
-    auto phrase = std::make_unique<SimplePhraseSearch>(std::move(children),
-                                                       std::move(md), childMatch,
-                                                       eval_order, *tfmda[0], strict);
-    phrase->setDoom(& _doom);
-    return phrase;
+    return std::make_unique<SimplePhraseSearch>(std::move(children),std::move(md), childMatch,
+                                                eval_order, *tfmda[0], strict);
 }
 
 SearchIterator::UP
diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.h
index 10cdac34f19..e32001f8ae1 100644
--- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.h
+++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.h
@@ -5,7 +5,6 @@
 #include "searchable.h"
 #include "irequestcontext.h"
 #include <vespa/searchlib/fef/matchdatalayout.h>
-#include <vespa/vespalib/util/doom.h>
 
 namespace search::fef { class TermFieldMatchData; }
 
@@ -14,18 +13,16 @@ namespace search::queryeval {
 class SimplePhraseBlueprint : public ComplexLeafBlueprint
 {
 private:
-    const vespalib::Doom    _doom; 
     FieldSpec               _field;
     HitEstimate             _estimate;
     fef::MatchDataLayout    _layout;
     std::vector<Blueprint*> _terms;
 
-    SimplePhraseBlueprint(const SimplePhraseBlueprint &); // disabled
-    SimplePhraseBlueprint &operator=(const SimplePhraseBlueprint &); // disabled
-
 public:
-    SimplePhraseBlueprint(const FieldSpec &field, const IRequestContext & requestContext, bool expensive);
-    ~SimplePhraseBlueprint();
+    SimplePhraseBlueprint(const FieldSpec &field, bool expensive);
+    SimplePhraseBlueprint(const SimplePhraseBlueprint &) = delete;
+    SimplePhraseBlueprint &operator=(const SimplePhraseBlueprint &) = delete;
+    ~SimplePhraseBlueprint() override;
 
     // used by create visitor
     FieldSpec getNextChildField(const FieldSpec &outer);
diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp
index f58f888393b..158c6b255b2 100644
--- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp
@@ -22,17 +22,21 @@ class PhraseMatcher {
     uint32_t _element_id;
     uint32_t _position;
 
-    TermFieldMatchData::PositionsIterator &iterator(uint32_t word_index)
-    { return _iterators[word_index]; }
+    TermFieldMatchData::PositionsIterator &iterator(uint32_t word_index) {
+        return _iterators[word_index];
+    }
 
-    TermFieldMatchData::PositionsIterator end(uint32_t word_index)
-    { return _tmds[word_index]->end(); }
+    TermFieldMatchData::PositionsIterator end(uint32_t word_index) {
+        return _tmds[word_index]->end();
+    }
 
-    uint32_t elementId(uint32_t word_index)
-    { return iterator(word_index)->getElementId(); }
+    uint32_t elementId(uint32_t word_index) {
+        return iterator(word_index)->getElementId();
+    }
 
-    uint32_t position(uint32_t word_index)
-    { return iterator(word_index)->getPosition(); }
+    uint32_t position(uint32_t word_index) {
+        return iterator(word_index)->getPosition();
+    }
 
     void iterateToElement(uint32_t word_index) {
         while (iterator(word_index) != end(word_index) &&
@@ -145,13 +149,9 @@ allTermsHaveMatch(const SimplePhraseSearch::Children &terms, const vector<uint32
 void
 SimplePhraseSearch::phraseSeek(uint32_t doc_id) {
     if (allTermsHaveMatch(getChildren(), _eval_order, doc_id)) {
-        if (doom()) {
-            setAtEnd();
-        } else {
-            AndSearch::doUnpack(doc_id);
-            if (PhraseMatcher(_childMatch, _eval_order, _iterators).hasMatch()) {
-                setDocId(doc_id);
-            }
+        AndSearch::doUnpack(doc_id);
+        if (PhraseMatcher(_childMatch, _eval_order, _iterators).hasMatch()) {
+            setDocId(doc_id);
         }
     }
 }
@@ -167,11 +167,10 @@ SimplePhraseSearch::SimplePhraseSearch(Children children,
       _childMatch(childMatch),
       _eval_order(std::move(eval_order)),
       _tmd(tmd),
-      _doom(nullptr),
       _strict(strict),
       _iterators(getChildren().size())
 {
-    assert(getChildren().size() > 0);
+    assert( ! getChildren().empty());
     assert(getChildren().size() == _childMatch.size());
     assert(getChildren().size() == _eval_order.size());
 }
diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.h b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.h
index 7b9d7c9365f..6374f49e4ab 100644
--- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.h
+++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.h
@@ -7,7 +7,6 @@
 #include <vespa/searchlib/fef/matchdata.h>
 #include <vespa/searchlib/fef/termfieldmatchdataarray.h>
 #include <vespa/searchlib/fef/termfieldmatchdata.h>
-#include <vespa/vespalib/util/doom.h>
 #include <memory>
 #include <vector>
 
@@ -22,7 +21,6 @@ class SimplePhraseSearch : public AndSearch
     fef::TermFieldMatchDataArray _childMatch;
     std::vector<uint32_t>        _eval_order;
     fef::TermFieldMatchData     &_tmd;
-    const vespalib::Doom        *_doom;
     bool                         _strict;
 
     typedef fef::TermFieldMatchData::PositionsIterator It;
@@ -30,8 +28,6 @@ class SimplePhraseSearch : public AndSearch
     std::vector<It> _iterators;
 
     void phraseSeek(uint32_t doc_id);
-    bool doom() const { return ((_doom != nullptr) && _doom->soft_doom()); }
-
 public:
     /**
      * Takes ownership of the contents of children.
@@ -51,7 +47,6 @@ public:
     void doSeek(uint32_t doc_id) override;
     void doUnpack(uint32_t doc_id) override;
     void visitMembers(vespalib::ObjectVisitor &visitor) const override;
-    SimplePhraseSearch & setDoom(const vespalib::Doom * doom) { _doom = doom; return *this; }
 };
 
 }