Reduce schema use in document retriever.

5 files changed, 36 insertions, 48 deletions

diff --git a/searchcore/src/tests/proton/server/documentretriever_test.cpp b/searchcore/src/tests/proton/server/documentretriever_test.cpp
index d3a1f4d47cf..e6c573cabe0 100644
--- a/searchcore/src/tests/proton/server/documentretriever_test.cpp
+++ b/searchcore/src/tests/proton/server/documentretriever_test.cpp
@@ -19,6 +19,7 @@
 #include <vespa/document/repo/documenttyperepo.h>
 #include <vespa/persistence/spi/bucket.h>
 #include <vespa/persistence/spi/result.h>
+#include <vespa/searchcommon/common/schema.h>
 #include <vespa/searchcore/proton/documentmetastore/documentmetastorecontext.h>
 #include <vespa/searchcore/proton/server/documentretriever.h>
 #include <vespa/searchcore/proton/test/dummy_document_store.h>
@@ -225,7 +226,7 @@ struct Fixture {
     search::AttributeManager attr_manager;
     Schema schema;
     DocTypeName _dtName;
-    DocumentRetriever retriever;
+    std::unique_ptr<DocumentRetriever> _retriever;
 
     template <typename T>
     T *addAttribute(const char *name,
@@ -267,8 +268,7 @@ struct Fixture {
           attr_manager(),
           schema(),
           _dtName(doc_type_name),
-          retriever(_dtName,
-                    repo, schema, meta_store, attr_manager, doc_store)
+          _retriever()
     {
         typedef DocumentMetaStore::Result Result;
         meta_store.constructFreeList();
@@ -314,12 +314,13 @@ struct Fixture {
                 dyn_wset_field_s, dyn_value_s, DataType::STRING, ct);
         addAttribute<FloatingPointAttribute>(
                 dyn_wset_field_n, DataType::FLOAT, ct);
+        _retriever = std::make_unique<DocumentRetriever>(_dtName, repo, schema, meta_store, attr_manager, doc_store);
     }
 };
 
 TEST_F("require that document retriever can retrieve document meta data",
        Fixture) {
-    DocumentMetaData meta_data = f.retriever.getDocumentMetaData(doc_id);
+    DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
     EXPECT_EQUAL(f.lid, meta_data.lid);
     EXPECT_EQUAL(f.timestamp, meta_data.timestamp);
 }
@@ -327,19 +328,19 @@ TEST_F("require that document retriever can retrieve document meta data",
 TEST_F("require that document retriever can retrieve bucket meta data",
        Fixture) {
     DocumentMetaData::Vector result;
-    f.retriever.getBucketMetaData(Bucket(f.bucket_id, PartitionId(0)), result);
+    f._retriever->getBucketMetaData(Bucket(f.bucket_id, PartitionId(0)), result);
     ASSERT_EQUAL(1u, result.size());
     EXPECT_EQUAL(f.lid, result[0].lid);
     EXPECT_EQUAL(f.timestamp, result[0].timestamp);
     result.clear();
-    f.retriever.getBucketMetaData(Bucket(BucketId(f.bucket_id.getId() + 1),
+    f._retriever->getBucketMetaData(Bucket(BucketId(f.bucket_id.getId() + 1),
                                          PartitionId(0)), result);
     EXPECT_EQUAL(0u, result.size());
 }
 
 TEST_F("require that document retriever can retrieve document", Fixture) {
-    DocumentMetaData meta_data = f.retriever.getDocumentMetaData(doc_id);
-    Document::UP doc = f.retriever.getDocument(meta_data.lid);
+    DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
+    Document::UP doc = f._retriever->getDocument(meta_data.lid);
     ASSERT_TRUE(doc.get());
     EXPECT_EQUAL(doc_id, doc->getId());
 }
@@ -377,8 +378,8 @@ void checkWset(FieldValue::UP wset, T v) {
 }
 
 TEST_F("require that attributes are patched into stored document", Fixture) {
-    DocumentMetaData meta_data = f.retriever.getDocumentMetaData(doc_id);
-    Document::UP doc = f.retriever.getDocument(meta_data.lid);
+    DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
+    Document::UP doc = f._retriever->getDocument(meta_data.lid);
     ASSERT_TRUE(doc.get());
 
     FieldValue::UP value = doc->getValue(static_field);
@@ -408,15 +409,15 @@ TEST_F("require that attributes are patched into stored document", Fixture) {
 
 TEST_F("require that attributes are patched into stored document unless also index field", Fixture) {
     f.schema.addIndexField(Schema::IndexField(dyn_field_s, DataType::STRING));
-    DocumentMetaData meta_data = f.retriever.getDocumentMetaData(doc_id);
-    Document::UP doc = f.retriever.getDocument(meta_data.lid);
+    DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
+    Document::UP doc = f._retriever->getDocument(meta_data.lid);
     ASSERT_TRUE(doc.get());
     checkFieldValue<StringFieldValue>(doc->getValue(dyn_field_s), static_value_s);
 }
 
 TEST_F("require that position fields are regenerated from zcurves", Fixture) {
-    DocumentMetaData meta_data = f.retriever.getDocumentMetaData(doc_id);
-    Document::UP doc = f.retriever.getDocument(meta_data.lid);
+    DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
+    Document::UP doc = f._retriever->getDocument(meta_data.lid);
     ASSERT_TRUE(doc.get());
 
     FieldValue::UP value = doc->getValue(position_field);
@@ -433,13 +434,13 @@ TEST_F("require that position fields are regenerated from zcurves", Fixture) {
 }
 
 TEST_F("require that non-existing lid returns null pointer", Fixture) {
-    Document::UP doc = f.retriever.getDocument(0);
+    Document::UP doc = f._retriever->getDocument(0);
     ASSERT_FALSE(doc.get());
 }
 
 TEST_F("require that predicate attributes can be retrieved", Fixture) {
-    DocumentMetaData meta_data = f.retriever.getDocumentMetaData(doc_id);
-    Document::UP doc = f.retriever.getDocument(meta_data.lid);
+    DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
+    Document::UP doc = f._retriever->getDocument(meta_data.lid);
     ASSERT_TRUE(doc.get());
 
     FieldValue::UP value = doc->getValue(dyn_field_p);
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp b/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
index a661cb371bc..09c68c567fa 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
@@ -6,6 +6,7 @@
 #include <vespa/searchcore/proton/attribute/document_field_retriever.h>
 #include <vespa/vespalib/geo/zcurve.h>
 #include <vespa/searchlib/attribute/attributevector.h>
+#include <vespa/searchcommon/common/schema.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".proton.server.documentretriever");
@@ -37,7 +38,8 @@ DocumentRetriever
       _schema(schema),
       _attr_manager(attr_manager),
       _doc_store(doc_store),
-      _possiblePositionFields()
+      _possiblePositionFields(),
+      _attributeFields()
 {
     const DocumentType * documentType = repo.getDocumentType(docTypeName.getName());
     document::Field::Set fields = documentType->getFieldSet();
@@ -48,10 +50,16 @@ DocumentRetriever
             LOG(debug, "Field '%s' is a position field", field->getName().c_str());
             const vespalib::string & zcurve_name = PositionDataType::getZCurveFieldName(field->getName());
             AttributeGuard::UP attr = attr_manager.getAttribute(zcurve_name);
-            if (attr) {
+            if (attr && attr->valid()) {
                 LOG(debug, "Field '%s' is a registered attribute field", zcurve_name.c_str());
                 _possiblePositionFields.emplace_back(field, zcurve_name);
             }
+        } else {
+            const vespalib::string &name = field->getName();
+            AttributeGuard::UP attr = attr_manager.getAttribute(name);
+            if (attr && attr->valid()) {
+                _attributeFields.emplace_back(name);
+            }
         }
     }
 }
@@ -74,10 +82,8 @@ void fillInPositionFields(Document &doc, DocumentIdT lid, const DocumentRetrieve
     for (const auto & it : possiblePositionFields) {
         if (doc.hasValue(*it.first)) {
             AttributeGuard::UP attr = attr_manager.getAttribute(it.second);
-            if (attr.get() && attr->valid()) {
-                int64_t zcurve = (*attr)->getInt(lid);
-                doc.setValue(*it.first, *positionFromZcurve(zcurve));
-            }
+            int64_t zcurve = (*attr)->getInt(lid);
+            doc.setValue(*it.first, *positionFromZcurve(zcurve));
         }
     }
 }
@@ -124,23 +130,13 @@ void DocumentRetriever::visitDocuments(const LidVector & lids, search::IDocument
 
 void DocumentRetriever::populate(DocumentIdT lid, Document & doc) const
 {
-    for (uint32_t i = 0; i < _schema.getNumAttributeFields(); ++i) {
-        const Schema::AttributeField &field = _schema.getAttributeField(i);
-        AttributeGuard::UP attr = _attr_manager.getAttribute(field.getName());
-        if (attr.get() && attr->valid()) {
-            DocumentFieldRetriever::populate(lid, doc, field.getName(), **attr, _schema.isIndexField(field.getName()));
-        }
+    for (const auto &field : _attributeFields) {
+        AttributeGuard::UP attr = _attr_manager.getAttribute(field);
+        DocumentFieldRetriever::populate(lid, doc, field, **attr, _schema.isIndexField(field));
     }
     fillInPositionFields(doc, lid, _possiblePositionFields, _attr_manager);
 }
 
-const Schema &
-DocumentRetriever::getSchema(void) const
-{
-    return _schema;
-}
-
-
 const IAttributeManager *
 DocumentRetriever::getAttrMgr(void) const
 {
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretriever.h b/searchcore/src/vespa/searchcore/proton/server/documentretriever.h
index 94446a0c738..7873c17401f 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentretriever.h
+++ b/searchcore/src/vespa/searchcore/proton/server/documentretriever.h
@@ -16,6 +16,7 @@ namespace proton {
 class DocumentRetriever : public DocumentRetrieverBase {
 public:
     typedef std::vector<std::pair<const document::Field *, vespalib::string>> PositionFields;
+    using AttributeFields = std::vector<std::string>;
     DocumentRetriever(const DocTypeName &docTypeName,
                       const document::DocumentTypeRepo &repo,
                       const search::index::Schema &schema,
@@ -31,8 +32,8 @@ private:
     const search::IAttributeManager &_attr_manager;
     const search::IDocumentStore    &_doc_store;
     PositionFields                   _possiblePositionFields;
+    AttributeFields                  _attributeFields;
 
-    const search::index::Schema & getSchema(void) const override;
     const search::IAttributeManager * getAttrMgr(void) const override;
 };
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp b/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp
index 7a49085e42c..6f7e4423396 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.cpp
@@ -6,12 +6,10 @@
 
 using document::DocumentId;
 using document::GlobalId;
-using search::index::Schema;
 
 namespace {
 
 const DocumentId docId("doc:test:1");
-const Schema emptySchema;
 
 }
 
@@ -62,13 +60,6 @@ DocumentRetrieverBase::getDocumentMetaData(const DocumentId &id) const {
 }
 
 
-const search::index::Schema &
-DocumentRetrieverBase::getSchema(void) const
-{
-    return emptySchema;
-}
-
-
 const search::IAttributeManager *
 DocumentRetrieverBase::getAttrMgr(void) const
 {
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.h b/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.h
index 22d31c26895..63fcdcd1f98 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.h
+++ b/searchcore/src/vespa/searchcore/proton/server/documentretrieverbase.h
@@ -6,8 +6,8 @@
 #include <vespa/searchcore/proton/common/doctypename.h>
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/vespalib/stllike/lrucache_map.h>
-#include <vespa/searchcommon/common/schema.h>
 #include <vespa/searchlib/attribute/iattributemanager.h>
+#include <vespa/vespalib/util/sync.h>
 
 namespace proton {
 
@@ -25,7 +25,6 @@ class DocumentRetrieverBase : public IDocumentRetriever
     const bool             _hasFields;
 
 protected:
-    virtual const search::index::Schema & getSchema(void) const;
     virtual const search::IAttributeManager * getAttrMgr(void) const;
 public:
     DocumentRetrieverBase(const DocTypeName &docTypeName,