diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2019-01-09 11:42:39 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-01-09 11:42:39 +0100 |
commit | 3187143c1b774684b5668fecd665a4452003ca32 (patch) | |
tree | 92abef3807ef582fbe726005c8e50c221a96ba83 /searchlib | |
parent | 5251626df44e98457ea111f440d9a79cb6033075 (diff) | |
parent | e2d1b55f50ee08f40fa16892dcd384fcfbb74781 (diff) |
Merge pull request #8061 from vespa-engine/balder/add-multibitvector-iterator-too-conformance-tests-too
Balder/add multibitvector iterator too conformance tests too
Diffstat (limited to 'searchlib')
12 files changed, 97 insertions, 50 deletions
diff --git a/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp b/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp index ba266a85470..f4aae4e2d73 100644 --- a/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp +++ b/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp @@ -80,10 +80,11 @@ private: FakePosting::SP _fp; }; - Verifier::Verifier(FakePosting::SP fp) : - _fp(std::move(fp)) - { } - Verifier::~Verifier() {} +Verifier::Verifier(FakePosting::SP fp) + : _fp(std::move(fp)) +{ } + +Verifier::~Verifier() = default; void Test::requireThatSearchIteratorsConforms() diff --git a/searchlib/src/tests/queryeval/multibitvectoriterator/CMakeLists.txt b/searchlib/src/tests/queryeval/multibitvectoriterator/CMakeLists.txt index c447aeabb76..718b0553fc9 100644 --- a/searchlib/src/tests/queryeval/multibitvectoriterator/CMakeLists.txt +++ b/searchlib/src/tests/queryeval/multibitvectoriterator/CMakeLists.txt @@ -4,6 +4,7 @@ vespa_add_executable(searchlib_multibitvectoriterator_test_app TEST multibitvectoriterator_test.cpp DEPENDS searchlib + searchlib_test ) vespa_add_test(NAME searchlib_multibitvectoriterator_test_app COMMAND searchlib_multibitvectoriterator_test_app) vespa_add_executable(searchlib_multibitvectoriterator_bench_app diff --git a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp index 91d39965531..28ba13bb4fc 100644 --- a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp +++ b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp @@ -1,6 +1,4 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("multibitvectoriterator_test"); #include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/queryeval/multibitvectoriterator.h> #include <vespa/searchlib/queryeval/emptysearch.h> @@ -11,10 +9,15 @@ LOG_SETUP("multibitvectoriterator_test"); #include <vespa/searchlib/queryeval/orsearch.h> #include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/searchlib/fef/termfieldmatchdataarray.h> +#include <vespa/searchlib/test/searchiteratorverifier.h> + +#include <vespa/log/log.h> +LOG_SETUP("multibitvectoriterator_test"); using namespace search::queryeval; using namespace search::fef; using namespace search; +using vespalib::Trinary; //----------------------------------------------------------------------------- @@ -29,6 +32,7 @@ public: void testOr(); void testAndWith(bool invert); void testEndGuard(bool invert); + void testIteratorConformance(); template<typename T> void testThatOptimizePreservesUnpack(); template <typename T> @@ -355,7 +359,7 @@ Test::testOptimizeCommon(bool isAnd, bool invert) EXPECT_EQUAL(2u, m.getChildren().size()); EXPECT_TRUE(dynamic_cast<const EmptySearch *>(m.getChildren()[0]) != nullptr); EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[1]) != nullptr); - EXPECT_FALSE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[1])->isStrict()); + EXPECT_TRUE(Trinary::False == m.getChildren()[1]->is_strict()); } { MultiSearch::Children children; @@ -371,7 +375,7 @@ Test::testOptimizeCommon(bool isAnd, bool invert) EXPECT_EQUAL(2u, m.getChildren().size()); EXPECT_TRUE(dynamic_cast<const EmptySearch *>(m.getChildren()[0]) != nullptr); EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[1]) != nullptr); - EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[1])->isStrict()); + EXPECT_TRUE(Trinary::True == m.getChildren()[1]->is_strict()); } { MultiSearch::Children children; @@ -418,7 +422,7 @@ Test::testOptimizeAndOr(bool invert) s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(s); EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(s.get()) != nullptr); - EXPECT_FALSE(dynamic_cast<const MultiBitVectorIteratorBase *>(s.get())->isStrict()); + EXPECT_TRUE(Trinary::False == s->is_strict()); } { MultiSearch::Children children; @@ -433,7 +437,7 @@ Test::testOptimizeAndOr(bool invert) const MultiSearch & m(dynamic_cast<const MultiSearch &>(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[0]) != nullptr); - EXPECT_FALSE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[0])->isStrict()); + EXPECT_TRUE(Trinary::False == m.getChildren()[0]->is_strict()); EXPECT_TRUE(dynamic_cast<const EmptySearch *>(m.getChildren()[1]) != nullptr); } { @@ -449,7 +453,7 @@ Test::testOptimizeAndOr(bool invert) const MultiSearch & m(dynamic_cast<const MultiSearch &>(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[0]) != nullptr); - EXPECT_FALSE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[0])->isStrict()); + EXPECT_TRUE(Trinary::False == m.getChildren()[0]->is_strict()); EXPECT_TRUE(dynamic_cast<const EmptySearch *>(m.getChildren()[1]) != nullptr); } { @@ -465,7 +469,7 @@ Test::testOptimizeAndOr(bool invert) const MultiSearch & m(dynamic_cast<const MultiSearch &>(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[0]) != nullptr); - EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[0])->isStrict()); + EXPECT_TRUE(Trinary::True == m.getChildren()[0]->is_strict()); EXPECT_TRUE(dynamic_cast<const EmptySearch *>(m.getChildren()[1]) != nullptr); } { @@ -481,7 +485,7 @@ Test::testOptimizeAndOr(bool invert) const MultiSearch & m(dynamic_cast<const MultiSearch &>(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[0]) != nullptr); - EXPECT_TRUE(dynamic_cast<const MultiBitVectorIteratorBase *>(m.getChildren()[0])->isStrict()); + EXPECT_TRUE(Trinary::True == m.getChildren()[0]->is_strict()); EXPECT_TRUE(dynamic_cast<const EmptySearch *>(m.getChildren()[1]) != nullptr); } } @@ -506,6 +510,60 @@ Test::testEndGuard(bool invert) EXPECT_FALSE(m.seek(_bvs[0]->size()+987)); } +class Verifier : public search::test::SearchIteratorVerifier { +public: + Verifier(size_t numBv, bool is_and); + ~Verifier(); + + SearchIterator::UP create(bool strict) const override; + +private: + bool _is_and; + mutable TermFieldMatchData _tfmd; + std::vector<BitVector::UP> _bvs; +}; + +Verifier::Verifier(size_t numBv, bool is_and) + : _is_and(is_and), + _bvs() +{ + for (size_t i(0); i < numBv; i++) { + _bvs.push_back(BitVector::create(getDocIdLimit())); + } + for (uint32_t docId: getExpectedDocIds()) { + if (_is_and) { + for (auto & bv : _bvs) { + bv->setBit(docId); + } + } else { + _bvs[docId%_bvs.size()]->setBit(docId); + } + } +} +Verifier::~Verifier() = default; + +SearchIterator::UP +Verifier::create(bool strict) const { + MultiSearch::Children bvs; + for (const auto & bv : _bvs) { + bvs.push_back(BitVectorIterator::create(bv.get(), getDocIdLimit(), _tfmd, strict, false).release()); + } + SearchIterator::UP iter(_is_and ? AndSearch::create(bvs, strict) : OrSearch::create(bvs, strict)); + auto mbvit = MultiBitVectorIteratorBase::optimize(std::move(iter)); + EXPECT_TRUE((bvs.size() < 2) || (dynamic_cast<const MultiBitVectorIteratorBase *>(mbvit.get()) != nullptr)); + EXPECT_EQUAL(strict, Trinary::True == mbvit->is_strict()); + return mbvit; +} + +void Test::testIteratorConformance() { + for (bool is_and : {false, true}) { + for (size_t i(1); i < 6; i++) { + Verifier searchIteratorVerifier(i, is_and); + searchIteratorVerifier.verify(); + } + } +} + int Test::Main() { @@ -527,6 +585,8 @@ Test::Main() testAndWith(false); testAndWith(true); TEST_FLUSH(); + testIteratorConformance(); + TEST_FLUSH(); TEST_DONE(); } diff --git a/searchlib/src/vespa/searchlib/common/bitvectoriterator.h b/searchlib/src/vespa/searchlib/common/bitvectoriterator.h index be19b1e1343..6200837d449 100644 --- a/searchlib/src/vespa/searchlib/common/bitvectoriterator.h +++ b/searchlib/src/vespa/searchlib/common/bitvectoriterator.h @@ -28,7 +28,6 @@ public: const void *getBitValues() const { return _bv.getStart(); } Trinary is_strict() const override { return Trinary::False; } - virtual bool isStrict() const { return (is_strict() == Trinary::True); } uint32_t getDocIdLimit() const { return _docIdLimit; } static UP create(const BitVector *const other, fef::TermFieldMatchData &matchData, bool strict, bool inverted = false); static UP create(const BitVector *const other, uint32_t docIdLimit, diff --git a/searchlib/src/vespa/searchlib/queryeval/iterator_pack.cpp b/searchlib/src/vespa/searchlib/queryeval/iterator_pack.cpp index b11f85e09f9..2f9242bb3fd 100644 --- a/searchlib/src/vespa/searchlib/queryeval/iterator_pack.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/iterator_pack.cpp @@ -6,7 +6,7 @@ namespace search::queryeval { -SearchIteratorPack::~SearchIteratorPack() { } +SearchIteratorPack::~SearchIteratorPack() = default; SearchIteratorPack::SearchIteratorPack() : _children(), _childMatch(), _md() {} diff --git a/searchlib/src/vespa/searchlib/queryeval/iterators.cpp b/searchlib/src/vespa/searchlib/queryeval/iterators.cpp index 7b8485ad2f5..32f6122558e 100644 --- a/searchlib/src/vespa/searchlib/queryeval/iterators.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/iterators.cpp @@ -2,9 +2,7 @@ #include "iterators.h" -namespace search { - -namespace queryeval { +namespace search::queryeval { RankedSearchIteratorBase:: RankedSearchIteratorBase(const fef::TermFieldMatchDataArray &matchData) @@ -13,6 +11,4 @@ RankedSearchIteratorBase(const fef::TermFieldMatchDataArray &matchData) _needUnpack(1) { } -} // namespace queryeval - -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/queryeval/iterators.h b/searchlib/src/vespa/searchlib/queryeval/iterators.h index 79c91f6be39..43d046a78b7 100644 --- a/searchlib/src/vespa/searchlib/queryeval/iterators.h +++ b/searchlib/src/vespa/searchlib/queryeval/iterators.h @@ -5,9 +5,7 @@ #include "searchiterator.h" #include <vespa/searchlib/fef/termfieldmatchdataarray.h> -namespace search { - -namespace queryeval { +namespace search::queryeval { class DocIdAndFeatures; @@ -28,7 +26,4 @@ public: RankedSearchIteratorBase(const fef::TermFieldMatchDataArray &matchData); }; -} // namespace queryeval - -} // namespace search - +} diff --git a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp index cf660624cff..bba331284f9 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp @@ -13,6 +13,8 @@ namespace search::queryeval { +using vespalib::Trinary; + namespace { template<typename Update> @@ -25,7 +27,7 @@ protected: void strictSeek(uint32_t docId); private: void doSeek(uint32_t docId) override; - bool isStrict() const override { return false; } + Trinary is_strict() const override { return Trinary::False; } bool acceptExtraFilter() const override { return Update::isAnd(); } Update _update; }; @@ -37,7 +39,7 @@ public: MultiBitVectorIteratorStrict(const MultiSearch::Children & children) : MultiBitVectorIterator<Update>(children) { } private: void doSeek(uint32_t docId) override { this->strictSeek(docId); } - bool isStrict() const override { return true; } + Trinary is_strict() const override { return Trinary::True; } }; template<typename Update> @@ -211,7 +213,7 @@ MultiBitVectorIteratorBase::optimizeMultiSearch(SearchIterator::UP parentIt) _unpackIndex.push_back(stolen.size()); } SearchIterator::UP bit = parent.remove(it); - if ( ! strict && static_cast<const BitVectorIterator &>(*bit).isStrict()) { + if ( ! strict && (bit->is_strict() == Trinary::True)) { strict = true; } stolen.push_back(bit.release()); diff --git a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h index f233ee66fd0..cc40f834114 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h +++ b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h @@ -12,7 +12,6 @@ class MultiBitVectorIteratorBase : public MultiSearch, protected BitWord { public: ~MultiBitVectorIteratorBase(); - virtual bool isStrict() const = 0; void initRange(uint32_t beginId, uint32_t endId) override; void addUnpackIndex(size_t index) { _unpackInfo.add(index); } /** diff --git a/searchlib/src/vespa/searchlib/test/document_weight_attribute_helper.h b/searchlib/src/vespa/searchlib/test/document_weight_attribute_helper.h index 308c305634a..163007b1828 100644 --- a/searchlib/src/vespa/searchlib/test/document_weight_attribute_helper.h +++ b/searchlib/src/vespa/searchlib/test/document_weight_attribute_helper.h @@ -8,8 +8,7 @@ #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/vespalib/testkit/test_kit.h> -namespace search { -namespace test { +namespace search::test { class DocumentWeightAttributeHelper { @@ -51,5 +50,4 @@ public: const IDocumentWeightAttribute &dwa() const { return *_dwa; } }; -} // namespace search::test -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/test/fakedata/fpfactory.cpp b/searchlib/src/vespa/searchlib/test/fakedata/fpfactory.cpp index 712e44002a8..f10af286dd7 100644 --- a/searchlib/src/vespa/searchlib/test/fakedata/fpfactory.cpp +++ b/searchlib/src/vespa/searchlib/test/fakedata/fpfactory.cpp @@ -12,9 +12,7 @@ namespace search::fakedata { using index::Schema; -FPFactory::~FPFactory(void) -{ -} +FPFactory::~FPFactory() = default; void FPFactory::setup(const FakeWordSet &fws) @@ -43,7 +41,7 @@ FPFactory::setup(const std::vector<const FakeWord *> &fws) typedef std::map<const std::string, FPFactoryMaker *const> FPFactoryMap; -static FPFactoryMap *fpFactoryMap = NULL; +static FPFactoryMap *fpFactoryMap = nullptr; /* * Posting list factory glue. @@ -52,15 +50,15 @@ static FPFactoryMap *fpFactoryMap = NULL; FPFactory * getFPFactory(const std::string &name, const Schema &schema) { - if (fpFactoryMap == NULL) - return NULL; + if (fpFactoryMap == nullptr) + return nullptr; FPFactoryMap::const_iterator i(fpFactoryMap->find(name)); if (i != fpFactoryMap->end()) return i->second(schema); else - return NULL; + return nullptr; } @@ -69,7 +67,7 @@ getPostingTypes() { std::vector<std::string> res; - if (fpFactoryMap != NULL) + if (fpFactoryMap != nullptr) for (FPFactoryMap::const_iterator i(fpFactoryMap->begin()); i != fpFactoryMap->end(); ++i) @@ -81,20 +79,20 @@ getPostingTypes() FPFactoryInit::FPFactoryInit(const FPFactoryMapEntry &fpFactoryMapEntry) : _key(fpFactoryMapEntry.first) { - if (fpFactoryMap == NULL) + if (fpFactoryMap == nullptr) fpFactoryMap = new FPFactoryMap; fpFactoryMap->insert(fpFactoryMapEntry); } FPFactoryInit::~FPFactoryInit() { - assert(fpFactoryMap != NULL); + assert(fpFactoryMap != nullptr); size_t eraseRes = fpFactoryMap->erase(_key); assert(eraseRes == 1); (void) eraseRes; if (fpFactoryMap->empty()) { delete fpFactoryMap; - fpFactoryMap = NULL; + fpFactoryMap = nullptr; } } diff --git a/searchlib/src/vespa/searchlib/test/initrange.h b/searchlib/src/vespa/searchlib/test/initrange.h index 4237a0ac0f9..9431740ac08 100644 --- a/searchlib/src/vespa/searchlib/test/initrange.h +++ b/searchlib/src/vespa/searchlib/test/initrange.h @@ -5,8 +5,7 @@ #include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vector> -namespace search { -namespace test { +namespace search::test { class InitRangeVerifier { public: @@ -37,4 +36,3 @@ private: }; } -} |