Skip to content

Commit

Permalink
Merge pull request #32987 from vespa-engine/toregge/make-empty-search…
Browse files Browse the repository at this point in the history
…-context-in-not-implemented-attribute

Make empty search context in not implemented attribute.
  • Loading branch information
geirst authored Dec 3, 2024
2 parents cb0096b + 212ae30 commit 2cb782f
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,23 +108,10 @@ convertPostingBaseToSlime(const IPostingListAttributeBase &postingBase, Cursor &
convertMemoryUsageToSlime(memory_usage.bitvectors, cursor.setObject("bitvectors"));
}

std::string
type_to_string(const Config& cfg)
{
if (cfg.basicType().type() == BasicType::TENSOR) {
return cfg.tensorType().to_spec();
}
if (cfg.collectionType().type() == CollectionType::SINGLE) {
return cfg.basicType().asString();
}
return std::string(cfg.collectionType().asString()) +
"<" + std::string(cfg.basicType().asString()) + ">";
}

void
convert_config_to_slime(const Config& cfg, bool full, Cursor& object)
{
object.setString("type", type_to_string(cfg));
object.setString("type", cfg.type_to_string());
object.setBool("fast_search", cfg.fastSearch());
object.setBool("filter", cfg.getIsFilter());
object.setBool("paged", cfg.paged());
Expand Down
28 changes: 28 additions & 0 deletions searchlib/src/tests/attribute/raw_attribute/raw_attribute_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,26 @@
#include <vespa/searchlib/attribute/single_raw_attribute.h>
#include <vespa/searchlib/attribute/address_space_components.h>
#include <vespa/searchlib/attribute/attributefactory.h>
#include <vespa/searchlib/attribute/empty_search_context.h>
#include <vespa/searchlib/query/query_term_simple.h>
#include <vespa/searchcommon/attribute/config.h>
#include <vespa/vespalib/gtest/gtest.h>
#include <vespa/vespalib/util/issue.h>
#include <filesystem>
#include <memory>

using search::AddressSpaceComponents;
using search::AddressSpaceUsage;
using search::AttributeFactory;
using search::AttributeVector;
using search::QueryTermSimple;
using search::attribute::BasicType;
using search::attribute::CollectionType;
using search::attribute::Config;
using search::attribute::EmptySearchContext;
using search::attribute::SearchContextParams;
using search::attribute::SingleRawAttribute;
using vespalib::Issue;

using namespace std::literals;

Expand All @@ -37,6 +44,13 @@ void remove_saved_attr() {
std::filesystem::remove(attr_path);
}

struct MyIssueHandler : Issue::Handler {
std::vector<std::string> list;
void handle(const Issue &issue) override {
list.push_back(issue.message());
}
};

class RawAttributeTest : public ::testing::Test
{
protected:
Expand Down Expand Up @@ -157,4 +171,18 @@ TEST_F(RawAttributeTest, address_space_usage_is_reported)
EXPECT_EQ(2, _attr->getAddressSpaceUsage().get_all().at(raw_store).used());
}

TEST_F(RawAttributeTest, search_is_not_implemented)
{
MyIssueHandler handler;
{
Issue::Binding binding(handler);
auto ctx = _attr->getSearch(std::make_unique<QueryTermSimple>("hello", QueryTermSimple::Type::WORD),
SearchContextParams());
EXPECT_NE(nullptr, dynamic_cast<const EmptySearchContext*>(ctx.get()));
}
std::vector<std::string> exp;
exp.emplace_back("Search is not supported for attribute 'raw' of type 'raw' ('search::attribute::SingleRawAttribute').");
EXPECT_EQ(exp, handler.list);
}

GTEST_MAIN_RUN_ALL_TESTS()
13 changes: 13 additions & 0 deletions searchlib/src/vespa/searchcommon/attribute/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,17 @@ Config::set_hnsw_index_params(const HnswIndexParams& params) {
return *this;
}

std::string
Config::type_to_string() const
{
if (basicType().type() == BasicType::TENSOR) {
return tensorType().to_spec();
}
if (collectionType().type() == CollectionType::SINGLE) {
return basicType().asString();
}
return std::string(collectionType().asString()) +
"<" + std::string(basicType().asString()) + ">";
}

}
1 change: 1 addition & 0 deletions searchlib/src/vespa/searchcommon/attribute/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class Config {

uint64_t getMaxUnCommittedMemory() const noexcept { return _maxUnCommittedMemory; }
Config & setMaxUnCommittedMemory(uint64_t value) { _maxUnCommittedMemory = value; return *this; }
std::string type_to_string() const;

private:
BasicType _basicType;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.

#include "not_implemented_attribute.h"
#include "empty_search_context.h"
#include "search_context.h"
#include <vespa/searchcommon/attribute/config.h>
#include <vespa/vespalib/util/exceptions.h>
#include <vespa/vespalib/util/issue.h>
#include <vespa/vespalib/util/classname.h>

using search::attribute::EmptySearchContext;
using vespalib::Issue;
using vespalib::make_string_short::fmt;
using vespalib::getClassName;

Expand Down Expand Up @@ -136,7 +140,9 @@ NotImplementedAttribute::addDoc(DocId &) {

std::unique_ptr<SearchContext>
NotImplementedAttribute::getSearch(QueryTermSimpleUP, const attribute::SearchContextParams &) const {
notImplemented();
Issue::report("Search is not supported for attribute '%s' of type '%s' ('%s').",
getName().c_str(), getConfig().type_to_string().c_str(), getClassName(*this).c_str());
return std::make_unique<EmptySearchContext>(*this);
}

void NotImplementedAttribute::onAddDocs(DocId ) { }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.

#include "single_raw_attribute.h"
#include "empty_search_context.h"
#include "single_raw_attribute_loader.h"
#include "single_raw_attribute_saver.h"
#include <vespa/searchcommon/attribute/config.h>
Expand Down Expand Up @@ -162,10 +161,4 @@ SingleRawAttribute::populate_address_space_usage(AddressSpaceUsage& usage) const
usage.set(AddressSpaceComponents::raw_store, _raw_store.get_address_space_usage());
}

std::unique_ptr<attribute::SearchContext>
SingleRawAttribute::getSearch(std::unique_ptr<QueryTermSimple>, const attribute::SearchContextParams&) const
{
return std::make_unique<EmptySearchContext>(*this);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class SingleRawAttribute : public RawAttribute
}
bool isUndefined(DocId docid) const override;
uint32_t clearDoc(DocId docId) override;
std::unique_ptr<attribute::SearchContext> getSearch(std::unique_ptr<QueryTermSimple>, const attribute::SearchContextParams&) const override;
};

}

0 comments on commit 2cb782f

Please sign in to comment.