Skip to content

Commit

Permalink
Merge pull request #30030 from vespa-engine/geirst/remove-single-term…
Browse files Browse the repository at this point in the history 
…-in-wset-optimization

Remove optimization for single term searching in wset string/int attr…
  • Loading branch information
@geirst
geirst authored Jan 23, 2024
2 parents 914b47b + 6a218aa commit 2a43344
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 103 deletions.
29 changes: 0 additions & 29 deletions searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
76 changes: 2 additions & 74 deletions searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -597,15 +534,6 @@ class CreateBlueprintVisitor : public CreateBlueprintVisitorHelper
}
~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());
Expand All @@ -628,7 +556,7 @@ class CreateBlueprintVisitor : public CreateBlueprintVisitorHelper
}
}

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); }

Expand All @@ -652,7 +580,7 @@ class CreateBlueprintVisitor : public CreateBlueprintVisitorHelper
}
}

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());
Expand Down

0 comments on commit 2a43344

Please sign in to comment.