diff options
author | Geir Storli <geirst@yahooinc.com> | 2024-01-23 15:01:16 +0000 |
---|---|---|
committer | Geir Storli <geirst@yahooinc.com> | 2024-01-23 15:01:16 +0000 |
commit | 6a218aa8b9621682acccecb29fe1908236d2e6b1 (patch) | |
tree | 0374aa1bdc2be72aed055e8d04b7c03a9e065a89 | |
parent | 18a604cbd2d99e1aa84dada602dd8babafbb0649 (diff) |
Remove optimization for single term searching in wset string/int attribute.
The benefits are no longer clear, and direct usage of underlying
btree iterators will not work in combination with the sameElement operator,
as this requires access to element ids that are unpacked via the attribute::ISearchContext interface.
This became clear when extending the attribute types (e.g. array) that
give direct access to underlying btree iterators via IDocidWithWeightPostingStore.
-rw-r--r-- | searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp | 29 | ||||
-rw-r--r-- | searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp | 76 |
2 files changed, 2 insertions, 103 deletions
diff --git a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp index ecc03ac54c5..3b346601245 100644 --- a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp @@ -473,35 +473,6 @@ TEST("require that attribute dot product can produce no hits") { } } -TEST("require that direct attribute iterators work") { - for (int i = 0; i <= 0x3; ++i) { - bool fast_search = ((i & 0x1) != 0); - bool strict = ((i & 0x2) != 0); - MyAttributeManager attribute_manager = make_weighted_string_attribute_manager(fast_search); - SimpleStringTerm empty_node("notfoo", "", 0, Weight(1)); - Result empty_result = do_search(attribute_manager, empty_node, strict); - EXPECT_EQUAL(0u, empty_result.hits.size()); - SimpleStringTerm node("foo", "", 0, Weight(1)); - Result result = do_search(attribute_manager, node, strict); - if (fast_search) { - EXPECT_EQUAL(3u, result.est_hits); - EXPECT_TRUE(result.has_minmax); - EXPECT_EQUAL(100, result.min_weight); - EXPECT_EQUAL(1000, result.max_weight); - EXPECT_TRUE(result.iterator_dump.find("DocidWithWeightSearchIterator") != vespalib::string::npos); - } else { - EXPECT_EQUAL(num_docs, result.est_hits); - EXPECT_FALSE(result.has_minmax); - EXPECT_TRUE(result.iterator_dump.find("DocidWithWeightSearchIterator") == vespalib::string::npos); - } - ASSERT_EQUAL(3u, result.hits.size()); - EXPECT_FALSE(result.est_empty); - EXPECT_EQUAL(20u, result.hits[0].docid); - EXPECT_EQUAL(40u, result.hits[1].docid); - EXPECT_EQUAL(50u, result.hits[2].docid); - } -} - TEST("require that single weighted set turns filter on filter fields") { bool fast_search = true; bool strict = true; diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index 5d689f5bd81..56714bbf033 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -494,69 +494,6 @@ AttributeFieldBlueprint::getRange(vespalib::string &from, vespalib::string &to) return false; } -//----------------------------------------------------------------------------- - -class DirectAttributeBlueprint : public queryeval::SimpleLeafBlueprint -{ -private: - const IAttributeVector &_iattr; - const IDocidWithWeightPostingStore &_attr; - vespalib::datastore::EntryRef _dictionary_snapshot; - IDirectPostingStore::LookupResult _dict_entry; - -public: - DirectAttributeBlueprint(const FieldSpec &field, const IAttributeVector &iattr, - const IDocidWithWeightPostingStore &attr, - const IDirectPostingStore::LookupKey & key) - : SimpleLeafBlueprint(field), - _iattr(iattr), - _attr(attr), - _dictionary_snapshot(_attr.get_dictionary_snapshot()), - _dict_entry(_attr.lookup(key, _dictionary_snapshot)) - { - setEstimate(HitEstimate(_dict_entry.posting_size, (_dict_entry.posting_size == 0))); - } - - SearchIterator::UP createLeafSearch(const TermFieldMatchDataArray &tfmda, bool strict) const override { - assert(tfmda.size() == 1); - if (_dict_entry.posting_size == 0) { - return std::make_unique<queryeval::EmptySearch>(); - } - if (tfmda[0]->isNotNeeded()) { - auto bitvector_iterator = _attr.make_bitvector_iterator(_dict_entry.posting_idx, get_docid_limit(), *tfmda[0], strict); - if (bitvector_iterator) { - return bitvector_iterator; - } - } - if (_attr.has_btree_iterator(_dict_entry.posting_idx)) { - return std::make_unique<queryeval::DocidWithWeightSearchIterator>(*tfmda[0], _attr, _dict_entry); - } else { - return _attr.make_bitvector_iterator(_dict_entry.posting_idx, get_docid_limit(), *tfmda[0], strict); - } - } - - SearchIteratorUP createFilterSearch(bool strict, FilterConstraint constraint) const override { - (void) constraint; // We provide an iterator with exact results, so no need to take constraint into consideration. - auto wrapper = std::make_unique<FilterWrapper>(getState().numFields()); - wrapper->wrap(createLeafSearch(wrapper->tfmda(), strict)); - return wrapper; - } - - void visitMembers(vespalib::ObjectVisitor &visitor) const override { - LeafBlueprint::visitMembers(visitor); - visit_attribute(visitor, _iattr); - } - std::unique_ptr<queryeval::MatchingElementsSearch> create_matching_elements_search(const MatchingElementsFields &fields) const override { - if (fields.has_field(_iattr.getName())) { - return queryeval::MatchingElementsSearch::create(_iattr, _dictionary_snapshot, vespalib::ConstArrayRef<IDirectPostingStore::LookupResult>(&_dict_entry, 1)); - } else { - return {}; - } - } -}; - -//----------------------------------------------------------------------------- - bool check_valid_diversity_attr(const IAttributeVector *attr) { if ((attr == nullptr) || attr->hasMultiValue()) { return false; @@ -598,15 +535,6 @@ public: ~CreateBlueprintVisitor() override; template <class TermNode> - void visitSimpleTerm(TermNode &n) { - if (use_docid_with_weight_posting_store() && !_field.isFilter() && n.isRanked() && !Term::isPossibleRangeTerm(n.getTerm())) { - NodeAsKey key(n, _scratchPad); - setResult(std::make_unique<DirectAttributeBlueprint>(_field, _attr, *_dwwps, key)); - } else { - visitTerm(n); - } - } - template <class TermNode> void visitTerm(TermNode &n) { SearchContextParams scParams = createContextParams(_field.isFilter()); scParams.fuzzy_matching_algorithm(getRequestContext().get_attribute_blueprint_params().fuzzy_matching_algorithm); @@ -628,7 +556,7 @@ public: } } - void visit(NumberTerm & n) override { visitSimpleTerm(n); } + void visit(NumberTerm & n) override { visitTerm(n); } void visit(LocationTerm &n) override { visitLocation(n); } void visit(PrefixTerm & n) override { visitTerm(n); } @@ -652,7 +580,7 @@ public: } } - void visit(StringTerm & n) override { visitSimpleTerm(n); } + void visit(StringTerm & n) override { visitTerm(n); } void visit(SubstringTerm & n) override { query::SimpleRegExpTerm re(vespalib::RegexpUtil::make_from_substring(n.getTerm()), n.getView(), n.getId(), n.getWeight()); |