diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2018-12-06 17:49:31 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-12-06 17:49:31 +0100 |
commit | 255eb258cd6508a34c202c77af645bbf81bf6e41 (patch) | |
tree | 6a8ad7a36555a8492af2ff9c08dd5ce08a179a7d /searchlib | |
parent | 7fd1c444a79a3b5778c5d09fa0008d768435890e (diff) | |
parent | 5c6f284e90b63875beea110dc9822be27a41c180 (diff) |
Merge pull request #7889 from vespa-engine/balder/test-and-fix-postinglist-fallback-iterator
Balder/test and fix postinglist fallback iterator
Diffstat (limited to 'searchlib')
6 files changed, 45 insertions, 37 deletions
diff --git a/searchlib/src/tests/attribute/searchcontext/searchcontext.cpp b/searchlib/src/tests/attribute/searchcontext/searchcontext.cpp index 167ffec7084..29849139fc5 100644 --- a/searchlib/src/tests/attribute/searchcontext/searchcontext.cpp +++ b/searchlib/src/tests/attribute/searchcontext/searchcontext.cpp @@ -71,8 +71,8 @@ public: } }; -DocSet::DocSet() : std::set<uint32_t>() {} -DocSet::~DocSet() {} +DocSet::DocSet() = default; +DocSet::~DocSet() = default; template <typename V, typename T> class PostingList @@ -97,7 +97,7 @@ template <typename V, typename T> PostingList<V, T>::PostingList(V & vec, T value) : _vec(&vec), _value(value), _hits() {} template <typename V, typename T> -PostingList<V, T>::~PostingList() {} +PostingList<V, T>::~PostingList() = default; class DocRange { @@ -145,7 +145,7 @@ private: void checkResultSet(const ResultSet & rs, const DocSet & exp, bool bitVector); template<typename T, typename A> - void testSearchIterator(T key, const vespalib::string &keyAsString, const ConfigMap &cfgs); + void testSearchIterator(const std::vector<T> & keys, const vespalib::string &keyAsString, const ConfigMap &cfgs); void testSearchIteratorConformance(); // test search functionality template <typename V, typename T> @@ -169,25 +169,25 @@ private: class AttributeIteratorTester : public IteratorTester { public: - virtual bool matches(const SearchIterator & base) const override { - return dynamic_cast<const AttributeIterator *>(&base) != NULL; + bool matches(const SearchIterator & base) const override { + return dynamic_cast<const AttributeIterator *>(&base) != nullptr; } }; class FlagAttributeIteratorTester : public IteratorTester { public: - virtual bool matches(const SearchIterator & base) const override { - return (dynamic_cast<const FlagAttributeIterator *>(&base) != NULL) || - (dynamic_cast<const BitVectorIterator *>(&base) != NULL) || - (dynamic_cast<const queryeval::EmptySearch *>(&base) != NULL); + bool matches(const SearchIterator & base) const override { + return (dynamic_cast<const FlagAttributeIterator *>(&base) != nullptr) || + (dynamic_cast<const BitVectorIterator *>(&base) != nullptr) || + (dynamic_cast<const queryeval::EmptySearch *>(&base) != nullptr); } }; class AttributePostingListIteratorTester : public IteratorTester { public: - virtual bool matches(const SearchIterator & base) const override { - return dynamic_cast<const AttributePostingListIterator *>(&base) != NULL || - dynamic_cast<const queryeval::EmptySearch *>(&base) != NULL; + bool matches(const SearchIterator & base) const override { + return dynamic_cast<const AttributePostingListIterator *>(&base) != nullptr || + dynamic_cast<const queryeval::EmptySearch *>(&base) != nullptr; } }; @@ -473,7 +473,7 @@ SearchContextTest::checkResultSet(const ResultSet & rs, const DocSet & expected, if (bitVector) { const BitVector * vec = rs.getBitOverflow(); if (expected.size() != 0) { - ASSERT_TRUE(vec != NULL); + ASSERT_TRUE(vec != nullptr); for (const auto & expect : expected) { EXPECT_TRUE(vec->testBit(expect)); } @@ -481,7 +481,7 @@ SearchContextTest::checkResultSet(const ResultSet & rs, const DocSet & expected, } else { const RankedHit * array = rs.getArray(); if (expected.size() != 0) { - ASSERT_TRUE(array != NULL); + ASSERT_TRUE(array != nullptr); uint32_t i = 0; for (DocSet::const_iterator iter = expected.begin(); iter != expected.end(); ++iter, ++i) @@ -618,11 +618,12 @@ void SearchContextTest::testSearch(const ConfigMap & cfgs) { template<typename T, typename A> class Verifier : public search::test::SearchIteratorVerifier { public: - Verifier(T key, const vespalib::string & keyAsString, const vespalib::string & name, + Verifier(const std::vector<T> & keys, const vespalib::string & keyAsString, const vespalib::string & name, const Config & cfg, bool withElementId); ~Verifier(); SearchIterator::UP create(bool strict) const override { + _sc->fetchPostings(strict); auto search = _sc->createIterator(&_dummy, strict); if (_withElementId) { search = std::make_unique<attribute::ElementIterator>(std::move(search), *_sc, _dummy); @@ -637,42 +638,51 @@ private: }; template<typename T, typename A> -Verifier<T, A>::Verifier(T key, const vespalib::string & keyAsString, const vespalib::string & name, +Verifier<T, A>::Verifier(const std::vector<T> & keys, const vespalib::string & keyAsString, const vespalib::string & name, const Config & cfg, bool withElementId) : _withElementId(withElementId), _attribute(AttributeFactory::createAttribute(name + "-initrange", cfg)), _sc() { SearchContextTest::addDocs(*_attribute, getDocIdLimit()); + size_t i(0); for (uint32_t doc : getExpectedDocIds()) { EXPECT_TRUE(nullptr != dynamic_cast<A *>(_attribute.get())); - EXPECT_TRUE(dynamic_cast<A *>(_attribute.get())->update(doc, key)); + EXPECT_TRUE(dynamic_cast<A *>(_attribute.get())->update(doc, keys[(i++)%keys.size()])); } _attribute->commit(true); _sc = SearchContextTest::getSearch(*_attribute, keyAsString); ASSERT_TRUE(_sc->valid()); - _sc->fetchPostings(true); } template<typename T, typename A> Verifier<T, A>::~Verifier() = default; template<typename T, typename A> -void SearchContextTest::testSearchIterator(T key, const vespalib::string &keyAsString, const ConfigMap &cfgs) { +void SearchContextTest::testSearchIterator(const std::vector<T> & keys, const vespalib::string &keyAsString, const ConfigMap &cfgs) { for (bool withElementId : {false, true} ) { for (const auto & cfg : cfgs) { - Verifier<T, A> verifier(key, keyAsString, cfg.first, cfg.second, withElementId); - verifier.verify(); + { + Verifier<T, A> verifier(keys, keyAsString, cfg.first, cfg.second, withElementId); + verifier.verify(); + } + { + Config withFilter(cfg.second); + withFilter.setIsFilter(true); + Verifier<T, A> verifier(keys, keyAsString, cfg.first + "-filter", withFilter, withElementId); + verifier.verify(); + } } } } void SearchContextTest::testSearchIteratorConformance() { - testSearchIterator<AttributeVector::largeint_t, IntegerAttribute>(42, "42", _integerCfg); - testSearchIterator<double, FloatingPointAttribute>(42.42, "42.42", _floatCfg); - testSearchIterator<vespalib::string, StringAttribute>("any-key", "any-key", _stringCfg); + testSearchIterator<AttributeVector::largeint_t, IntegerAttribute>({42,45,46}, "[0;100]", _integerCfg); + testSearchIterator<AttributeVector::largeint_t, IntegerAttribute>({42}, "42", _integerCfg); + testSearchIterator<double, FloatingPointAttribute>({42.42}, "42.42", _floatCfg); + testSearchIterator<vespalib::string, StringAttribute>({"any-key"}, "any-key", _stringCfg); } void diff --git a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.h b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.h index b25438f8f2a..08233c09ef8 100644 --- a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.h @@ -135,7 +135,7 @@ protected: return -1; } - bool + int32_t find(DocId doc, int32_t elemId) const { WeightedIndexArrayRef indices(_toBeSearched._mvMapping.get(doc)); diff --git a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp index d94a59555bd..6a212b8d8b6 100644 --- a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp @@ -192,9 +192,9 @@ MultiValueNumericEnumAttribute<B, M>::ArraySearchContext::createFilterIterator(f template <typename B, typename M> MultiValueNumericEnumAttribute<B, M>::ArraySearchContext::ArraySearchContext(QueryTermSimpleUP qTerm, const NumericAttribute & toBeSearched) : - NumericAttribute::Range<T>(*qTerm), - SearchContext(toBeSearched), - _toBeSearched(static_cast<const MultiValueNumericEnumAttribute<B, M> &>(toBeSearched)) + NumericAttribute::Range<T>(*qTerm), + SearchContext(toBeSearched), + _toBeSearched(static_cast<const MultiValueNumericEnumAttribute<B, M> &>(toBeSearched)) { } template <typename B, typename M> diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp index 4c2c0043de8..b520b53551c 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp @@ -40,9 +40,7 @@ PostingListSearchContext(const Dictionary &dictionary, } -PostingListSearchContext::~PostingListSearchContext() -{ -} +PostingListSearchContext::~PostingListSearchContext() = default; void diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp index cf9450cbcca..15ff9c1ea7a 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp @@ -113,12 +113,12 @@ template <typename DataT> void PostingListSearchContextT<DataT>::fetchPostings(bool strict) { - assert (! _fetchPostingsDone); + if (_fetchPostingsDone) return; _fetchPostingsDone = true; - if (_uniqueValues < 2u) { - return; - } + + if (_uniqueValues < 2u) return; + if (strict && !fallbackToFiltering()) { size_t sum(countHits()); if (sum < _docIdLimit / 64) { diff --git a/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp b/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp index 59b4e2ab64c..fce768ab0d2 100644 --- a/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp +++ b/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp @@ -114,7 +114,7 @@ SearchIteratorVerifier::createFullIterator() const { return make_unique<TrueSearch>(_trueTfmd); } -SearchIteratorVerifier::~SearchIteratorVerifier() { } +SearchIteratorVerifier::~SearchIteratorVerifier() = default; void SearchIteratorVerifier::verify() const { |