diff options
25 files changed, 200 insertions, 62 deletions
diff --git a/searchcommon/src/vespa/searchcommon/attribute/i_attribute_functor.h b/searchcommon/src/vespa/searchcommon/attribute/i_attribute_functor.h index 90484f8cc3a..c802730e6fe 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/i_attribute_functor.h +++ b/searchcommon/src/vespa/searchcommon/attribute/i_attribute_functor.h @@ -17,19 +17,19 @@ class IConstAttributeFunctor { public: virtual void operator()(const IAttributeVector &attributeVector) = 0; - virtual ~IConstAttributeFunctor() { } + virtual ~IConstAttributeFunctor() = default; }; class IAttributeFunctor { public: virtual void operator()(IAttributeVector &attributeVector) = 0; - virtual ~IAttributeFunctor() { } + virtual ~IAttributeFunctor() = default; }; class IAttributeExecutor { public: - virtual ~IAttributeExecutor() { } + virtual ~IAttributeExecutor() = default; virtual void asyncForAttribute(const vespalib::string &name, std::unique_ptr<IAttributeFunctor> func) const = 0; }; diff --git a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp index c6d49c479c4..44ff8cb925e 100644 --- a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp @@ -215,7 +215,7 @@ struct SequentialAttributeManager { mgr.addInitializedAttributes(initializer.getInitializedAttributes()); } - ~SequentialAttributeManager() {} + ~SequentialAttributeManager() = default; }; struct DummyInitializerTask : public InitializerTask @@ -262,23 +262,33 @@ ParallelAttributeManager::ParallelAttributeManager(search::SerialNum configSeria initializer::TaskRunner taskRunner(executor); taskRunner.runTask(initializer); } -ParallelAttributeManager::~ParallelAttributeManager() {} +ParallelAttributeManager::~ParallelAttributeManager() = default; TEST_F("require that attributes are added", Fixture) { - EXPECT_TRUE(f.addAttribute("a1").get() != NULL); - EXPECT_TRUE(f.addAttribute("a2").get() != NULL); + EXPECT_TRUE(f.addAttribute("a1").get() != nullptr); + EXPECT_TRUE(f.addAttribute("a2").get() != nullptr); EXPECT_EQUAL("a1", (*f._m.getAttribute("a1"))->getName()); EXPECT_EQUAL("a1", (*f._m.getAttributeReadGuard("a1", true))->getName()); EXPECT_EQUAL("a2", (*f._m.getAttribute("a2"))->getName()); EXPECT_EQUAL("a2", (*f._m.getAttributeReadGuard("a2", true))->getName()); EXPECT_TRUE(!f._m.getAttribute("not")->valid()); + + auto rv = f._m.readable_attribute_vector("a1"); + ASSERT_TRUE(rv.get() != nullptr); + EXPECT_EQUAL("a1", rv->makeReadGuard(true)->attribute()->getName()); + + rv = f._m.readable_attribute_vector("a2"); + ASSERT_TRUE(rv.get() != nullptr); + EXPECT_EQUAL("a2", rv->makeReadGuard(true)->attribute()->getName()); + + EXPECT_TRUE(f._m.readable_attribute_vector("not_valid").get() == nullptr); } TEST_F("require that predicate attributes are added", Fixture) { EXPECT_TRUE(f._m.addAttribute({"p1", AttributeUtils::getPredicateConfig()}, - createSerialNum).get() != NULL); + createSerialNum).get() != nullptr); EXPECT_EQUAL("p1", (*f._m.getAttribute("p1"))->getName()); EXPECT_EQUAL("p1", (*f._m.getAttributeReadGuard("p1", true))->getName()); } @@ -376,7 +386,7 @@ TEST_F("require that predicate attributes are flushed and loaded", BaseFixture) AttributeVector::SP a1 = am.addAttribute({"a1", AttributeUtils::getPredicateConfig()}, createSerialNum); EXPECT_EQUAL(1u, a1->getNumDocs()); - PredicateAttribute &pa = static_cast<PredicateAttribute &>(*a1); + auto &pa = static_cast<PredicateAttribute &>(*a1); PredicateIndex &index = pa.getIndex(); uint32_t doc_id; a1->addDoc(doc_id); @@ -396,7 +406,7 @@ TEST_F("require that predicate attributes are flushed and loaded", BaseFixture) AttributeVector::SP a1 = am.addAttribute({"a1", AttributeUtils::getPredicateConfig()}, createSerialNum); // loaded EXPECT_EQUAL(2u, a1->getNumDocs()); - PredicateAttribute &pa = static_cast<PredicateAttribute &>(*a1); + auto &pa = static_cast<PredicateAttribute &>(*a1); PredicateIndex &index = pa.getIndex(); uint32_t doc_id; a1->addDoc(doc_id); @@ -746,7 +756,7 @@ TEST_F("require that we can acquire exclusive read access to attribute", Fixture EXPECT_TRUE(noneAccessor.get() == nullptr); } -TEST_F("require that imported attributes are exposed via attribute context together vi regular attributes", Fixture) +TEST_F("require that imported attributes are exposed via attribute context together with regular attributes", Fixture) { f.addAttribute("attr"); f.addImportedAttribute("imported"); @@ -767,6 +777,17 @@ TEST_F("require that imported attributes are exposed via attribute context toget EXPECT_EQUAL("imported", all[1]->getName()); } +TEST_F("imported attributes are transparently returned from readable_attribute_vector", Fixture) +{ + f.addAttribute("attr"); + f.addImportedAttribute("imported"); + f.setImportedAttributes(); + auto av = f._m.readable_attribute_vector("imported"); + ASSERT_TRUE(av); + auto g = av->makeReadGuard(false); + EXPECT_EQUAL("imported", g->attribute()->getName()); +} + TEST_F("require that attribute vector of wrong type is dropped", BaseFixture) { AVConfig generic_tensor(BasicType::TENSOR); diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp index 14b72c9d8f8..edb5a07b059 100644 --- a/searchcore/src/tests/proton/attribute/attribute_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp @@ -20,6 +20,7 @@ #include <vespa/searchcore/proton/test/attribute_utils.h> #include <vespa/searchcorespi/flush/iflushtarget.h> #include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/attribute_read_guard.h> #include <vespa/searchlib/attribute/bitvector_search_cache.h> #include <vespa/searchlib/attribute/imported_attribute_vector.h> #include <vespa/searchlib/attribute/imported_attribute_vector_factory.h> @@ -588,8 +589,8 @@ struct FilterFixture TEST_F("require that filter attribute manager can filter attributes", FilterFixture) { - EXPECT_TRUE(f._filterMgr.getAttribute("a1").get() == NULL); - EXPECT_TRUE(f._filterMgr.getAttribute("a2").get() != NULL); + EXPECT_TRUE(f._filterMgr.getAttribute("a1").get() == nullptr); + EXPECT_TRUE(f._filterMgr.getAttribute("a2").get() != nullptr); std::vector<AttributeGuard> attrs; f._filterMgr.getAttributeList(attrs); EXPECT_EQUAL(1u, attrs.size()); @@ -607,6 +608,16 @@ TEST_F("require that filter attribute manager can return flushed serial number", EXPECT_EQUAL(100u, f._filterMgr.getFlushedSerialNum("a2")); } +TEST_F("readable_attribute_vector filters attributes", FilterFixture) +{ + auto av = f._filterMgr.readable_attribute_vector("a2"); + ASSERT_TRUE(av); + EXPECT_EQUAL("a2", av->makeReadGuard(false)->attribute()->getName()); + + av = f._filterMgr.readable_attribute_vector("a1"); + EXPECT_FALSE(av); +} + namespace { Tensor::UP make_tensor(const TensorSpec &spec) { diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp index e39061a8389..02bf4f4d7cc 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp @@ -11,6 +11,7 @@ #include <vespa/searchcore/proton/flushengine/shrink_lid_space_flush_target.h> #include <vespa/searchlib/attribute/attributecontext.h> #include <vespa/searchlib/attribute/attribute_read_guard.h> +#include <vespa/searchlib/attribute/imported_attribute_vector.h> #include <vespa/searchcommon/attribute/i_attribute_functor.h> #include <vespa/searchlib/attribute/interlock.h> #include <vespa/searchlib/common/isequencedtaskexecutor.h> @@ -613,4 +614,14 @@ AttributeManager::setImportedAttributes(std::unique_ptr<ImportedAttributesRepo> _importedAttributes = std::move(attributes); } +std::shared_ptr<search::attribute::ReadableAttributeVector> +AttributeManager::readable_attribute_vector(const string& name) const +{ + auto attribute = findAttribute(name); + if (attribute || !_importedAttributes) { + return attribute; + } + return _importedAttributes->get(name); +} + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h index cf60a0a41e9..881452c6b3d 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h @@ -178,6 +178,8 @@ public: void setImportedAttributes(std::unique_ptr<ImportedAttributesRepo> attributes) override; const ImportedAttributesRepo *getImportedAttributes() const override { return _importedAttributes.get(); } + + std::shared_ptr<search::attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override; }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp index fd1f95e13ba..aee68457b62 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp @@ -231,4 +231,13 @@ FilterAttributeManager::getImportedAttributes() const throw vespalib::IllegalArgumentException("Not implemented"); } +std::shared_ptr<search::attribute::ReadableAttributeVector> +FilterAttributeManager::readable_attribute_vector(const string& name) const +{ + if (acceptAttribute(name)) { + return _mgr->readable_attribute_vector(name); + } + return {}; +} + } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h index 099bb2e84ba..0bd04b8f0a5 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h @@ -54,6 +54,7 @@ public: ExclusiveAttributeReadAccessor::UP getExclusiveReadAccessor(const vespalib::string &name) const override; void setImportedAttributes(std::unique_ptr<ImportedAttributesRepo> attributes) override; const ImportedAttributesRepo *getImportedAttributes() const override; + std::shared_ptr<search::attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override; void asyncForAttribute(const vespalib::string &name, std::unique_ptr<IAttributeFunctor> func) const override; }; diff --git a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp index ab3b0588f3f..3a15bb16409 100644 --- a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp @@ -4,6 +4,7 @@ #include "selectcontext.h" #include <vespa/searchcommon/attribute/attributecontent.h> #include <vespa/searchlib/attribute/attributevector.h> +#include <vespa/searchlib/attribute/attribute_read_guard.h> #include <vespa/vespalib/util/exceptions.h> #include <vespa/log/log.h> @@ -31,14 +32,10 @@ using vespalib::make_string; AttributeFieldValueNode:: AttributeFieldValueNode(const vespalib::string& doctype, const vespalib::string& field, - const std::shared_ptr<search::AttributeVector> &attribute) + uint32_t attr_guard_index) : FieldValueNode(doctype, field), - _attribute(attribute) + _attr_guard_index(attr_guard_index) { - const AttributeVector &v(*_attribute); - // Only handle single value attribute vectors for now - assert(v.getCollectionType() == CollectionType::SINGLE); - (void) v; } @@ -46,10 +43,10 @@ std::unique_ptr<document::select::Value> AttributeFieldValueNode:: getValue(const Context &context) const { - const SelectContext &sc(static_cast<const SelectContext &>(context)); + const auto &sc(static_cast<const SelectContext &>(context)); uint32_t docId(sc._docId); assert(docId != 0u); - const AttributeVector &v(*_attribute); + const auto& v = *sc.read_guard_at_index(_attr_guard_index).attribute(); if (v.isUndefined(docId)) { return std::make_unique<NullValue>(); } @@ -86,10 +83,10 @@ getValue(const Context &context) const case BasicType::PREDICATE: case BasicType::TENSOR: case BasicType::REFERENCE: - throw new IllegalArgumentException(make_string("Attribute '%s' of type '%s' can not be used for selection", - v.getName().c_str(), v.getInternalBasicType().asString())); + throw IllegalArgumentException(make_string("Attribute '%s' of type '%s' can not be used for selection", + v.getName().c_str(), BasicType(v.getBasicType()).asString())); case BasicType::MAX_TYPE: - throw new IllegalStateException(make_string("Attribute '%s' has illegal type '%d'", v.getName().c_str(), v.getBasicType())); + throw IllegalStateException(make_string("Attribute '%s' has illegal type '%d'", v.getName().c_str(), v.getBasicType())); } return std::make_unique<NullValue>();; @@ -106,7 +103,7 @@ AttributeFieldValueNode::traceValue(const Context &context, std::ostream& out) c document::select::ValueNode::UP AttributeFieldValueNode::clone() const { - return wrapParens(new AttributeFieldValueNode(getDocType(), getFieldName(), _attribute)); + return wrapParens(new AttributeFieldValueNode(getDocType(), getFieldName(), _attr_guard_index)); } } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.h b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.h index 9ac6ce0e32b..89d1dac321b 100644 --- a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.h +++ b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.h @@ -3,18 +3,19 @@ #include <vespa/document/select/valuenodes.h> -namespace search { class AttributeVector; } +namespace search { class ReadableAttributeVector; } namespace proton { class AttributeFieldValueNode : public document::select::FieldValueNode { using Context = document::select::Context; - std::shared_ptr<search::AttributeVector> _attribute; + uint32_t _attr_guard_index; public: + // Precondition: attribute must be of a single-value type. AttributeFieldValueNode(const vespalib::string& doctype, const vespalib::string& field, - const std::shared_ptr<search::AttributeVector> &attribute); + uint32_t attr_guard_index); std::unique_ptr<document::select::Value> getValue(const Context &context) const override; std::unique_ptr<document::select::Value> traceValue(const Context &context, std::ostream& out) const override; diff --git a/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp b/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp index d007b030e6b..21f757fba79 100644 --- a/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp @@ -7,6 +7,7 @@ #include "selectpruner.h" #include <vespa/document/select/parser.h> #include <vespa/searchlib/attribute/attributevector.h> +#include <vespa/searchlib/attribute/attribute_read_guard.h> #include <vespa/searchlib/attribute/iattributemanager.h> namespace proton { @@ -24,7 +25,7 @@ namespace { class AttrVisitor : public document::select::CloningVisitor { public: - typedef std::map<vespalib::string, uint32_t> AttrMap; + using AttrMap = std::map<vespalib::string, uint32_t>; AttrMap _amap; const search::IAttributeManager &_amgr; @@ -62,7 +63,7 @@ AttrVisitor::AttrVisitor(const search::IAttributeManager &amgr, CachedSelect::At AttrVisitor::~AttrVisitor() = default; -bool isSingleValueThatWEHandle(BasicType type) { +bool isSingleValueThatWeHandle(BasicType type) { return (type != BasicType::PREDICATE) && (type != BasicType::TENSOR) && (type != BasicType::REFERENCE); } @@ -75,17 +76,18 @@ AttrVisitor::visitFieldValueNode(const FieldValueNode &expr) bool complex = false; vespalib::string name = SelectUtils::extractFieldName(expr, complex); - AttributeGuard::UP ag(_amgr.getAttribute(name)); - if (ag->valid()) { + auto av = _amgr.readable_attribute_vector(name); + if (av) { if (complex) { ++_complexAttrs; // Don't try to optimize complex attribute references yet. _valueNode = expr.clone(); return; } - std::shared_ptr<search::AttributeVector> av(ag->getSP()); - if (av->getCollectionType() == CollectionType::SINGLE) { - if (isSingleValueThatWEHandle(av->getBasicType())) { + auto guard = av->makeReadGuard(false); + const auto* attr = guard->attribute(); + if (attr->getCollectionType() == CollectionType::SINGLE) { + if (isSingleValueThatWeHandle(attr->getBasicType())) { ++_svAttrs; auto it(_amap.find(name)); uint32_t idx(invalidIdx()); @@ -99,7 +101,7 @@ AttrVisitor::visitFieldValueNode(const FieldValueNode &expr) idx = it->second; } assert(idx != invalidIdx()); - _valueNode = std::make_unique<AttributeFieldValueNode>(expr.getDocType(), name, av); + _valueNode = std::make_unique<AttributeFieldValueNode>(expr.getDocType(), name, idx); } else { ++_complexAttrs; // Don't try to optimize predicate/tensor/reference attributes yet. diff --git a/searchcore/src/vespa/searchcore/proton/common/cachedselect.h b/searchcore/src/vespa/searchcore/proton/common/cachedselect.h index e9eb452889e..563eed75c32 100644 --- a/searchcore/src/vespa/searchcore/proton/common/cachedselect.h +++ b/searchcore/src/vespa/searchcore/proton/common/cachedselect.h @@ -15,6 +15,8 @@ namespace search { class IAttributeManager; } +namespace search::attribute { class ReadableAttributeVector; } + namespace proton { class SelectContext; @@ -44,7 +46,7 @@ public: const document::select::Node &selectNode() const; }; - using AttributeVectors = std::vector<std::shared_ptr<search::AttributeVector>>; + using AttributeVectors = std::vector<std::shared_ptr<search::attribute::ReadableAttributeVector>>; private: // Single value attributes referenced from selection expression diff --git a/searchcore/src/vespa/searchcore/proton/common/selectcontext.cpp b/searchcore/src/vespa/searchcore/proton/common/selectcontext.cpp index 3275fbe06d4..ae6a1f58bdc 100644 --- a/searchcore/src/vespa/searchcore/proton/common/selectcontext.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/selectcontext.cpp @@ -3,20 +3,22 @@ #include "selectcontext.h" #include "cachedselect.h" #include <vespa/document/select/value.h> -#include <vespa/searchlib/attribute/attributeguard.h> +#include <vespa/searchlib/attribute/attribute_read_guard.h> +#include <vespa/searchlib/attribute/readable_attribute_vector.h> +#include <cassert> namespace proton { using document::select::Value; using document::select::Context; -using search::AttributeGuard; using search::AttributeVector; +using search::attribute::AttributeReadGuard; namespace select { - struct Guards : public std::vector<AttributeGuard> { - using Parent = std::vector<AttributeGuard>; - using Parent::Parent; - }; +struct Guards : public std::vector<std::unique_ptr<AttributeReadGuard>> { + using Parent = std::vector<std::unique_ptr<AttributeReadGuard>>; + using Parent::Parent; +}; } SelectContext::SelectContext(const CachedSelect &cachedSelect) @@ -26,23 +28,30 @@ SelectContext::SelectContext(const CachedSelect &cachedSelect) _cachedSelect(cachedSelect) { } -SelectContext::~SelectContext() { } +SelectContext::~SelectContext() = default; void SelectContext::getAttributeGuards() { - _guards->resize(_cachedSelect.attributes().size()); - auto j(_cachedSelect.attributes().begin()); - for (std::vector<AttributeGuard>::iterator i(_guards->begin()), ie(_guards->end()); i != ie; ++i, ++j) { - *i = AttributeGuard(*j); + _guards->clear(); + _guards->reserve(_cachedSelect.attributes().size()); + for (const auto& attr : _cachedSelect.attributes()) { + _guards->emplace_back(attr->makeReadGuard(false)); } } - void SelectContext::dropAttributeGuards() { _guards->clear(); } +const search::attribute::AttributeReadGuard& +SelectContext::read_guard_at_index(uint32_t index) const noexcept +{ + assert(index < _guards->size()); + assert((*_guards)[index].get() != nullptr); + return *((*_guards)[index]); +} + } diff --git a/searchcore/src/vespa/searchcore/proton/common/selectcontext.h b/searchcore/src/vespa/searchcore/proton/common/selectcontext.h index d37c6eb6f14..84e51bc611b 100644 --- a/searchcore/src/vespa/searchcore/proton/common/selectcontext.h +++ b/searchcore/src/vespa/searchcore/proton/common/selectcontext.h @@ -3,6 +3,8 @@ #include <vespa/document/select/context.h> +namespace search::attribute { class AttributeReadGuard; } + namespace proton { class CachedSelect; @@ -19,6 +21,8 @@ public: void dropAttributeGuards(); uint32_t _docId; + + const search::attribute::AttributeReadGuard& read_guard_at_index(uint32_t index) const noexcept; private: std::unique_ptr<select::Guards> _guards; const CachedSelect &_cachedSelect; diff --git a/searchcore/src/vespa/searchcore/proton/common/selectpruner.cpp b/searchcore/src/vespa/searchcore/proton/common/selectpruner.cpp index 6cad54f134b..4ed404333b5 100644 --- a/searchcore/src/vespa/searchcore/proton/common/selectpruner.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/selectpruner.cpp @@ -12,6 +12,7 @@ #include <vespa/document/select/invalidconstant.h> #include <vespa/document/select/valuenodes.h> #include <vespa/searchlib/attribute/attributevector.h> +#include <vespa/searchlib/attribute/attribute_read_guard.h> #include <vespa/searchlib/attribute/iattributemanager.h> using document::select::And; @@ -395,7 +396,6 @@ SelectPruner::visitIdValueNode(const IdValueNode &expr) CloningVisitor::visitIdValueNode(expr); } - void SelectPruner::visitFieldValueNode(const FieldValueNode &expr) { @@ -440,11 +440,11 @@ SelectPruner::visitFieldValueNode(const FieldValueNode &expr) bool svAttr = false; bool attrField = false; if (_amgr != nullptr) { - AttributeGuard::UP ag(_amgr->getAttribute(name)); - if (ag->valid()) { + auto attr = _amgr->readable_attribute_vector(name); + if (attr) { attrField = true; - auto av(ag->getSP()); - if (av->getCollectionType() == CollectionType::SINGLE && !complex) { + auto ag = attr->makeReadGuard(false); + if ((ag->attribute()->getCollectionType() == CollectionType::SINGLE) && !complex) { svAttr = true; } } diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h index 87fa2053508..57c86855217 100644 --- a/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h +++ b/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h @@ -78,6 +78,9 @@ public: void asyncForAttribute(const vespalib::string & name, std::unique_ptr<IAttributeFunctor> func) const override { _mock.asyncForAttribute(name, std::move(func)); } + std::shared_ptr<search::attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override { + return _mock.readable_attribute_vector(name); + } }; } diff --git a/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp b/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp index 62cde4d2c9c..7d09b2aa0b8 100644 --- a/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp +++ b/searchlib/src/tests/attribute/attributemanager/attributemanager_test.cpp @@ -34,6 +34,7 @@ private: void testGuards(); void testConfigConvert(); void testContext(); + void can_get_readable_attribute_vector_by_name(); bool assertDataType(BT::Type exp, @@ -377,6 +378,21 @@ AttributeManagerTest::testContext() } } +void +AttributeManagerTest::can_get_readable_attribute_vector_by_name() +{ + auto attr = AttributeFactory::createAttribute("cool_attr", Config(BT::INT32, CT::SINGLE)); + // Ensure there's something to actually load, or fetching the attribute will throw. + attr->addDocs(64); + attr->commit(); + AttributeManager manager; + manager.add(attr); + auto av = manager.readable_attribute_vector("cool_attr"); + EXPECT_EQUAL(av.get(), static_cast<ReadableAttributeVector*>(attr.get())); + av = manager.readable_attribute_vector("uncool_attr"); + EXPECT_TRUE(av.get() == nullptr); +} + int AttributeManagerTest::Main() { TEST_INIT("attributemanager_test"); @@ -385,6 +401,7 @@ int AttributeManagerTest::Main() testGuards(); testConfigConvert(); testContext(); + can_get_readable_attribute_vector_by_name(); TEST_DONE(); } 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 4818287b429..2eafeab20bd 100644 --- a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp @@ -123,6 +123,15 @@ public: return IAttributeContext::UP(); } + std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override { + if (name == field) { + return _attribute_vector; + } else if (name == other) { + return _other; + } + return {}; + } + void asyncForAttribute(const vespalib::string &name, std::unique_ptr<IAttributeFunctor> func) const override; }; diff --git a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp index e9addae07b9..24be21f65ec 100644 --- a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp @@ -100,6 +100,10 @@ public: void asyncForAttribute(const vespalib::string &, std::unique_ptr<IAttributeFunctor>) const override { assert(!"Not implemented"); } + + std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string&) const override { + return _attribute_vector; + } }; constexpr uint32_t DOCID_LIMIT = 3; diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp index da280aa7bda..5fa32e8ad75 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.cpp @@ -131,7 +131,7 @@ bool AttributeManager::hasReaders() const const AttributeManager::VectorHolder * AttributeManager::findAndLoadAttribute(const string & name) const { - const VectorHolder * loadedVector(NULL); + const VectorHolder * loadedVector(nullptr); AttributeMap::const_iterator found = _attributes.find(name); if (found != _attributes.end()) { AttributeVector & vec = *found->second; @@ -263,5 +263,14 @@ AttributeManager::asyncForAttribute(const vespalib::string &, std::unique_ptr<at throw std::runtime_error("search::AttributeManager::asyncForAttribute should never be called."); } +std::shared_ptr<attribute::ReadableAttributeVector> +AttributeManager::readable_attribute_vector(const string& name) const +{ + const auto* attr = findAndLoadAttribute(name); + if (attr) { + return *attr; + } + return {}; +} } diff --git a/searchlib/src/vespa/searchlib/attribute/attributemanager.h b/searchlib/src/vespa/searchlib/attribute/attributemanager.h index 1c60ab00585..9f30ff87a70 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributemanager.h +++ b/searchlib/src/vespa/searchlib/attribute/attributemanager.h @@ -49,6 +49,8 @@ public: void getAttributeList(AttributeList & list) const override; attribute::IAttributeContext::UP createContext() const override; + std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override; + const Snapshot & getSnapshot() const { return _snapShot; } const string & getBaseDir() const { return _baseDir; } void setBaseDir(const string & base); diff --git a/searchlib/src/vespa/searchlib/attribute/iattributemanager.h b/searchlib/src/vespa/searchlib/attribute/iattributemanager.h index ef9403e1c4b..51e432f2035 100644 --- a/searchlib/src/vespa/searchlib/attribute/iattributemanager.h +++ b/searchlib/src/vespa/searchlib/attribute/iattributemanager.h @@ -8,7 +8,10 @@ namespace search { -namespace attribute { class AttributeReadGuard; } +namespace attribute { +class AttributeReadGuard; +class ReadableAttributeVector; +} /** * This is an interface used to access all registered attribute vectors. @@ -17,12 +20,17 @@ class IAttributeManager : public attribute::IAttributeExecutor { public: IAttributeManager(const IAttributeManager &) = delete; IAttributeManager & operator = (const IAttributeManager &) = delete; - typedef std::shared_ptr<IAttributeManager> SP; - typedef vespalib::string string; + using SP = std::shared_ptr<IAttributeManager>; + using string = vespalib::string; /** * Returns a view of the attribute vector with the given name. * + * NOTE: this method is deprecated! Prefer using readable_attribute_vector(name) instead, + * as that enforces appropriate guards to be taken before accessing the underlying vector. + * + * TODO remove this when all usages are gone. + * * @param name name of the attribute vector. * @return view of the attribute vector or empty view if the attribute vector does not exists. **/ @@ -52,9 +60,15 @@ public: virtual attribute::IAttributeContext::UP createContext() const = 0; /** - * Virtual destructor to allow safe subclassing. - **/ - virtual ~IAttributeManager() {} + * Looks up and returns a readable attribute vector shared_ptr with the provided name. + * This transparently supports imported attribute vectors. + * + * @param name name of the attribute vector. + * @return The attribute vector, or an empty shared_ptr if no vector was found with the given name. + */ + virtual std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const = 0; + + ~IAttributeManager() override = default; protected: IAttributeManager() = default; }; diff --git a/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h b/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h index 03058362cb1..0e3954bdcb5 100644 --- a/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h +++ b/searchlib/src/vespa/searchlib/attribute/readable_attribute_vector.h @@ -13,7 +13,7 @@ class AttributeReadGuard; */ class ReadableAttributeVector { public: - virtual ~ReadableAttributeVector() {} + virtual ~ReadableAttributeVector() = default; virtual std::unique_ptr<AttributeReadGuard> makeReadGuard(bool stableEnumGuard) const = 0; }; diff --git a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp index 5ef9b6cb2d4..4c80c6f869c 100644 --- a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp +++ b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.cpp @@ -61,4 +61,9 @@ MockAttributeManager::addAttribute(const AttributeVector::SP &attr) { addAttribute(attr->getName(), attr); } +std::shared_ptr<attribute::ReadableAttributeVector> +MockAttributeManager::readable_attribute_vector(const string& name) const { + return findAttribute(name); +} + } diff --git a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h index dbf84a40e84..a8ec686433a 100644 --- a/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h +++ b/searchlib/src/vespa/searchlib/test/mock_attribute_manager.h @@ -28,6 +28,7 @@ public: IAttributeContext::UP createContext() const override; void addAttribute(const vespalib::string &name, const AttributeVector::SP &attr); void addAttribute(const AttributeVector::SP &attr); + std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string& name) const override; }; } diff --git a/searchsummary/src/tests/docsummary/positionsdfw_test.cpp b/searchsummary/src/tests/docsummary/positionsdfw_test.cpp index b161958fc43..4cad98a8e01 100644 --- a/searchsummary/src/tests/docsummary/positionsdfw_test.cpp +++ b/searchsummary/src/tests/docsummary/positionsdfw_test.cpp @@ -100,6 +100,10 @@ public: IAttributeContext::UP createContext() const override { return IAttributeContext::UP(new MyAttributeContext(_attr)); } + + std::shared_ptr<attribute::ReadableAttributeVector> readable_attribute_vector(const string&) const override { + LOG_ABORT("should not be reached"); + } }; struct MyGetDocsumsStateCallback : GetDocsumsStateCallback { |