Merge pull request #6187 from vespa-engine/havardpe/test-iterator-wrapping

verify that attribute iterators are wrapped for element unpack

6 files changed, 67 insertions, 5 deletions

diff --git a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp
index d89883bc417..45ebdd78fb3 100644
--- a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp
+++ b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp
@@ -5,17 +5,24 @@
 #include <vespa/searchlib/queryeval/leaf_blueprints.h>
 #include <vespa/searchlib/queryeval/simpleresult.h>
 #include <vespa/searchlib/queryeval/same_element_blueprint.h>
+#include <vespa/searchlib/queryeval/same_element_search.h>
+#include <vespa/searchlib/queryeval/emptysearch.h>
+#include <vespa/searchcommon/attribute/i_search_context.h>
+#include <vespa/searchlib/attribute/elementiterator.h>
 
 using namespace search::fef;
 using namespace search::queryeval;
+using search::attribute::ElementIterator;
 
-std::unique_ptr<SameElementBlueprint> make_blueprint(const std::vector<FakeResult> &children) {
+std::unique_ptr<SameElementBlueprint> make_blueprint(const std::vector<FakeResult> &children, bool fake_attr = false) {
     auto result = std::make_unique<SameElementBlueprint>();
     for (size_t i = 0; i < children.size(); ++i) {
         uint32_t field_id = i;
         vespalib::string field_name = vespalib::make_string("f%u", field_id);
         FieldSpec field = result->getNextChildField(field_name, field_id);
-        result->addTerm(std::make_unique<FakeBlueprint>(field, children[i]));
+        auto fake = std::make_unique<FakeBlueprint>(field, children[i]);
+        fake->is_attr(fake_attr);
+        result->addTerm(std::move(fake));
     }
     return result;
 }
@@ -96,4 +103,17 @@ TEST("require that children are sorted") {
     EXPECT_EQUAL(dynamic_cast<SameElementBlueprint&>(*bp).terms()[2]->getState().estimate().estHits, 4u);
 }
 
+TEST("require that attribute iterators are wrapped for element unpacking") {
+    auto a = make_result({{5, {1,3,7}}});
+    auto b = make_result({{5, {3,5,10}}});
+    auto bp = finalize(make_blueprint({a,b}, true), true);
+    auto md = MatchData::makeTestInstance(0, 0);
+    auto search = bp->createSearch(*md, false);
+    SameElementSearch *se = dynamic_cast<SameElementSearch*>(search.get());
+    ASSERT_TRUE(se != nullptr);
+    ASSERT_EQUAL(se->children().size(), 2u);
+    EXPECT_TRUE(dynamic_cast<ElementIterator*>(se->children()[0].get()) != nullptr);
+    EXPECT_TRUE(dynamic_cast<ElementIterator*>(se->children()[1].get()) != nullptr);   
+}
+
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/searchlib/src/vespa/searchlib/queryeval/fake_search.cpp b/searchlib/src/vespa/searchlib/queryeval/fake_search.cpp
index 226f41a53c0..3165cd9b68a 100644
--- a/searchlib/src/vespa/searchlib/queryeval/fake_search.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/fake_search.cpp
@@ -4,10 +4,37 @@
 #include <vespa/searchlib/fef/termfieldmatchdataposition.h>
 #include <vespa/searchlib/fef/termfieldmatchdata.h>
 #include <vespa/vespalib/objects/visit.h>
+#include <vespa/searchcommon/attribute/i_search_context.h>
 
 namespace search {
 namespace queryeval {
 
+namespace {
+
+struct FakeContext : search::attribute::ISearchContext {
+    int32_t onFind(DocId, int32_t, int32_t &) const override { return -1; }
+    int32_t onFind(DocId, int32_t) const override { return -1; }
+    unsigned int approximateHits() const override { return 0; }
+    std::unique_ptr<SearchIterator> createIterator(fef::TermFieldMatchData *, bool) override { abort(); }
+    void fetchPostings(bool) override { }
+    bool valid() const override { return true; }
+    search::Int64Range getAsIntegerTerm() const override { abort(); }
+    const search::QueryTermBase &queryTerm() const override { abort(); }
+    const vespalib::string &attributeName() const override { abort(); }
+};
+
+} // namespace search::queryeval::<unnamed>
+
+void
+FakeSearch::is_attr(bool value)
+{
+    if (value) {
+        _ctx = std::make_unique<FakeContext>();
+    } else {
+        _ctx.reset();
+    }
+}
+
 void
 FakeSearch::doSeek(uint32_t docid)
 {
@@ -49,5 +76,5 @@ FakeSearch::visitMembers(vespalib::ObjectVisitor &visitor) const
     visit(visitor, "term",  _term);
 }
 
-} // namespace queryeval
+} // namespace search::queryeval
 } // namespace search
diff --git a/searchlib/src/vespa/searchlib/queryeval/fake_search.h b/searchlib/src/vespa/searchlib/queryeval/fake_search.h
index c320d497edc..aa6df480a21 100644
--- a/searchlib/src/vespa/searchlib/queryeval/fake_search.h
+++ b/searchlib/src/vespa/searchlib/queryeval/fake_search.h
@@ -5,6 +5,7 @@
 #include "searchiterator.h"
 #include "fake_result.h"
 #include <vespa/searchlib/fef/termfieldmatchdataarray.h>
+#include <vespa/searchcommon/attribute/i_search_context.h>
 
 namespace search {
 namespace queryeval {
@@ -18,6 +19,7 @@ private:
     FakeResult                   _result;
     uint32_t                     _offset;
     fef::TermFieldMatchDataArray _tfmda;
+    std::unique_ptr<attribute::ISearchContext> _ctx;
 
     bool valid() const { return _offset < _result.inspect().size(); }
     uint32_t currId() const { return _result.inspect()[_offset].docId; }
@@ -34,10 +36,12 @@ public:
     {
         assert(_tfmda.size() == 1);
     }
+    void is_attr(bool value);
     void doSeek(uint32_t docid) override;
     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.get(); }
 };
 
 } // namespace queryeval
diff --git a/searchlib/src/vespa/searchlib/queryeval/leaf_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/leaf_blueprints.cpp
index bbfa487ae7d..a140fb146d5 100644
--- a/searchlib/src/vespa/searchlib/queryeval/leaf_blueprints.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/leaf_blueprints.cpp
@@ -64,7 +64,9 @@ SimpleBlueprint::tag(const vespalib::string &t)
 SearchIterator::UP
 FakeBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &tfmda, bool) const
 {
-    return std::make_unique<FakeSearch>(_tag, _field.getName(), _term, _result, tfmda);
+    auto result = std::make_unique<FakeSearch>(_tag, _field.getName(), _term, _result, tfmda);
+    result->is_attr(_is_attr);
+    return result;
 }
 
 FakeBlueprint::FakeBlueprint(const FieldSpec &field, const FakeResult &result)
@@ -72,7 +74,8 @@ FakeBlueprint::FakeBlueprint(const FieldSpec &field, const FakeResult &result)
       _tag("<tag>"),
       _term("<term>"),
       _field(field),
-      _result(result)
+      _result(result),
+      _is_attr(false)
 {
     setEstimate(HitEstimate(result.inspect().size(), result.inspect().empty()));
 }
diff --git a/searchlib/src/vespa/searchlib/queryeval/leaf_blueprints.h b/searchlib/src/vespa/searchlib/queryeval/leaf_blueprints.h
index 698e2235690..85d30aaf003 100644
--- a/searchlib/src/vespa/searchlib/queryeval/leaf_blueprints.h
+++ b/searchlib/src/vespa/searchlib/queryeval/leaf_blueprints.h
@@ -51,6 +51,7 @@ private:
     vespalib::string _term;
     FieldSpec   _field;
     FakeResult  _result;
+    bool _is_attr;
 
 protected:
     SearchIterator::UP
@@ -66,6 +67,12 @@ public:
     }
     const vespalib::string &tag() const { return _tag; }
 
+    FakeBlueprint &is_attr(bool value) {
+        _is_attr = value;
+        return *this;
+    }
+    bool is_attr() const { return _is_attr; }
+
     FakeBlueprint &term(const vespalib::string &t) {
         _term = t;
         return *this;
diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_search.h b/searchlib/src/vespa/searchlib/queryeval/same_element_search.h
index 6a116c76e73..1fd381eb1ae 100644
--- a/searchlib/src/vespa/searchlib/queryeval/same_element_search.h
+++ b/searchlib/src/vespa/searchlib/queryeval/same_element_search.h
@@ -39,6 +39,7 @@ public:
     void doSeek(uint32_t docid) override;
     void doUnpack(uint32_t) override {}
     void visitMembers(vespalib::ObjectVisitor &visitor) const override;
+    const std::vector<SearchIterator::UP> &children() const { return _children; }
 };
 
 }