Merge pull request #11188 from vespa-engine/geirst/robust-same-element-iterator-setup

Fix setup of same element iterator to use the attribute search contex…

8 files changed, 12 insertions, 64 deletions

diff --git a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
index d0a04e2a007..5d2dad39ed0 100644
--- a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
+++ b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
@@ -85,7 +85,7 @@ public:
 
 constexpr uint32_t DOCID_LIMIT = 3;
 
-bool search(const Node &node, IAttributeManager &attribute_manager) {
+bool search(const Node &node, IAttributeManager &attribute_manager, bool expect_attribute_search_context = true) {
     AttributeContext ac(attribute_manager);
     FakeRequestContext requestContext(&ac);
     MatchData::UP md(MatchData::makeTestInstance(1, 1));
@@ -94,6 +94,11 @@ bool search(const Node &node, IAttributeManager &attribute_manager) {
     ASSERT_TRUE(result.get());
     EXPECT_TRUE(!result->getState().estimate().empty);
     EXPECT_EQUAL(3u, result->getState().estimate().estHits);
+    if (expect_attribute_search_context) {
+        EXPECT_TRUE(result->get_attribute_search_context() != nullptr);
+    } else {
+        EXPECT_TRUE(result->get_attribute_search_context() == nullptr);
+    }
     result->fetchPostings(true);
     result->setDocIdLimit(DOCID_LIMIT);
     SearchIterator::UP iterator = result->createSearch(*md, true);
@@ -181,13 +186,13 @@ TEST("requireThatLocationTermsWork") {
     MyAttributeManager attribute_manager = makeAttributeManager(int64_t(0xcc));
 
     SimpleLocationTerm node(Location(Point(10, 10), 3, 0), field, 0, Weight(0));
-    EXPECT_TRUE(search(node, attribute_manager));
+    EXPECT_TRUE(search(node, attribute_manager, false));
     node = SimpleLocationTerm(Location(Point(100, 100), 3, 0), field, 0, Weight(0));
-    EXPECT_TRUE(!search(node, attribute_manager));
+    EXPECT_TRUE(!search(node, attribute_manager, false));
     node = SimpleLocationTerm(Location(Point(13, 13), 4, 0), field, 0, Weight(0));
-    EXPECT_TRUE(!search(node, attribute_manager));
+    EXPECT_TRUE(!search(node, attribute_manager, false));
     node = SimpleLocationTerm(Location(Point(10, 13), 3, 0), field, 0, Weight(0));
-    EXPECT_TRUE(search(node, attribute_manager));
+    EXPECT_TRUE(search(node, attribute_manager, false));
 }
 
 TEST("requireThatFastSearchLocationTermsWork") {
diff --git a/searchlib/src/tests/attribute/searchcontext/searchcontext.cpp b/searchlib/src/tests/attribute/searchcontext/searchcontext.cpp
index 8329398f8af..f7d49d6a06a 100644
--- a/searchlib/src/tests/attribute/searchcontext/searchcontext.cpp
+++ b/searchlib/src/tests/attribute/searchcontext/searchcontext.cpp
@@ -266,11 +266,6 @@ private:
     void requireThatOutOfBoundsSearchTermGivesZeroHits(const vespalib::string &name, const Config &cfg, int64_t maxValue);
     void requireThatOutOfBoundsSearchTermGivesZeroHits();
 
-
-    template <typename AttributeType, typename ValueType>
-    void requireThatSearchIteratorExposesSearchContext(const ConfigMap &cfg, ValueType value, const vespalib::string &searchTerm);
-    void requireThatSearchIteratorExposesSearchContext();
-
     // init maps with config objects
     void initIntegerConfig();
     void initFloatConfig();
@@ -1830,47 +1825,6 @@ SearchContextTest::requireThatOutOfBoundsSearchTermGivesZeroHits()
 }
 
 void
-assertSearchIteratorExposesSearchContext(search::attribute::ISearchContext &ctx)
-{
-    ASSERT_TRUE(ctx.valid());
-    ctx.fetchPostings(true);
-    TermFieldMatchData dummy;
-    SearchBasePtr itr = ctx.createIterator(&dummy, true);
-    EXPECT_TRUE(itr->getAttributeSearchContext() != nullptr);
-    EXPECT_EQUAL(&ctx, itr->getAttributeSearchContext());
-}
-
-template <typename AttributeType, typename ValueType>
-void
-SearchContextTest::requireThatSearchIteratorExposesSearchContext(const ConfigMap &cfgMap,
-                                                                 ValueType value,
-                                                                 const vespalib::string &searchTerm)
-{
-    vespalib::string attrSuffix = "-itr-exposes-ctx";
-    std::vector<ValueType> values = {value};
-    for (const auto &cfg : cfgMap) {
-        vespalib::string attrName = cfg.first + attrSuffix;
-        AttributePtr attr = AttributeFactory::createAttribute(attrName, cfg.second);
-        addDocs(*attr, 2);
-        auto &concreteAttr = dynamic_cast<AttributeType &>(*attr);
-        if (attr->hasMultiValue()) {
-            fillAttribute(concreteAttr, values);
-        } else {
-            resetAttribute(concreteAttr, value);
-        }
-        assertSearchIteratorExposesSearchContext(*getSearch(*attr, searchTerm));
-    }
-}
-
-void
-SearchContextTest::requireThatSearchIteratorExposesSearchContext()
-{
-    requireThatSearchIteratorExposesSearchContext<IntegerAttribute, largeint_t>(_integerCfg, 3, "3");
-    requireThatSearchIteratorExposesSearchContext<FloatingPointAttribute, double>(_floatCfg, 5.7, "5.7");
-    requireThatSearchIteratorExposesSearchContext<StringAttribute, vespalib::string>(_stringCfg, "foo", "foo");
-}
-
-void
 SearchContextTest::initIntegerConfig()
 {
     { // CollectionType::SINGLE
@@ -2000,7 +1954,6 @@ SearchContextTest::Main()
     TEST_DO(requireThatInvalidSearchTermGivesZeroHits());
     TEST_DO(requireThatFlagAttributeHandlesTheByteRange());
     TEST_DO(requireThatOutOfBoundsSearchTermGivesZeroHits());
-    TEST_DO(requireThatSearchIteratorExposesSearchContext());
 
     TEST_DONE();
 }
diff --git a/searchlib/src/tests/queryeval/fake_searchable/fake_searchable_test.cpp b/searchlib/src/tests/queryeval/fake_searchable/fake_searchable_test.cpp
index 6cef4479439..6fc75c8e696 100644
--- a/searchlib/src/tests/queryeval/fake_searchable/fake_searchable_test.cpp
+++ b/searchlib/src/tests/queryeval/fake_searchable/fake_searchable_test.cpp
@@ -371,7 +371,7 @@ TEST_F(FakeSearchableTest, require_that_relevant_data_can_be_obtained_from_fake_
     MatchData::UP md = MatchData::makeTestInstance(100, 10);
     bp->fetchPostings(false);
     SearchIterator::UP search = bp->createSearch(*md, false);
-    EXPECT_EQ(bp->get_attribute_search_context(), search->getAttributeSearchContext());
+    EXPECT_TRUE(bp->get_attribute_search_context() != nullptr);
     const auto *attr_ctx = bp->get_attribute_search_context();
     ASSERT_TRUE(attr_ctx);
     EXPECT_EQ(attr_ctx->attributeName(), "attrfoo");
diff --git a/searchlib/src/vespa/searchlib/attribute/attributeiterators.h b/searchlib/src/vespa/searchlib/attribute/attributeiterators.h
index 4d81f46a65c..6d304b61663 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributeiterators.h
+++ b/searchlib/src/vespa/searchlib/attribute/attributeiterators.h
@@ -40,7 +40,6 @@ public:
           _matchPosition(_matchData->populate_fixed())
     { }
     Trinary is_strict() const override { return Trinary::False; }
-    const attribute::ISearchContext *getAttributeSearchContext() const override { return &_baseSearchCtx; }
 };
 
 
diff --git a/searchlib/src/vespa/searchlib/queryeval/fake_search.h b/searchlib/src/vespa/searchlib/queryeval/fake_search.h
index e629e849408..f5b95a94e99 100644
--- a/searchlib/src/vespa/searchlib/queryeval/fake_search.h
+++ b/searchlib/src/vespa/searchlib/queryeval/fake_search.h
@@ -43,7 +43,6 @@ public:
     void doUnpack(uint32_t docid) override;
     const PostingInfo *getPostingInfo() const override { return _result.postingInfo(); }
     void visitMembers(vespalib::ObjectVisitor &visitor) const override;
-    const attribute::ISearchContext *getAttributeSearchContext() const override { return _ctx; }
 };
 
 } // namespace queryeval
diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp
index 9b84136e67c..8ca8ef0f102 100644
--- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp
@@ -77,7 +77,7 @@ SameElementBlueprint::create_same_element_search(bool strict) const
     for (size_t i = 0; i < _terms.size(); ++i) {
         const State &childState = _terms[i]->getState();
         SearchIterator::UP child = _terms[i]->createSearch(*md, (strict && (i == 0)));
-        const attribute::ISearchContext *context = child->getAttributeSearchContext();
+        const attribute::ISearchContext *context = _terms[i]->get_attribute_search_context();
         if (context == nullptr) {
             children[i] = std::move(child);
             childMatch.add(childState.field(0).resolve(*md));
diff --git a/searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp b/searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp
index 9450aceb2be..38830262714 100644
--- a/searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp
@@ -117,12 +117,6 @@ SearchIterator::visitMembers(vespalib::ObjectVisitor &visitor) const
     visit(visitor, "endid", _endid);
 }
 
-const attribute::ISearchContext *
-SearchIterator::getAttributeSearchContext() const
-{
-    return nullptr;
-}
-
 }
 
 //-----------------------------------------------------------------------------
diff --git a/searchlib/src/vespa/searchlib/queryeval/searchiterator.h b/searchlib/src/vespa/searchlib/queryeval/searchiterator.h
index 27494e08f90..8ea45646af8 100644
--- a/searchlib/src/vespa/searchlib/queryeval/searchiterator.h
+++ b/searchlib/src/vespa/searchlib/queryeval/searchiterator.h
@@ -356,8 +356,6 @@ public:
 
     virtual Trinary is_strict() const { return Trinary::Undefined; }
 
-    /** return the underlying attribute search context (or null if none available) */
-    virtual const attribute::ISearchContext *getAttributeSearchContext() const;
 };
 
 }