diff options
8 files changed, 138 insertions, 65 deletions
diff --git a/searchcore/src/tests/proton/matching/query_test.cpp b/searchcore/src/tests/proton/matching/query_test.cpp index 8ecf6fd4a43..03f20876adc 100644 --- a/searchcore/src/tests/proton/matching/query_test.cpp +++ b/searchcore/src/tests/proton/matching/query_test.cpp @@ -56,6 +56,7 @@ using search::queryeval::Blueprint; using search::queryeval::FakeResult; using search::queryeval::FakeSearchable; using search::queryeval::FakeRequestContext; +using search::queryeval::FakeBlueprint; using search::queryeval::FieldSpec; using search::queryeval::FieldSpecList; using search::queryeval::Searchable; @@ -105,6 +106,7 @@ class Test : public vespalib::TestApp { void requireThatUnknownFieldActsEmpty(); void requireThatIllegalFieldsAreIgnored(); void requireThatQueryGluesEverythingTogether(); + void requireThatLocationIsAddedTheCorrectPlace(); void requireThatQueryAddsLocation(); void requireThatQueryAddsLocationCutoff(); void requireThatFakeFieldSearchDumpsDiffer(); @@ -119,7 +121,7 @@ class Test : public vespalib::TestApp { void requireThatSameElementIteratorsCanBeBuilt(); public: - ~Test(); + ~Test() override; int Main() override; }; @@ -129,16 +131,17 @@ public: TEST_DO(tearDown()) void Test::setUp() { - _match_data.reset(0); - _blueprint.reset(0); + _match_data.reset(); + _blueprint.reset(); } void Test::tearDown() { - _match_data.reset(0); - _blueprint.reset(0); + _match_data.reset(); + _blueprint.reset(); } const string field = "field"; +const string loc_field = "location"; const string resolved_field1 = "resolved1"; const string resolved_field2 = "resolved2"; const string unknown_field = "unknown_field"; @@ -173,6 +176,10 @@ void setupIndexEnvironments() FieldInfo attr_info(FieldType::ATTRIBUTE, CollectionType::SINGLE, field, 0); attribute_index_env.getFields().push_back(attr_info); + FieldInfo loc_field_info = FieldInfo(FieldType::ATTRIBUTE, CollectionType::SINGLE, + PositionDataType::getZCurveFieldName(loc_field), field_id + 1); + plain_index_env.getFields().push_back(loc_field_info); + attribute_index_env.getFields().push_back(loc_field_info); } Node::UP buildQueryTree(const ViewResolver &resolver, @@ -689,8 +696,6 @@ void Test::requireThatQueryGluesEverythingTogether() { } void checkQueryAddsLocation(Test &test, const string &loc_string) { - const string loc_field = "location"; - fef_test::IndexEnvironment index_environment; FieldInfo field_info(FieldType::INDEX, CollectionType::SINGLE, field, 0); index_environment.getFields().push_back(field_info); @@ -726,6 +731,60 @@ void checkQueryAddsLocation(Test &test, const string &loc_string) { } } +template<typename T1, typename T2> +void verifyThatRankBlueprintAndAndNotStaysOnTopAfterLocation(QueryBuilder<ProtonNodeTypes> & builder) { + const string loc_string = "(2,10,10,3,0,1,0,0)"; + builder.addStringTerm("foo", field, field_id, string_weight); + builder.addStringTerm("bar", field, field_id, string_weight); + builder.addStringTerm("baz", field, field_id, string_weight); + std::string stackDump = StackDumpCreator::create(*builder.build()); + + Query query; + query.buildTree(stackDump, loc_field + ":" + loc_string, ViewResolver(), attribute_index_env); + FakeSearchContext context(42); + context.addIdx(0).idx(0).getFake() + .addResult(field, "foo", FakeResult().doc(1)); + context.setLimit(42); + + query.setWhiteListBlueprint(std::make_unique<SimpleBlueprint>(SimpleResult())); + + FakeRequestContext requestContext; + MatchDataLayout mdl; + query.reserveHandles(requestContext, context, mdl); + const IntermediateBlueprint * root = dynamic_cast<const T1 *>(query.peekRoot()); + ASSERT_TRUE(root != nullptr); + EXPECT_EQUAL(2u, root->childCnt()); + const IntermediateBlueprint * second = dynamic_cast<const T2 *>(&root->getChild(0)); + ASSERT_TRUE(second != nullptr); + EXPECT_EQUAL(2u, second->childCnt()); + auto first = dynamic_cast<const AndBlueprint *>(&second->getChild(0)); + ASSERT_TRUE(first != nullptr); + EXPECT_EQUAL(2u, first->childCnt()); + EXPECT_TRUE(dynamic_cast<const AndBlueprint *>(&first->getChild(0))); + auto bottom = dynamic_cast<const AndBlueprint *>(&first->getChild(0)); + EXPECT_EQUAL(2u, bottom->childCnt()); + EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&bottom->getChild(0))); + EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&bottom->getChild(1))); + EXPECT_TRUE(dynamic_cast<const SimpleBlueprint *>(&first->getChild(1))); + EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&second->getChild(1))); + EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&root->getChild(1))); +} + +void Test::requireThatLocationIsAddedTheCorrectPlace() { + { + QueryBuilder<ProtonNodeTypes> builder; + builder.addRank(2); + builder.addAndNot(2); + verifyThatRankBlueprintAndAndNotStaysOnTopAfterLocation<RankBlueprint, AndNotBlueprint>(builder); + } + { + QueryBuilder<ProtonNodeTypes> builder; + builder.addAndNot(2); + builder.addRank(2); + verifyThatRankBlueprintAndAndNotStaysOnTopAfterLocation<AndNotBlueprint, RankBlueprint>(builder); + } +} + void Test::requireThatQueryAddsLocation() { checkQueryAddsLocation(*this, "(2,10,10,3,0,1,0,0)"); } @@ -817,8 +876,8 @@ void Test::requireThatWeakAndBlueprintsAreCreatedCorrectly() { wand.accept(reserve_visitor); Blueprint::UP blueprint = BlueprintBuilder::build(requestContext, wand, context); - WeakAndBlueprint *wbp = dynamic_cast<WeakAndBlueprint*>(blueprint.get()); - ASSERT_TRUE(wbp != 0); + auto *wbp = dynamic_cast<WeakAndBlueprint*>(blueprint.get()); + ASSERT_TRUE(wbp != nullptr); ASSERT_EQUAL(2u, wbp->getWeights().size()); ASSERT_EQUAL(2u, wbp->childCnt()); EXPECT_EQUAL(123u, wbp->getN()); @@ -851,7 +910,7 @@ void Test::requireThatParallelWandBlueprintsAreCreatedCorrectly() { wand.accept(reserve_visitor); Blueprint::UP blueprint = BlueprintBuilder::build(requestContext, wand, context); - ParallelWeakAndBlueprint *wbp = dynamic_cast<ParallelWeakAndBlueprint*>(blueprint.get()); + auto *wbp = dynamic_cast<ParallelWeakAndBlueprint*>(blueprint.get()); ASSERT_TRUE(wbp != nullptr); EXPECT_EQUAL(9000, wbp->getScoreThreshold()); EXPECT_EQUAL(1.25, wbp->getThresholdBoostFactor()); @@ -913,7 +972,7 @@ void verifyThatRankBlueprintAndAndNotStaysOnTopAfterWhiteListing(QueryBuilder<Pr const IntermediateBlueprint * second = dynamic_cast<const T2 *>(&root->getChild(0)); ASSERT_TRUE(second != nullptr); EXPECT_EQUAL(2u, second->childCnt()); - const AndBlueprint * first = dynamic_cast<const AndBlueprint *>(&second->getChild(0)); + auto first = dynamic_cast<const AndBlueprint *>(&second->getChild(0)); ASSERT_TRUE(first != nullptr); EXPECT_EQUAL(2u, first->childCnt()); EXPECT_TRUE(dynamic_cast<const SourceBlenderBlueprint *>(&first->getChild(0))); @@ -956,7 +1015,7 @@ void Test::requireThatSameElementTermsAreProperlyPrefixed() { search::query::Node::UP query = make_same_element_stack_dump("", ""); - search::query::SameElement * root = dynamic_cast<search::query::SameElement *>(query.get()); + auto * root = dynamic_cast<search::query::SameElement *>(query.get()); EXPECT_EQUAL(root->getView(), ""); EXPECT_EQUAL(root->getChildren().size(), 2u); EXPECT_EQUAL(dynamic_cast<ProtonStringTerm *>(root->getChildren()[0])->getView(), "f1"); @@ -1035,6 +1094,7 @@ Test::Main() TEST_CALL(requireThatUnknownFieldActsEmpty); TEST_CALL(requireThatIllegalFieldsAreIgnored); TEST_CALL(requireThatQueryGluesEverythingTogether); + TEST_CALL(requireThatLocationIsAddedTheCorrectPlace); TEST_CALL(requireThatQueryAddsLocation); TEST_CALL(requireThatQueryAddsLocationCutoff); TEST_CALL(requireThatFakeFieldSearchDumpsDiffer); diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index a8f75928a6d..351de63de37 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -40,7 +40,25 @@ using std::vector; namespace proton::matching { namespace { -void AddLocationNode(const string &location_str, Node::UP &query_tree, Location &fef_location) { + +Node::UP +inject(Node::UP query, Node::UP to_inject) { + if (auto * my_and = dynamic_cast<search::query::And *>(query.get())) { + my_and->append(std::move(to_inject)); + } else if (dynamic_cast<search::query::Rank *>(query.get()) || dynamic_cast<search::query::AndNot *>(query.get())) { + search::query::Intermediate & root = static_cast<search::query::Intermediate &>(*query); + root.prepend(inject(root.stealFirst(), std::move(to_inject))); + } else { + auto new_root = std::make_unique<ProtonAnd>(); + new_root->append(std::move(query)); + new_root->append(std::move(to_inject)); + query = std::move(new_root); + } + return query; +} + +void +addLocationNode(const string &location_str, Node::UP &query_tree, Location &fef_location) { if (location_str.empty()) { return; } @@ -61,20 +79,16 @@ void AddLocationNode(const string &location_str, Node::UP &query_tree, Location int32_t id = -1; Weight weight(100); - auto new_base = std::make_unique<ProtonAnd>(); - new_base->append(std::move(query_tree)); - if (locationSpec.getRankOnDistance()) { - new_base->append(std::make_unique<ProtonLocationTerm>(loc, view, id, weight)); + query_tree = inject(std::move(query_tree), std::make_unique<ProtonLocationTerm>(loc, view, id, weight)); fef_location.setAttribute(view); fef_location.setXPosition(locationSpec.getX()); fef_location.setYPosition(locationSpec.getY()); fef_location.setXAspect(locationSpec.getXAspect()); fef_location.setValid(true); } else if (locationSpec.getPruneOnDistance()) { - new_base->append(std::make_unique<ProtonLocationTerm>(loc, view, id, weight)); + query_tree = inject(std::move(query_tree), std::make_unique<ProtonLocationTerm>(loc, view, id, weight)); } - query_tree = std::move(new_base); } IntermediateBlueprint * @@ -111,7 +125,7 @@ Query::buildTree(vespalib::stringref stack, const string &location, if (_query_tree) { SameElementModifier prefixSameElementSubIndexes; _query_tree->accept(prefixSameElementSubIndexes); - AddLocationNode(location, _query_tree, _location); + addLocationNode(location, _query_tree, _location); ResolveViewVisitor resolve_visitor(resolver, indexEnv); _query_tree->accept(resolve_visitor); return true; diff --git a/searchlib/src/tests/query/querybuilder_test.cpp b/searchlib/src/tests/query/querybuilder_test.cpp index 0f05419c4e5..6673a107d44 100644 --- a/searchlib/src/tests/query/querybuilder_test.cpp +++ b/searchlib/src/tests/query/querybuilder_test.cpp @@ -522,13 +522,13 @@ TEST("require that Using Position Data Can Be Turned Off") { Node::UP node = builder.build(); ASSERT_TRUE(!builder.hasError()); Intermediate * andNode = dynamic_cast<Intermediate *>(node.get()); - ASSERT_TRUE(andNode != NULL); + ASSERT_TRUE(andNode != nullptr); ASSERT_TRUE(andNode->getChildren().size() == 2); Term * term = dynamic_cast<Term *>(andNode->getChildren()[0]); - ASSERT_TRUE(term != NULL); + ASSERT_TRUE(term != nullptr); EXPECT_TRUE(!term->usePositionData()); Phrase * phrase = dynamic_cast<Phrase *>(andNode->getChildren()[1]); - ASSERT_TRUE(phrase != NULL); + ASSERT_TRUE(phrase != nullptr); EXPECT_TRUE(!phrase->usePositionData()); } @@ -536,13 +536,11 @@ TEST("require that Weight Override Works Across Multiple Levels") { QueryBuilder<SimpleQueryNodeTypes> builder; builder.addPhrase(2, view[0], id[0], weight[0]); - SimpleStringTerm &string_term_1 = - builder.addStringTerm(str[1], view[1], id[1], weight[1]); + SimpleStringTerm &string_term_1 = builder.addStringTerm(str[1], view[1], id[1], weight[1]); EXPECT_EQUAL(weight[0].percent(), string_term_1.getWeight().percent()); builder.addAnd(2); - SimpleStringTerm &string_term_2 = - builder.addStringTerm(str[2], view[2], id[2], weight[2]); + SimpleStringTerm &string_term_2 = builder.addStringTerm(str[2], view[2], id[2], weight[2]); EXPECT_EQUAL(weight[0].percent(), string_term_2.getWeight().percent()); } @@ -579,14 +577,12 @@ TEST("require that All Range Syntaxes Work") { string stackDump = StackDumpCreator::create(*node); SimpleQueryStackDumpIterator iterator(stackDump); - Node::UP new_node = - QueryTreeCreator<SimpleQueryNodeTypes>::create(iterator); + Node::UP new_node = QueryTreeCreator<SimpleQueryNodeTypes>::create(iterator); And *and_node = dynamic_cast<And *>(new_node.get()); ASSERT_TRUE(and_node); EXPECT_EQUAL(3u, and_node->getChildren().size()); - RangeTerm *range_term = - dynamic_cast<RangeTerm *>(and_node->getChildren()[0]); + auto range_term = dynamic_cast<RangeTerm *>(and_node->getChildren()[0]); ASSERT_TRUE(range_term); EXPECT_TRUE(range0 == range_term->getTerm()); @@ -608,8 +604,7 @@ TEST("require that empty intermediate node can be added") { string stackDump = StackDumpCreator::create(*node); SimpleQueryStackDumpIterator iterator(stackDump); - Node::UP new_node = - QueryTreeCreator<SimpleQueryNodeTypes>::create(iterator); + Node::UP new_node = QueryTreeCreator<SimpleQueryNodeTypes>::create(iterator); And *and_node = dynamic_cast<And *>(new_node.get()); ASSERT_TRUE(and_node); EXPECT_EQUAL(0u, and_node->getChildren().size()); diff --git a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp index b9e3377d8f9..755245438f2 100644 --- a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp @@ -4,9 +4,7 @@ #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/vespalib/util/stringfmt.h> -namespace search { -namespace fef { -namespace test { +namespace search::fef::test { using vespalib::eval::ValueType; using vespalib::eval::ErrorValue; @@ -18,21 +16,14 @@ IndexEnvironment::Constant notFoundError(ValueType::error_type(), } -IndexEnvironment::IndexEnvironment() : - _properties(), - _fields(), - _attrMap(), - _tableMan(), - _constants() -{ -} +IndexEnvironment::IndexEnvironment() = default; -IndexEnvironment::~IndexEnvironment() {} +IndexEnvironment::~IndexEnvironment() = default; const FieldInfo * IndexEnvironment::getField(uint32_t id) const { - return id < _fields.size() ? &_fields[id] : NULL; + return id < _fields.size() ? &_fields[id] : nullptr; } const FieldInfo * @@ -44,7 +35,7 @@ IndexEnvironment::getFieldByName(const string &name) const return &(*it); } } - return NULL; + return nullptr; } @@ -71,6 +62,4 @@ IndexEnvironment::addConstantValue(const vespalib::string &name, (void) insertRes; } -} // namespace test -} // namespace fef -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/query/tree/intermediate.cpp b/searchlib/src/vespa/searchlib/query/tree/intermediate.cpp index f56da9c2cf9..a5cfe69c2d5 100644 --- a/searchlib/src/vespa/searchlib/query/tree/intermediate.cpp +++ b/searchlib/src/vespa/searchlib/query/tree/intermediate.cpp @@ -9,10 +9,29 @@ Intermediate::~Intermediate() { } } -Intermediate &Intermediate::append(Node::UP child) +Intermediate & +Intermediate::append(Node::UP child) { _children.push_back(child.release()); return *this; } +Intermediate & +Intermediate::prepend(Node::UP child) +{ + _children.insert(_children.begin(), child.release()); + return *this; +} + +Node::UP +Intermediate::stealFirst() +{ + if ( _children.empty()) { + return Node::UP(); + } + Node::UP first(_children.front()); + _children.erase(_children.begin()); + return first; +} + } diff --git a/searchlib/src/vespa/searchlib/query/tree/intermediate.h b/searchlib/src/vespa/searchlib/query/tree/intermediate.h index 052dc1db269..2f4323f8e87 100644 --- a/searchlib/src/vespa/searchlib/query/tree/intermediate.h +++ b/searchlib/src/vespa/searchlib/query/tree/intermediate.h @@ -20,7 +20,9 @@ class Intermediate : public Node const std::vector<Node *> &getChildren() const { return _children; } Intermediate &reserve(size_t sz) { _children.reserve(sz); return *this; } + Intermediate &prepend(Node::UP child); Intermediate &append(Node::UP child); + Node::UP stealFirst(); }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/fake_searchable.cpp b/searchlib/src/vespa/searchlib/queryeval/fake_searchable.cpp index 212c9439e6b..997f9afc54f 100644 --- a/searchlib/src/vespa/searchlib/queryeval/fake_searchable.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/fake_searchable.cpp @@ -17,8 +17,7 @@ using search::query::StringTerm; using search::query::SubstringTerm; using search::query::SuffixTerm; -namespace search { -namespace queryeval { +namespace search::queryeval { FakeSearchable::FakeSearchable() : _tag("<undef>"), @@ -74,7 +73,7 @@ LookupVisitor<Map>::LookupVisitor(Searchable &searchable, const IRequestContext {} template <class Map> -LookupVisitor<Map>::~LookupVisitor() { } +LookupVisitor<Map>::~LookupVisitor() = default; template <class Map> template <class TermNode> @@ -83,15 +82,13 @@ LookupVisitor<Map>::visitTerm(TermNode &n) { const vespalib::string term_string = termAsString(n); FakeResult result; - typename Map::const_iterator pos = - _map.find(typename Map::key_type(getField().getName(), term_string)); + auto pos = _map.find(typename Map::key_type(getField().getName(), term_string)); if (pos != _map.end()) { result = pos->second; } - FakeBlueprint *fake = new FakeBlueprint(getField(), result); - Blueprint::UP b(fake); + auto fake = std::make_unique<FakeBlueprint>(getField(), result); fake->tag(_tag).term(term_string); - setResult(std::move(b)); + setResult(std::move(fake)); } } // namespace search::queryeval::<unnamed> @@ -106,9 +103,6 @@ FakeSearchable::createBlueprint(const IRequestContext & requestContext, return visitor.getResult(); } -FakeSearchable::~FakeSearchable() -{ -} +FakeSearchable::~FakeSearchable() = default; -} // namespace search::queryeval -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp index 5cb51aa4fd0..0a11203c390 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp @@ -208,7 +208,7 @@ AndBlueprint::inheritStrict(size_t i) const SearchIterator::UP AndBlueprint::createIntermediateSearch(const MultiSearch::Children &subSearches, - bool strict, search::fef::MatchData & md) const + bool strict, search::fef::MatchData & md) const { UnpackInfo unpackInfo(calculateUnpackInfo(md)); AndSearch * search = 0; |