diff options
37 files changed, 175 insertions, 80 deletions
diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index 70f6cf73a72..ca689d4c4c6 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -99,9 +99,9 @@ vespalib::string make_simple_stack_dump(const vespalib::string &field, const ves vespalib::string make_same_element_stack_dump(const vespalib::string &a1_term, const vespalib::string &f1_term) { QueryBuilder<ProtonNodeTypes> builder; - builder.addSameElement(2, "my"); - builder.addStringTerm(a1_term, "a1", 1, search::query::Weight(1)); - builder.addStringTerm(f1_term, "f1", 2, search::query::Weight(1)); + builder.addSameElement(2, "my", 0, Weight(1)); + builder.addStringTerm(a1_term, "a1", 1, Weight(1)); + builder.addStringTerm(f1_term, "f1", 2, Weight(1)); return StackDumpCreator::create(*builder.build()); } diff --git a/searchcore/src/tests/proton/matching/query_test.cpp b/searchcore/src/tests/proton/matching/query_test.cpp index 4c81aa7e347..575a52d01fb 100644 --- a/searchcore/src/tests/proton/matching/query_test.cpp +++ b/searchcore/src/tests/proton/matching/query_test.cpp @@ -119,7 +119,7 @@ class Test : public vespalib::TestApp { void requireThatRankBlueprintStaysOnTopAfterWhiteListing(); void requireThatAndNotBlueprintStaysOnTopAfterWhiteListing(); void requireThatSameElementTermsAreProperlyPrefixed(); - void requireThatSameElementDoesNotAllocateMatchData(); + void requireThatSameElementAllocatesMatchData(); void requireThatSameElementIteratorsCanBeBuilt(); void requireThatConstBoolBlueprintsAreCreatedCorrectly(); void global_filter_is_calculated_and_handled(); @@ -225,7 +225,7 @@ Node::UP buildSameElementQueryTree(const ViewResolver &resolver, const search::fef::IIndexEnvironment &idxEnv) { QueryBuilder<ProtonNodeTypes> query_builder; - query_builder.addSameElement(2, field); + query_builder.addSameElement(2, field, 2, Weight(0)); query_builder.addStringTerm(string_term, field, 0, Weight(0)); query_builder.addStringTerm(prefix_term, field, 1, Weight(0)); Node::UP node = query_builder.build(); @@ -1026,9 +1026,9 @@ search::query::Node::UP make_same_element_stack_dump(const vespalib::string &prefix, const vespalib::string &term_prefix) { QueryBuilder<ProtonNodeTypes> builder; - builder.addSameElement(2, prefix); - builder.addStringTerm("xyz", term_prefix + "f1", 1, search::query::Weight(1)); - builder.addStringTerm("abc", term_prefix + "f2", 2, search::query::Weight(1)); + builder.addSameElement(2, prefix, 0, Weight(1)); + builder.addStringTerm("xyz", term_prefix + "f1", 1, Weight(1)); + builder.addStringTerm("abc", term_prefix + "f2", 2, Weight(1)); vespalib::string stack = StackDumpCreator::create(*builder.build()); search::SimpleQueryStackDumpIterator stack_dump_iterator(stack); SameElementModifier sem; @@ -1070,14 +1070,14 @@ Test::requireThatSameElementTermsAreProperlyPrefixed() } void -Test::requireThatSameElementDoesNotAllocateMatchData() +Test::requireThatSameElementAllocatesMatchData() { Node::UP node = buildSameElementQueryTree(ViewResolver(), plain_index_env); MatchDataLayout mdl; MatchDataReserveVisitor visitor(mdl); node->accept(visitor); MatchData::UP match_data = mdl.createMatchData(); - EXPECT_EQUAL(0u, match_data->getNumTermFields()); + EXPECT_EQUAL(1u, match_data->getNumTermFields()); } void @@ -1215,7 +1215,7 @@ Test::Main() TEST_CALL(requireThatRankBlueprintStaysOnTopAfterWhiteListing); TEST_CALL(requireThatAndNotBlueprintStaysOnTopAfterWhiteListing); TEST_CALL(requireThatSameElementTermsAreProperlyPrefixed); - TEST_CALL(requireThatSameElementDoesNotAllocateMatchData); + TEST_CALL(requireThatSameElementAllocatesMatchData); TEST_CALL(requireThatSameElementIteratorsCanBeBuilt); TEST_CALL(requireThatConstBoolBlueprintsAreCreatedCorrectly); TEST_CALL(global_filter_is_calculated_and_handled); diff --git a/searchcore/src/tests/proton/matching/querynodes_test.cpp b/searchcore/src/tests/proton/matching/querynodes_test.cpp index c73483efed5..15fcc8a3fd7 100644 --- a/searchcore/src/tests/proton/matching/querynodes_test.cpp +++ b/searchcore/src/tests/proton/matching/querynodes_test.cpp @@ -214,7 +214,7 @@ public: using QB = QueryBuilder<ProtonNodeTypes>; struct Phrase { void addToBuilder(QB& b) { b.addPhrase(2, view, id, weight); }}; -struct SameElement { void addToBuilder(QB& b) { b.addSameElement(2, view); }}; +struct SameElement { void addToBuilder(QB& b) { b.addSameElement(2, view, id, weight); }}; struct Near { void addToBuilder(QB& b) { b.addNear(2, distance); } }; struct ONear { void addToBuilder(QB& b) { b.addONear(2, distance); } }; struct Or { void addToBuilder(QB& b) { b.addOr(2); } }; @@ -301,7 +301,7 @@ SearchIterator *getParent<SameElement>(SearchIterator *a, SearchIterator *b) { // we only check how many term/field combinations // are below the SameElement parent: // two terms searching in one index field - return new SameElementSearch(nullptr, std::move(children), true); + return new SameElementSearch(tmd, nullptr, std::move(children), true); } template <> diff --git a/searchcore/src/tests/proton/matching/resolveviewvisitor_test.cpp b/searchcore/src/tests/proton/matching/resolveviewvisitor_test.cpp index c9216c85af2..a47531c5575 100644 --- a/searchcore/src/tests/proton/matching/resolveviewvisitor_test.cpp +++ b/searchcore/src/tests/proton/matching/resolveviewvisitor_test.cpp @@ -136,12 +136,13 @@ TEST_F("require that equiv nodes resolve view from children", Fixture) { EXPECT_EQUAL(field2, base.field(1).field_name); } -TEST_F("require that view is resolved for SameElement children", Fixture) { +TEST_F("require that view is resolved for SameElement and its children", Fixture) { ViewResolver resolver; resolver.add(view, field1); + resolver.add("view2", field2); QueryBuilder<ProtonNodeTypes> builder; - builder.addSameElement(2, ""); + ProtonSameElement &same_elem = builder.addSameElement(2, "view2", 13, weight); ProtonStringTerm &my_term = builder.addStringTerm(term, view, 42, weight); builder.addStringTerm(term, field2, 43, weight); Node::UP node = builder.build(); @@ -149,6 +150,9 @@ TEST_F("require that view is resolved for SameElement children", Fixture) { ResolveViewVisitor visitor(resolver, f.index_environment); node->accept(visitor); + ASSERT_EQUAL(1u, same_elem.numFields()); + EXPECT_EQUAL(field2, same_elem.field(0).field_name); + ASSERT_EQUAL(1u, my_term.numFields()); EXPECT_EQUAL(field1, my_term.field(0).field_name); } diff --git a/searchcore/src/tests/proton/matching/same_element_builder/same_element_builder_test.cpp b/searchcore/src/tests/proton/matching/same_element_builder/same_element_builder_test.cpp index cb2fa52767a..e52995c1bd2 100644 --- a/searchcore/src/tests/proton/matching/same_element_builder/same_element_builder_test.cpp +++ b/searchcore/src/tests/proton/matching/same_element_builder/same_element_builder_test.cpp @@ -35,6 +35,7 @@ using search::queryeval::Blueprint; using search::queryeval::EmptyBlueprint; using search::queryeval::FakeBlueprint; using search::queryeval::FakeRequestContext; +using search::queryeval::FieldSpec; using search::queryeval::IntermediateBlueprint; using search::queryeval::SameElementBlueprint; @@ -89,8 +90,9 @@ struct FakeTerms { struct BuilderFixture { FakeRequestContext req_ctx; FakeSearchContext ctx; + FieldSpec field; SameElementBuilder builder; - BuilderFixture() : req_ctx(), ctx(), builder(req_ctx, ctx, "foo", false) { + BuilderFixture() : req_ctx(), ctx(), field("foo", 3, 5), builder(req_ctx, ctx, field, false) { ctx.attr().tag("attr"); ctx.addIdx(0).idx(0).getFake().tag("idx"); } @@ -104,9 +106,14 @@ const FakeBlueprint *as_fake(const Blueprint *bp) { return dynamic_cast<const FakeBlueprint*>(bp); } -void verify_children(Blueprint *bp, std::initializer_list<const char *> tags) { +void verify_blueprint(Blueprint *bp, std::initializer_list<const char *> tags) { SameElementBlueprint *se = dynamic_cast<SameElementBlueprint*>(bp); ASSERT_TRUE(se != nullptr); + ASSERT_EQUAL(1u, se->getState().numFields()); + const auto &field = se->getState().field(0); + EXPECT_EQUAL(3u, field.getFieldId()); + EXPECT_EQUAL(5u, field.getHandle()); + EXPECT_FALSE(field.isFilter()); ASSERT_EQUAL(se->terms().size(), tags.size()); size_t idx = 0; for (const char *tag: tags) { @@ -120,7 +127,7 @@ TEST_FF("require that same element blueprint can be built", BuilderFixture(), Fa f1.builder.add_child(f2.idx_string_term); f1.builder.add_child(f2.attr_string_term); Blueprint::UP result = f1.builder.build(); - TEST_DO(verify_children(result.get(), {"idx", "attr"})); + TEST_DO(verify_blueprint(result.get(), {"idx", "attr"})); } TEST_FF("require that terms searching multiple fields are ignored", BuilderFixture(), FakeTerms()) { @@ -128,7 +135,7 @@ TEST_FF("require that terms searching multiple fields are ignored", BuilderFixtu f1.builder.add_child(f2.attr_string_term); f1.builder.add_child(f2.both_string_term); // ignored Blueprint::UP result = f1.builder.build(); - TEST_DO(verify_children(result.get(), {"idx", "attr"})); + TEST_DO(verify_blueprint(result.get(), {"idx", "attr"})); } TEST_FF("require that all relevant term types can be used", BuilderFixture(), FakeTerms()) { @@ -141,7 +148,7 @@ TEST_FF("require that all relevant term types can be used", BuilderFixture(), Fa f1.builder.add_child(f2.attr_suffix_term); f1.builder.add_child(f2.attr_regexp_term); Blueprint::UP result = f1.builder.build(); - TEST_DO(verify_children(result.get(), {"idx", "idx", "idx", "idx", "attr", "attr", "attr", "attr"})); + TEST_DO(verify_blueprint(result.get(), {"idx", "idx", "idx", "idx", "attr", "attr", "attr", "attr"})); } TEST_F("require that building same element with no children gives EmptyBlueprint", BuilderFixture()) { diff --git a/searchcore/src/tests/proton/matching/termdataextractor_test.cpp b/searchcore/src/tests/proton/matching/termdataextractor_test.cpp index 443cc1686b6..1f96eb5d9d6 100644 --- a/searchcore/src/tests/proton/matching/termdataextractor_test.cpp +++ b/searchcore/src/tests/proton/matching/termdataextractor_test.cpp @@ -130,11 +130,11 @@ TEST("requireThatNegativeTermsAreSkipped") { EXPECT_EQUAL(id[1], term_data[1]->getUniqueId()); } -TEST("requireThatSameElementIsSkipped") +TEST("requireThatSameElementIsExtractedAsOneTerm") { QueryBuilder<ProtonNodeTypes> query_builder; query_builder.addAnd(2); - query_builder.addSameElement(2, field); + query_builder.addSameElement(2, field, id[3], Weight(7)); query_builder.addStringTerm("term1", field, id[0], Weight(1)); query_builder.addStringTerm("term2", field, id[1], Weight(1)); query_builder.addStringTerm("term3", field, id[2], Weight(1)); @@ -142,9 +142,9 @@ TEST("requireThatSameElementIsSkipped") vector<const ITermData *> term_data; TermDataExtractor::extractTerms(*node, term_data); - EXPECT_EQUAL(1u, term_data.size()); - ASSERT_TRUE(term_data.size() >= 1); - EXPECT_EQUAL(id[2], term_data[0]->getUniqueId()); + ASSERT_EQUAL(2u, term_data.size()); + EXPECT_EQUAL(id[3], term_data[0]->getUniqueId()); + EXPECT_EQUAL(id[2], term_data[1]->getUniqueId()); } } // namespace diff --git a/searchcore/src/tests/proton/matching/unpacking_iterators_optimizer/unpacking_iterators_optimizer_test.cpp b/searchcore/src/tests/proton/matching/unpacking_iterators_optimizer/unpacking_iterators_optimizer_test.cpp index 279db26f946..1bd7a9bec9f 100644 --- a/searchcore/src/tests/proton/matching/unpacking_iterators_optimizer/unpacking_iterators_optimizer_test.cpp +++ b/searchcore/src/tests/proton/matching/unpacking_iterators_optimizer/unpacking_iterators_optimizer_test.cpp @@ -99,7 +99,7 @@ void add_phrase(QueryBuilder<ProtonNodeTypes> &builder) { } void add_same_element(QueryBuilder<ProtonNodeTypes> &builder) { - builder.addSameElement(2, view); + builder.addSameElement(2, view, id, weight); { builder.addStringTerm("x", view, id, weight); builder.addStringTerm("y", view, id, weight); diff --git a/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.cpp b/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.cpp index 7573137d98c..2d3323949d1 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/blueprintbuilder.cpp @@ -10,6 +10,7 @@ #include <vespa/searchlib/queryeval/intermediate_blueprints.h> #include <vespa/searchlib/queryeval/equiv_blueprint.h> #include <vespa/searchlib/queryeval/get_weight_from_node.h> +#include <vespa/vespalib/util/issue.h> using namespace search::queryeval; @@ -102,11 +103,16 @@ private: } void buildSameElement(ProtonSameElement &n) { - SameElementBuilder builder(_requestContext, _context, n.getView(), n.is_expensive()); - for (search::query::Node *node : n.getChildren()) { - builder.add_child(*node); + if (n.numFields() == 1) { + SameElementBuilder builder(_requestContext, _context, n.field(0).fieldSpec(), n.is_expensive()); + for (search::query::Node *node: n.getChildren()) { + builder.add_child(*node); + } + _result = builder.build(); + } else { + vespalib::Issue::report("SameElement operator searches in unexpected number of fields. Expected 1 but was %zu", n.numFields()); + _result = std::make_unique<EmptyBlueprint>(); } - _result = builder.build(); } template <typename NodeType> diff --git a/searchcore/src/vespa/searchcore/proton/matching/docsum_matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/docsum_matcher.cpp index 032991cbf6a..fd5c3782b9a 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/docsum_matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/docsum_matcher.cpp @@ -59,7 +59,8 @@ template<typename T> const T *as(const Blueprint &bp) { return dynamic_cast<const T *>(&bp); } void find_matching_elements(const std::vector<uint32_t> &docs, const SameElementBlueprint &same_element, MatchingElements &result) { - auto search = same_element.create_same_element_search(false); + search::fef::TermFieldMatchData dummy_tfmd; + auto search = same_element.create_same_element_search(dummy_tfmd, false); search->initRange(docs.front(), docs.back()+1); std::vector<uint32_t> matches; for (uint32_t i = 0; i < docs.size(); ++i) { diff --git a/searchcore/src/vespa/searchcore/proton/matching/matchdatareservevisitor.h b/searchcore/src/vespa/searchcore/proton/matching/matchdatareservevisitor.h index ee0b4fba183..bf6e6346e24 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matchdatareservevisitor.h +++ b/searchcore/src/vespa/searchcore/proton/matching/matchdatareservevisitor.h @@ -27,7 +27,6 @@ public: } n.allocateTerms(_mdl); } - void visit(ProtonNodeTypes::SameElement &) override { } MatchDataReserveVisitor(search::fef::MatchDataLayout &mdl) : _mdl(mdl) {} }; diff --git a/searchcore/src/vespa/searchcore/proton/matching/querynodes.h b/searchcore/src/vespa/searchcore/proton/matching/querynodes.h index 20d602b7814..fc3f12af4f7 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/querynodes.h +++ b/searchcore/src/vespa/searchcore/proton/matching/querynodes.h @@ -114,7 +114,6 @@ using ProtonONear = search::query::SimpleONear; using ProtonOr = search::query::SimpleOr; using ProtonRank = search::query::SimpleRank; using ProtonWeakAnd = search::query::SimpleWeakAnd; -using ProtonSameElement = search::query::SimpleSameElement; using ProtonTrue = search::query::SimpleTrue; using ProtonFalse = search::query::SimpleFalse; @@ -123,6 +122,10 @@ struct ProtonEquiv final : public ProtonTermBase<search::query::Equiv> { using ProtonTermBase::ProtonTermBase; }; +struct ProtonSameElement final : public ProtonTermBase<search::query::SameElement> { + using ProtonTermBase::ProtonTermBase; +}; + struct ProtonNearestNeighborTerm : public ProtonTermBase<search::query::NearestNeighborTerm> { using ProtonTermBase::ProtonTermBase; std::optional<vespalib::string> query_tensor_name() const override { diff --git a/searchcore/src/vespa/searchcore/proton/matching/resolveviewvisitor.h b/searchcore/src/vespa/searchcore/proton/matching/resolveviewvisitor.h index a24dcc96b8e..3f778cc05f0 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/resolveviewvisitor.h +++ b/searchcore/src/vespa/searchcore/proton/matching/resolveviewvisitor.h @@ -28,6 +28,11 @@ public: visitChildren(n); n.resolveFromChildren(n.getChildren()); } + + void visit(ProtonNodeTypes::SameElement &n) override { + visitChildren(n); + visitTerm(n); + } }; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.cpp b/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.cpp index 574df384a63..5f77cfefcdf 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.cpp @@ -75,10 +75,10 @@ public: } // namespace proton::matching::<unnamed> SameElementBuilder::SameElementBuilder(const search::queryeval::IRequestContext &requestContext, ISearchContext &context, - const vespalib::string &struct_field_name, bool expensive) + const search::queryeval::FieldSpec &field, bool expensive) : _requestContext(requestContext), _context(context), - _result(std::make_unique<SameElementBlueprint>(struct_field_name, expensive)) + _result(std::make_unique<SameElementBlueprint>(field, expensive)) { } @@ -92,7 +92,7 @@ SameElementBuilder::add_child(search::query::Node &node) Blueprint::UP SameElementBuilder::build() { - if (!_result || _result->terms().empty()) { + if (_result->terms().empty()) { return std::make_unique<EmptyBlueprint>(); } return std::move(_result); diff --git a/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.h b/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.h index ece0006049e..dd89e952a34 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.h +++ b/searchcore/src/vespa/searchcore/proton/matching/same_element_builder.h @@ -7,6 +7,8 @@ #include <vespa/searchlib/queryeval/blueprint.h> #include <vespa/searchlib/queryeval/same_element_blueprint.h> +namespace search::queryeval { class FieldSpec; } + namespace proton::matching { class SameElementBuilder @@ -17,7 +19,7 @@ private: std::unique_ptr<search::queryeval::SameElementBlueprint> _result; public: SameElementBuilder(const search::queryeval::IRequestContext &requestContext, ISearchContext &context, - const vespalib::string &struct_field_name, bool expensive); + const search::queryeval::FieldSpec &field, bool expensive); void add_child(search::query::Node &node); search::queryeval::Blueprint::UP build(); }; diff --git a/searchcore/src/vespa/searchcore/proton/matching/termdataextractor.cpp b/searchcore/src/vespa/searchcore/proton/matching/termdataextractor.cpp index efddd24c964..4252f5385cb 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/termdataextractor.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/termdataextractor.cpp @@ -39,7 +39,6 @@ public: _term_data.push_back(&n); } - virtual void visit(ProtonNodeTypes::SameElement &) override {} }; } // namespace diff --git a/searchlib/src/tests/features/prod_features.cpp b/searchlib/src/tests/features/prod_features.cpp index 03563196379..81c46558381 100644 --- a/searchlib/src/tests/features/prod_features.cpp +++ b/searchlib/src/tests/features/prod_features.cpp @@ -1717,6 +1717,17 @@ Test::testMatches() EXPECT_TRUE(ft.execute(RankResult().addScore("matches(foo,2)", 0))); EXPECT_TRUE(ft.execute(RankResult().addScore("matches(foo,3)", 0))); } + { // Test executor for virtual fields + FtFeatureTest ft(_factory, StringList().add("matches(foo)")); + ft.getIndexEnv().getBuilder().addField(FieldType::VIRTUAL, CollectionType::ARRAY, "foo"); + ASSERT_TRUE(ft.getQueryEnv().getBuilder().add_virtual_node("foo") != nullptr); // query term 0 hits in foo + ASSERT_TRUE(ft.setup()); + + auto mdb = ft.createMatchDataBuilder(); + mdb->setWeight("foo", 0, 100); + mdb->apply(1); + EXPECT_TRUE(ft.execute(RankResult().addScore("matches(foo)", 1))); + } } bool diff --git a/searchlib/src/tests/query/customtypevisitor_test.cpp b/searchlib/src/tests/query/customtypevisitor_test.cpp index 86288fda498..3f68e423b08 100644 --- a/searchlib/src/tests/query/customtypevisitor_test.cpp +++ b/searchlib/src/tests/query/customtypevisitor_test.cpp @@ -27,7 +27,7 @@ struct MyNear : Near { MyNear() : Near(1) {} }; struct MyONear : ONear { MyONear() : ONear(1) {} }; struct MyOr : Or {}; struct MyPhrase : Phrase { MyPhrase() : Phrase("view", 0, Weight(42)) {} }; -struct MySameElement : SameElement { MySameElement() : SameElement("view") {} }; +struct MySameElement : SameElement { MySameElement() : SameElement("view", 0, Weight(42)) {} }; struct MyRank : Rank {}; struct MyNumberTerm : InitTerm<NumberTerm> {}; struct MyLocationTerm : InitTerm<LocationTerm> {}; diff --git a/searchlib/src/tests/query/query_visitor_test.cpp b/searchlib/src/tests/query/query_visitor_test.cpp index 8e8be997be0..48efb32afb3 100644 --- a/searchlib/src/tests/query/query_visitor_test.cpp +++ b/searchlib/src/tests/query/query_visitor_test.cpp @@ -68,7 +68,7 @@ TEST("requireThatAllNodesCanBeVisited") { checkVisit<ONear>(new SimpleONear(0)); checkVisit<Or>(new SimpleOr); checkVisit<Phrase>(new SimplePhrase("field", 0, Weight(42))); - checkVisit<SameElement>(new SimpleSameElement("field")); + checkVisit<SameElement>(new SimpleSameElement("field", 0, Weight(42))); checkVisit<WeightedSetTerm>(new SimpleWeightedSetTerm(0, "field", 0, Weight(42))); checkVisit<DotProduct>(new SimpleDotProduct(0, "field", 0, Weight(42))); checkVisit<WandTerm>(new SimpleWandTerm(0, "field", 0, Weight(42), 57, 67, 77.7)); diff --git a/searchlib/src/tests/query/querybuilder_test.cpp b/searchlib/src/tests/query/querybuilder_test.cpp index 32285918826..3922c581004 100644 --- a/searchlib/src/tests/query/querybuilder_test.cpp +++ b/searchlib/src/tests/query/querybuilder_test.cpp @@ -105,7 +105,7 @@ Node::UP createQueryTree() { n.addTerm(str[2], weight[2]); } builder.addRegExpTerm(str[5], view[5], id[5], weight[5]); - builder.addSameElement(3, view[4]); + builder.addSameElement(3, view[4], id[4], weight[4]); { builder.addStringTerm(str[4], view[4], id[4], weight[5]); builder.addStringTerm(str[5], view[5], id[5], weight[6]); @@ -381,7 +381,7 @@ struct MyONear : ONear { MyONear(size_t dist) : ONear(dist) {} }; struct MyWeakAnd : WeakAnd { MyWeakAnd(uint32_t minHits, const vespalib::string & v) : WeakAnd(minHits, v) {} }; struct MyOr : Or {}; struct MyPhrase : Phrase { MyPhrase(const string &f, int32_t i, Weight w) : Phrase(f, i, w) {}}; -struct MySameElement : SameElement { MySameElement(const string &f) : SameElement(f) {}}; +struct MySameElement : SameElement { MySameElement(const string &f, int32_t i, Weight w) : SameElement(f, i, w) {}}; struct MyWeightedSetTerm : WeightedSetTerm { MyWeightedSetTerm(uint32_t n, const string &f, int32_t i, Weight w) : WeightedSetTerm(n, f, i, w) {} diff --git a/searchlib/src/tests/query/streaming_query_test.cpp b/searchlib/src/tests/query/streaming_query_test.cpp index 4b5433dd370..f354f635def 100644 --- a/searchlib/src/tests/query/streaming_query_test.cpp +++ b/searchlib/src/tests/query/streaming_query_test.cpp @@ -734,7 +734,7 @@ namespace { } TEST("testSameElementEvaluate") { QueryBuilder<SimpleQueryNodeTypes> builder; - builder.addSameElement(3, "field"); + builder.addSameElement(3, "field", 0, Weight(0)); { builder.addStringTerm("a", "f1", 0, Weight(0)); builder.addStringTerm("b", "f2", 1, Weight(0)); diff --git a/searchlib/src/tests/query/templatetermvisitor_test.cpp b/searchlib/src/tests/query/templatetermvisitor_test.cpp index 7643139d5ad..15ce314b01f 100644 --- a/searchlib/src/tests/query/templatetermvisitor_test.cpp +++ b/searchlib/src/tests/query/templatetermvisitor_test.cpp @@ -71,12 +71,12 @@ void Test::requireThatAllTermsCanBeVisited() { EXPECT_TRUE(checkVisit<SimplePredicateQuery>()); EXPECT_TRUE(checkVisit<SimpleRegExpTerm>()); EXPECT_TRUE(checkVisit(new SimplePhrase("field", 0, Weight(0)))); + EXPECT_TRUE(checkVisit(new SimpleSameElement("foo", 0, Weight(0)))); EXPECT_TRUE(!checkVisit(new SimpleAnd)); EXPECT_TRUE(!checkVisit(new SimpleAndNot)); EXPECT_TRUE(!checkVisit(new SimpleEquiv(17, Weight(100)))); EXPECT_TRUE(!checkVisit(new SimpleNear(2))); EXPECT_TRUE(!checkVisit(new SimpleONear(2))); - EXPECT_TRUE(!checkVisit(new SimpleSameElement("foo"))); EXPECT_TRUE(!checkVisit(new SimpleOr)); EXPECT_TRUE(!checkVisit(new SimpleRank)); } @@ -84,4 +84,3 @@ void Test::requireThatAllTermsCanBeVisited() { } // namespace TEST_APPHOOK(Test); -#include <vespa/vespalib/testkit/testapp.h> diff --git a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp index 83dc574c16c..6c344d787ab 100644 --- a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp +++ b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp @@ -306,8 +306,9 @@ struct ParallelWeakAndAdapter { // enable Make-ing same element struct SameElementAdapter { + FieldSpec field; SameElementBlueprint blueprint; - SameElementAdapter() : blueprint("foo", false) {} + SameElementAdapter() : field("foo", 5, 11), blueprint(field, false) {} void addChild(std::unique_ptr<Blueprint> child) { auto child_field = blueprint.getNextChildField("foo", 3); auto term = std::make_unique<LeafProxy>(child_field, std::move(child)); diff --git a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp index 7c2b5d0b135..fba8d5d7899 100644 --- a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp +++ b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp @@ -22,8 +22,19 @@ void verify_elements(SameElementSearch &se, uint32_t docid, const std::initializ EXPECT_EQUAL(actual, expect); } +FieldSpec make_field_spec() { + // This field spec is aligned with the match data created below. + uint32_t field_id = 0; + TermFieldHandle handle = 0; + return {"foo", field_id, handle}; +} + +MatchData::UP make_match_data() { + return MatchData::makeTestInstance(1, 1); +} + std::unique_ptr<SameElementBlueprint> make_blueprint(const std::vector<FakeResult> &children, bool fake_attr = false) { - auto result = std::make_unique<SameElementBlueprint>("foo", false); + auto result = std::make_unique<SameElementBlueprint>(make_field_spec(), false); for (size_t i = 0; i < children.size(); ++i) { uint32_t field_id = i; vespalib::string field_name = vespalib::make_string("f%u", field_id); @@ -43,7 +54,7 @@ Blueprint::UP finalize(Blueprint::UP bp, bool strict) { } SimpleResult find_matches(const std::vector<FakeResult> &children) { - auto md = MatchData::makeTestInstance(0, 0); + auto md = make_match_data(); auto bp = finalize(make_blueprint(children), false); auto search = bp->createSearch(*md, false); return SimpleResult().search(*search, 1000); @@ -73,7 +84,7 @@ TEST("require that matching elements can be identified") { auto a = make_result({{5, {1,3,7,12}}, {10, {1,2,3}}}); auto b = make_result({{5, {3,5,7,10}}, {10, {4,5,6}}}); auto bp = finalize(make_blueprint({a,b}), false); - auto md = MatchData::makeTestInstance(0, 0); + auto md = make_match_data(); auto search = bp->createSearch(*md, false); search->initRange(1, 1000); SameElementSearch *se = dynamic_cast<SameElementSearch*>(search.get()); @@ -91,18 +102,24 @@ TEST("require that children must match within same element") { EXPECT_EQUAL(result, expect); } -TEST("require that strict iterator seeks to next hit") { - auto md = MatchData::makeTestInstance(0, 0); +TEST("require that strict iterator seeks to next hit and can unpack matching docid") { + auto md = make_match_data(); auto a = make_result({{5, {1,2}}, {7, {1,2}}, {8, {1,2}}, {9, {1,2}}}); auto b = make_result({{5, {3}}, {6, {1,2}}, {7, {2,4}}, {9, {1}}}); auto bp = finalize(make_blueprint({a,b}), true); auto search = bp->createSearch(*md, true); + auto* tfmd = md->resolveTermField(0); search->initRange(1, 1000); EXPECT_LESS(search->getDocId(), 1u); EXPECT_FALSE(search->seek(1)); EXPECT_EQUAL(search->getDocId(), 7u); + search->unpack(7); + EXPECT_EQUAL(tfmd->getDocId(), 7u); EXPECT_TRUE(search->seek(9)); EXPECT_EQUAL(search->getDocId(), 9u); + EXPECT_EQUAL(tfmd->getDocId(), 7u); + search->unpack(9); + EXPECT_EQUAL(tfmd->getDocId(), 9u); EXPECT_FALSE(search->seek(10)); EXPECT_TRUE(search->isAtEnd()); } @@ -129,7 +146,7 @@ TEST("require that attribute iterators are wrapped for element unpacking") { auto a = make_result({{5, {1,3,7}}}); auto b = make_result({{5, {3,5,10}}}); auto bp = finalize(make_blueprint({a,b}, true), true); - auto md = MatchData::makeTestInstance(0, 0); + auto md = make_match_data(); auto search = bp->createSearch(*md, false); SameElementSearch *se = dynamic_cast<SameElementSearch*>(search.get()); ASSERT_TRUE(se != nullptr); diff --git a/searchlib/src/vespa/searchlib/fef/parameterdescriptions.h b/searchlib/src/vespa/searchlib/fef/parameterdescriptions.h index ec39e7bdf09..e47ce0df7a5 100644 --- a/searchlib/src/vespa/searchlib/fef/parameterdescriptions.h +++ b/searchlib/src/vespa/searchlib/fef/parameterdescriptions.h @@ -68,7 +68,8 @@ private: return (normalTypesMask() | asMask(DataType::BOOLEANTREE) | asMask(DataType::TENSOR) | - asMask(DataType::REFERENCE)); + asMask(DataType::REFERENCE) | + asMask(DataType::COMBINED)); } ParameterDataTypeSet(uint32_t typeMask) : _typeMask(typeMask) diff --git a/searchlib/src/vespa/searchlib/fef/test/queryenvironmentbuilder.cpp b/searchlib/src/vespa/searchlib/fef/test/queryenvironmentbuilder.cpp index ec9875ad63f..da72374e7e3 100644 --- a/searchlib/src/vespa/searchlib/fef/test/queryenvironmentbuilder.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/queryenvironmentbuilder.cpp @@ -52,10 +52,26 @@ QueryEnvironmentBuilder::addAttributeNode(const vespalib::string &attrName) if (info == nullptr || info->type() != FieldType::ATTRIBUTE) { return nullptr; } + return add_node(*info); +} + +SimpleTermData * +QueryEnvironmentBuilder::add_virtual_node(const vespalib::string &virtual_field) +{ + const auto *info = _queryEnv.getIndexEnv()->getFieldByName(virtual_field); + if (info == nullptr || info->type() != FieldType::VIRTUAL) { + return nullptr; + } + return add_node(*info); +} + +SimpleTermData * +QueryEnvironmentBuilder::add_node(const FieldInfo &info) +{ _queryEnv.getTerms().push_back(SimpleTermData()); SimpleTermData &td = _queryEnv.getTerms().back(); td.setWeight(search::query::Weight(100)); - SimpleTermFieldData &tfd = td.addField(info->id()); + SimpleTermFieldData &tfd = td.addField(info.id()); tfd.setHandle(_layout.allocTermField(tfd.getFieldId())); return &td; } diff --git a/searchlib/src/vespa/searchlib/fef/test/queryenvironmentbuilder.h b/searchlib/src/vespa/searchlib/fef/test/queryenvironmentbuilder.h index 19cf74673a2..d9378280f45 100644 --- a/searchlib/src/vespa/searchlib/fef/test/queryenvironmentbuilder.h +++ b/searchlib/src/vespa/searchlib/fef/test/queryenvironmentbuilder.h @@ -45,6 +45,11 @@ public: */ SimpleTermData *addAttributeNode(const vespalib::string & attrName); + /** + * Add a term node searching in the given virtual field. + */ + SimpleTermData *add_virtual_node(const vespalib::string &virtual_field); + /** Returns a reference to the query environment of this. */ QueryEnvironment &getQueryEnv() { return _queryEnv; } @@ -62,6 +67,7 @@ public: private: QueryEnvironmentBuilder(const QueryEnvironmentBuilder &); // hide QueryEnvironmentBuilder & operator=(const QueryEnvironmentBuilder &); // hide + SimpleTermData *add_node(const FieldInfo &info); private: QueryEnvironment &_queryEnv; diff --git a/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h b/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h index 98825fc5a32..30160c3d532 100644 --- a/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h +++ b/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h @@ -107,18 +107,17 @@ private: bool _expensive; }; -class SameElement : public QueryNodeMixin<SameElement, Intermediate> { +class SameElement : public QueryNodeMixin<SameElement, Intermediate>, public Term { public: - SameElement(const vespalib::string &view) : _view(view), _expensive(false) {} + SameElement(const vespalib::string &view, int32_t id, Weight weight) + : Term(view, id, weight), _expensive(false) {} virtual ~SameElement() = 0; - const vespalib::string & getView() const { return _view; } SameElement &set_expensive(bool value) { _expensive = value; return *this; } bool is_expensive() const { return _expensive; } private: - vespalib::string _view; bool _expensive; }; diff --git a/searchlib/src/vespa/searchlib/query/tree/querybuilder.h b/searchlib/src/vespa/searchlib/query/tree/querybuilder.h index 2273dadbfcf..979f48fec89 100644 --- a/searchlib/src/vespa/searchlib/query/tree/querybuilder.h +++ b/searchlib/src/vespa/searchlib/query/tree/querybuilder.h @@ -129,8 +129,8 @@ typename NodeTypes::Phrase *createPhrase(vespalib::stringref view, int32_t id, W return new typename NodeTypes::Phrase(view, id, weight); } template <class NodeTypes> -typename NodeTypes::SameElement *createSameElement(vespalib::stringref view) { - return new typename NodeTypes::SameElement(view); +typename NodeTypes::SameElement *createSameElement(vespalib::stringref view, int32_t id, Weight weight) { + return new typename NodeTypes::SameElement(view, id, weight); } template <class NodeTypes> typename NodeTypes::WeightedSetTerm *createWeightedSetTerm(uint32_t num_terms, vespalib::stringref view, int32_t id, Weight weight) { @@ -271,8 +271,8 @@ public: setWeightOverride(weight); return node; } - typename NodeTypes::SameElement &addSameElement(int child_count, stringref view) { - return addIntermediate(createSameElement<NodeTypes>(view), child_count); + typename NodeTypes::SameElement &addSameElement(int child_count, stringref view, int32_t id, Weight weight) { + return addIntermediate(createSameElement<NodeTypes>(view, id, weight), child_count); } typename NodeTypes::WeightedSetTerm &addWeightedSetTerm( int child_count, stringref view, int32_t id, Weight weight) { adjustWeight(weight); diff --git a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h index 52cfbc5effb..612c3b68382 100644 --- a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h +++ b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h @@ -74,7 +74,8 @@ private: } void visit(SameElement &node) override { - _builder.addSameElement(node.getChildren().size(), node.getView()).set_expensive(node.is_expensive()); + _builder.addSameElement(node.getChildren().size(), node.getView(), + node.getId(), node.getWeight()).set_expensive(node.is_expensive()); visitNodes(node.getChildren()); } diff --git a/searchlib/src/vespa/searchlib/query/tree/simplequery.h b/searchlib/src/vespa/searchlib/query/tree/simplequery.h index 05e731c0c5c..31cd73d9f48 100644 --- a/searchlib/src/vespa/searchlib/query/tree/simplequery.h +++ b/searchlib/src/vespa/searchlib/query/tree/simplequery.h @@ -54,7 +54,8 @@ struct SimplePhrase : Phrase { }; struct SimpleSameElement : SameElement { - SimpleSameElement(vespalib::stringref view) : SameElement(view) {} + SimpleSameElement(vespalib::stringref view, int32_t id, Weight weight) + : SameElement(view, id, weight) {} ~SimpleSameElement() override; }; struct SimpleWeightedSetTerm : WeightedSetTerm { diff --git a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h index 4103c3b1766..90bd87979c7 100644 --- a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h +++ b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h @@ -110,7 +110,9 @@ private: pureTermView = view; } else if (type == ParseItem::ITEM_SAME_ELEMENT) { vespalib::stringref view = queryStack.getIndexName(); - builder.addSameElement(arity, view); + int32_t id = queryStack.getUniqueId(); + Weight weight = queryStack.GetWeight(); + builder.addSameElement(arity, view, id, weight); pureTermView = view; } else if (type == ParseItem::ITEM_WEIGHTED_SET) { vespalib::stringref view = queryStack.getIndexName(); diff --git a/searchlib/src/vespa/searchlib/query/tree/templatetermvisitor.h b/searchlib/src/vespa/searchlib/query/tree/templatetermvisitor.h index a6eae257afd..cafb9a214e7 100644 --- a/searchlib/src/vespa/searchlib/query/tree/templatetermvisitor.h +++ b/searchlib/src/vespa/searchlib/query/tree/templatetermvisitor.h @@ -53,6 +53,11 @@ class TemplateTermVisitor : public CustomTypeTermVisitor<NodeTypes> { // term's children, unless this member function is overridden // to do so. void visit(typename NodeTypes::WandTerm &n) override { myVisit(n); } + + // SameElement have children. This visitor will not visit the + // term's children, unless this member function is overridden + // to do so. + void visit(typename NodeTypes::SameElement &n) override { myVisit(n); } }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp index aeaa0f98c2f..3be28ab75de 100644 --- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp @@ -13,12 +13,12 @@ namespace search::queryeval { -SameElementBlueprint::SameElementBlueprint(const vespalib::string &field_name_in, bool expensive) - : ComplexLeafBlueprint(FieldSpecBaseList()), +SameElementBlueprint::SameElementBlueprint(const FieldSpec &field, bool expensive) + : ComplexLeafBlueprint(field), _estimate(), _layout(), _terms(), - _field_name(field_name_in) + _field_name(field.getName()) { if (expensive) { set_cost_tier(State::COST_TIER_EXPENSIVE); @@ -64,7 +64,7 @@ SameElementBlueprint::fetchPostings(const ExecuteInfo &execInfo) } std::unique_ptr<SameElementSearch> -SameElementBlueprint::create_same_element_search(bool strict) const +SameElementBlueprint::create_same_element_search(search::fef::TermFieldMatchData& tfmd, bool strict) const { fef::MatchDataLayout my_layout = _layout; fef::MatchData::UP md = my_layout.createMatchData(); @@ -79,15 +79,14 @@ SameElementBlueprint::create_same_element_search(bool strict) const children[i] = std::make_unique<attribute::SearchContextElementIterator>(std::move(child), *context); } } - return std::make_unique<SameElementSearch>(std::move(md), std::move(children), strict); + return std::make_unique<SameElementSearch>(tfmd, std::move(md), std::move(children), strict); } SearchIterator::UP SameElementBlueprint::createLeafSearch(const search::fef::TermFieldMatchDataArray &tfmda, bool strict) const { - (void) tfmda; - assert(!tfmda.valid()); - return create_same_element_search(strict); + assert(tfmda.size() == 1); + return create_same_element_search(*tfmda[0], strict); } SearchIterator::UP diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h index 2d3cb5cca99..4fa42d87d0e 100644 --- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h @@ -20,7 +20,7 @@ private: vespalib::string _field_name; public: - SameElementBlueprint(const vespalib::string &field_name_in, bool expensive); + SameElementBlueprint(const FieldSpec &field, bool expensive); SameElementBlueprint(const SameElementBlueprint &) = delete; SameElementBlueprint &operator=(const SameElementBlueprint &) = delete; ~SameElementBlueprint() override; @@ -37,7 +37,7 @@ public: void optimize_self() override; void fetchPostings(const ExecuteInfo &execInfo) override; - std::unique_ptr<SameElementSearch> create_same_element_search(bool strict) const; + std::unique_ptr<SameElementSearch> create_same_element_search(search::fef::TermFieldMatchData& tfmd, bool strict) const; SearchIteratorUP createLeafSearch(const search::fef::TermFieldMatchDataArray &tfmda, bool strict) const override; SearchIteratorUP createFilterSearch(bool strict, FilterConstraint constraint) const override; diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_search.cpp b/searchlib/src/vespa/searchlib/queryeval/same_element_search.cpp index dfa98b6b206..98c51d7f1ca 100644 --- a/searchlib/src/vespa/searchlib/queryeval/same_element_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/same_element_search.cpp @@ -39,13 +39,16 @@ SameElementSearch::check_element_match(uint32_t docid) return !_matchingElements.empty(); } -SameElementSearch::SameElementSearch(fef::MatchData::UP md, +SameElementSearch::SameElementSearch(fef::TermFieldMatchData &tfmd, + fef::MatchData::UP md, std::vector<ElementIterator::UP> children, bool strict) - : _md(std::move(md)), + : _tfmd(tfmd), + _md(std::move(md)), _children(std::move(children)), _strict(strict) { + _tfmd.reset(0); assert(!_children.empty()); } @@ -76,6 +79,12 @@ SameElementSearch::doSeek(uint32_t docid) { } void +SameElementSearch::doUnpack(uint32_t docid) +{ + _tfmd.resetOnlyDocId(docid); +} + +void SameElementSearch::visitMembers(vespalib::ObjectVisitor &visitor) const { SearchIterator::visitMembers(visitor); diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_search.h b/searchlib/src/vespa/searchlib/queryeval/same_element_search.h index b50a3ad8666..e1ec1869ac3 100644 --- a/searchlib/src/vespa/searchlib/queryeval/same_element_search.h +++ b/searchlib/src/vespa/searchlib/queryeval/same_element_search.h @@ -21,6 +21,7 @@ class SameElementSearch : public SearchIterator private: using It = fef::TermFieldMatchData::PositionsIterator; + fef::TermFieldMatchData &_tfmd; fef::MatchData::UP _md; std::vector<ElementIterator::UP> _children; std::vector<uint32_t> _matchingElements; @@ -31,12 +32,13 @@ private: bool check_element_match(uint32_t docid); public: - SameElementSearch(fef::MatchData::UP md, + SameElementSearch(fef::TermFieldMatchData &tfmd, + fef::MatchData::UP md, std::vector<ElementIterator::UP> children, bool strict); void initRange(uint32_t begin_id, uint32_t end_id) override; void doSeek(uint32_t docid) override; - void doUnpack(uint32_t) override {} + void doUnpack(uint32_t docid) override; void visitMembers(vespalib::ObjectVisitor &visitor) const override; const std::vector<ElementIterator::UP> &children() const { return _children; } diff --git a/streamingvisitors/src/tests/matching_elements_filler/matching_elements_filler_test.cpp b/streamingvisitors/src/tests/matching_elements_filler/matching_elements_filler_test.cpp index cf9494a9f27..59e84b7d08c 100644 --- a/streamingvisitors/src/tests/matching_elements_filler/matching_elements_filler_test.cpp +++ b/streamingvisitors/src/tests/matching_elements_filler/matching_elements_filler_test.cpp @@ -102,7 +102,7 @@ struct MyQueryBuilder : public search::query::QueryBuilder<search::query::Simple } } void make_same_element(vespalib::string field, BoundTerm term1, int32_t id1, BoundTerm term2, int32_t id2) { - addSameElement(2, field); + addSameElement(2, field, 0, Weight(0)); add_term(term1, id1); add_term(term2, id2); } |