Extend test for unpack of multibitvector to cover all aspects variants for OR.

5 files changed, 70 insertions, 15 deletions

diff --git a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp
index 12c38d3fe02..b767ce814cc 100644
--- a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp
+++ b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp
@@ -34,6 +34,7 @@ public:
     void testAndWith(bool invert);
     void testEndGuard(bool invert);
     void testIteratorConformance();
+    void testUnpackOfOr();
     template<typename T>
     void testThatOptimizePreservesUnpack();
     template <typename T>
@@ -44,6 +45,7 @@ public:
     void testSearch(bool strict, bool invert);
     int Main() override;
 private:
+    void verifyUnpackOfOr(const UnpackInfo & unpackInfo);
     void verifySelectiveUnpack(SearchIterator & s, const TermFieldMatchData * tfmd);
     void searchAndCompare(SearchIterator::UP s, uint32_t docIdLimit);
     void setup();
@@ -253,6 +255,63 @@ Test::testThatOptimizePreservesUnpack()
     fixup_bitvectors();
 }
 
+void verifyOrUnpack(SearchIterator & s, const TermFieldMatchData * tfmd) {
+    s.initFullRange();
+    s.seek(1);
+    for (size_t i = 0; i < 3; i++) {
+        EXPECT_EQUAL(0u, tfmd[0].getDocId());
+    }
+    s.unpack(1);
+    EXPECT_EQUAL(0u, tfmd[0].getDocId());
+    EXPECT_EQUAL(1u, tfmd[1].getDocId());
+    EXPECT_EQUAL(0u, tfmd[2].getDocId());
+}
+
+void
+Test::testUnpackOfOr() {
+    _bvs[0]->clearBit(1);
+    _bvs[1]->setBit(1);
+    _bvs[2]->clearBit(1);
+    UnpackInfo all;
+    all.forceAll();
+    verifyUnpackOfOr(all);
+
+    UnpackInfo unpackInfo;
+    unpackInfo.add(1);
+    unpackInfo.add(2);
+    verifyUnpackOfOr(unpackInfo);
+
+    fixup_bitvectors();
+}
+
+void
+Test::verifyUnpackOfOr(const UnpackInfo &unpackInfo)
+{
+    TermFieldMatchData tfmdA[3];
+    MultiSearch::Children children;
+    children.push_back(createIter(0, false, tfmdA[0], false).release());
+    children.push_back(createIter(1, false, tfmdA[1], false).release());
+    children.push_back(createIter(2, false, tfmdA[2], false).release());
+    SearchIterator::UP s(OrSearch::create(children, false, unpackInfo));
+    verifyOrUnpack(*s, tfmdA);
+
+    for (auto & tfmd : tfmdA) {
+        tfmd.resetOnlyDocId(0);
+    }
+
+    const MultiSearch * ms = dynamic_cast<const MultiSearch *>(s.get());
+    EXPECT_TRUE(ms != nullptr);
+    EXPECT_EQUAL(3u, ms->getChildren().size());
+
+    s = MultiBitVectorIteratorBase::optimize(std::move(s));
+    s->initFullRange();
+    ms = dynamic_cast<const MultiSearch *>(s.get());
+    EXPECT_TRUE(ms != nullptr);
+    EXPECT_EQUAL(3u, ms->getChildren().size());
+    verifyOrUnpack(*s, tfmdA);
+
+}
+
 void
 Test::verifySelectiveUnpack(SearchIterator & s, const TermFieldMatchData * tfmd)
 {
@@ -584,6 +643,8 @@ Test::Main()
     testThatOptimizePreservesUnpack<OrSearch>();
     testThatOptimizePreservesUnpack<AndSearch>();
     TEST_FLUSH();
+    testUnpackOfOr();
+    TEST_FLUSH();
     testEndGuard(false);
     testEndGuard(true);
     TEST_FLUSH();
diff --git a/searchlib/src/vespa/searchlib/common/bitvectoriterator.cpp b/searchlib/src/vespa/searchlib/common/bitvectoriterator.cpp
index 9c17dfa6f74..18cece7b8b0 100644
--- a/searchlib/src/vespa/searchlib/common/bitvectoriterator.cpp
+++ b/searchlib/src/vespa/searchlib/common/bitvectoriterator.cpp
@@ -2,7 +2,6 @@
 
 #include "bitvectoriterator.h"
 #include <vespa/searchlib/queryeval/emptysearch.h>
-#include <vespa/searchlib/fef/termfieldmatchdata.h>
 #include <vespa/searchlib/fef/termfieldmatchdataarray.h>
 #include <vespa/vespalib/objects/visit.h>
 
@@ -39,12 +38,6 @@ BitVectorIterator::visitMembers(vespalib::ObjectVisitor &visitor) const
     visit(visitor, "termfieldmatchdata.docid", _tfmd.getDocId());
 }
 
-void
-BitVectorIterator::doUnpack(uint32_t docId)
-{
-    _tfmd.resetOnlyDocId(docId);
-}
-
 template<bool inverse>
 class BitVectorIteratorT : public BitVectorIterator {
 public:
diff --git a/searchlib/src/vespa/searchlib/common/bitvectoriterator.h b/searchlib/src/vespa/searchlib/common/bitvectoriterator.h
index 6200837d449..8b1112de97e 100644
--- a/searchlib/src/vespa/searchlib/common/bitvectoriterator.h
+++ b/searchlib/src/vespa/searchlib/common/bitvectoriterator.h
@@ -4,11 +4,11 @@
 
 #include "bitvector.h"
 #include <vespa/searchlib/queryeval/searchiterator.h>
+#include <vespa/searchlib/fef/termfieldmatchdata.h>
 
 namespace search {
 
 namespace fef { class TermFieldMatchDataArray; }
-namespace fef { class TermFieldMatchData; }
 
 class BitVectorIterator : public queryeval::SearchIterator
 {
@@ -20,7 +20,9 @@ protected:
     const BitVector & _bv;
 private:
     void visitMembers(vespalib::ObjectVisitor &visitor) const override;
-    void doUnpack(uint32_t docId) override;
+    void doUnpack(uint32_t docId) override final {
+        _tfmd.resetOnlyDocId(docId);
+    }
     bool isBitVector() const override { return true; }
     fef::TermFieldMatchData  &_tfmd;
 public:
diff --git a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp
index 85136611b42..e9e34b4f5ce 100644
--- a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp
@@ -177,7 +177,9 @@ MultiBitVectorIteratorBase::doUnpack(uint32_t docid)
         MultiSearch::doUnpack(docid);
     } else {
         auto &children = getChildren();
-        _unpackInfo.each([&children,docid](size_t i){children[i]->unpack(docid);}, children.size());
+        _unpackInfo.each([&children,docid](size_t i) {
+                             static_cast<BitVectorIterator *>(children[i])->unpack(docid);
+                         }, children.size());
     }
 }
 
diff --git a/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp b/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp
index 977080f8266..b5e0ff9c423 100644
--- a/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp
@@ -5,8 +5,7 @@
 #include "termwise_helper.h"
 #include <vespa/searchlib/common/bitvector.h>
 
-namespace search {
-namespace queryeval {
+namespace search::queryeval {
 
 namespace {
 
@@ -88,7 +87,6 @@ OrSearch::create(const MultiSearch::Children &children, bool strict) {
 
 SearchIterator *
 OrSearch::create(const MultiSearch::Children &children, bool strict, const UnpackInfo & unpackInfo) {
-    (void) unpackInfo;
     if (strict) {
         if (unpackInfo.unpackAll()) {
             return new OrLikeSearch<true, FullUnpack>(children, FullUnpack());
@@ -108,5 +106,4 @@ OrSearch::create(const MultiSearch::Children &children, bool strict, const Unpac
     }
 }
 
-}  // namespace queryeval
-}  // namespace search
+}