summaryrefslogtreecommitdiffstats
path: root/searchlib/src
diff options
context:
space:
mode:
authorGeir Storli <geirst@verizonmedia.com>2019-11-01 08:48:06 +0000
committerGeir Storli <geirst@verizonmedia.com>2019-11-01 09:26:50 +0000
commita98348f736f16506a9e7d5fa80ddb029fd2162c8 (patch)
tree5e10a92b623a5f154715dd68d05d45d59bb1453d /searchlib/src
parent7274edd73bcbc1956b9956b6d15743a77dae53e9 (diff)
Fix setup of same element iterator to use the attribute search context from the child blueprint instead of the child search iterator.
This fixes a bug that occurs if the search iterator from a fast-search attribute is a bit vector iterator. The bit vector iterator doesn't expose the attribute search context, so the setup of the same element iterator doesn't wrap it into an attribute element iterator that handles finding which elements that match. The result is that the same element iterator will not match any documents.
Diffstat (limited to 'searchlib/src')
-rw-r--r--searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp15
-rw-r--r--searchlib/src/tests/attribute/searchcontext/searchcontext.cpp47
-rw-r--r--searchlib/src/tests/queryeval/fake_searchable/fake_searchable_test.cpp2
-rw-r--r--searchlib/src/vespa/searchlib/attribute/attributeiterators.h1
-rw-r--r--searchlib/src/vespa/searchlib/queryeval/fake_search.h1
-rw-r--r--searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp2
-rw-r--r--searchlib/src/vespa/searchlib/queryeval/searchiterator.cpp6
-rw-r--r--searchlib/src/vespa/searchlib/queryeval/searchiterator.h2
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;
};
}