Refactor MultiAttrDFW in preparation to support filtering of matched elements.

6 files changed, 118 insertions, 93 deletions

diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp
index f1b12d8a227..704876f4838 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp
@@ -1,25 +1,27 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include "docsumwriter.h"
 #include "attributedfw.h"
 #include "docsumstate.h"
-#include <vespa/searchlib/attribute/stringbase.h>
-#include <vespa/searchlib/attribute/integerbase.h>
+#include "docsumwriter.h"
+#include <vespa/eval/tensor/serialization/typed_binary_format.h>
+#include <vespa/eval/tensor/tensor.h>
+#include <vespa/searchcommon/attribute/iattributecontext.h>
 #include <vespa/searchlib/attribute/iattributemanager.h>
+#include <vespa/searchlib/attribute/integerbase.h>
+#include <vespa/searchlib/attribute/stringbase.h>
 #include <vespa/searchlib/tensor/i_tensor_attribute.h>
-#include <vespa/searchcommon/attribute/iattributecontext.h>
-#include <vespa/eval/tensor/tensor.h>
-#include <vespa/eval/tensor/serialization/typed_binary_format.h>
-#include <vespa/vespalib/objects/nbostream.h>
 #include <vespa/vespalib/data/slime/slime.h>
+#include <vespa/vespalib/objects/nbostream.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".searchlib.docsummary.attributedfw");
 
 using namespace search;
+using search::attribute::BasicType;
 using search::attribute::IAttributeContext;
 using search::attribute::IAttributeVector;
-using search::attribute::BasicType;
+using vespalib::Memory;
+using vespalib::slime::Cursor;
 using vespalib::slime::Inserter;
 
 namespace search::docsummary {
@@ -30,7 +32,7 @@ AttrDFW::AttrDFW(const vespalib::string & attrName) :
 }
 
 const attribute::IAttributeVector &
-AttrDFW::vec(const GetDocsumsState & s) const {
+AttrDFW::get_attribute(const GetDocsumsState& s) const {
     return *s.getAttribute(getIndex());
 }
 
@@ -48,13 +50,13 @@ public:
 
 bool SingleAttrDFW::isDefaultValue(uint32_t docid, const GetDocsumsState * state) const
 {
-    return vec(*state).isUndefined(docid);
+    return get_attribute(*state).isUndefined(docid);
 }
 
 void
 SingleAttrDFW::insertField(uint32_t docid, GetDocsumsState * state, ResType type, Inserter &target)
 {
-    const IAttributeVector & v = vec(*state);
+    const auto& v = get_attribute(*state);
     switch (type) {
     case RES_INT: {
         uint32_t val = v.getInt(docid);
@@ -133,46 +135,92 @@ SingleAttrDFW::insertField(uint32_t docid, GetDocsumsState * state, ResType type
 
 //-----------------------------------------------------------------------------
 
-class MultiAttrDFW : public AttrDFW
-{
-public:
-    explicit MultiAttrDFW(const vespalib::string & attrName) : AttrDFW(attrName) {}
-    void insertField(uint32_t docid, GetDocsumsState *state, ResType type, Inserter &target) override;
+template <typename DataType>
+class MultiAttrDFW : public AttrDFW {
+private:
+    bool _is_weighted_set;
 
+public:
+    explicit MultiAttrDFW(const vespalib::string& attr_name, bool is_weighted_set)
+        : AttrDFW(attr_name),
+          _is_weighted_set(is_weighted_set)
+    {}
+    void insertField(uint32_t docid, GetDocsumsState* state, ResType type, Inserter& target) override;
 };
 
 void
-MultiAttrDFW::insertField(uint32_t docid, GetDocsumsState *state, ResType, Inserter &target)
+insert_element(const std::vector<IAttributeVector::WeightedString>& elements,
+               size_t idx, bool is_weighted_set, Cursor& arr)
+{
+    const vespalib::string& sv = elements[idx].getValue();
+    Memory value(sv.c_str(), sv.size());
+    if (is_weighted_set) {
+        Cursor& elem = arr.addObject();
+        elem.setString("item", value);
+        elem.setLong("weight", elements[idx].getWeight());
+    } else {
+        arr.addString(value);
+    }
+}
+
+void
+insert_element(const std::vector<IAttributeVector::WeightedInt>& elements,
+               size_t idx, bool is_weighted_set, Cursor& arr)
 {
-    using vespalib::slime::Cursor;
-    using vespalib::Memory;
-    const IAttributeVector & v = vec(*state);
-    bool isWeightedSet = v.hasWeightedSetType();
+    if (is_weighted_set) {
+        Cursor& elem = arr.addObject();
+        elem.setLong("item", elements[idx].getValue());
+        elem.setLong("weight", elements[idx].getWeight());
+    } else {
+        arr.addLong(elements[idx].getValue());
+    }
+}
 
-    uint32_t entries = v.getValueCount(docid);
+void
+insert_element(const std::vector<IAttributeVector::WeightedFloat>& elements,
+               size_t idx, bool is_weighted_set, Cursor& arr)
+{
+    if (is_weighted_set) {
+        Cursor& elem = arr.addObject();
+        elem.setDouble("item", elements[idx].getValue());
+        elem.setLong("weight", elements[idx].getWeight());
+    } else {
+        arr.addDouble(elements[idx].getValue());
+    }
+}
+
+template <typename DataType>
+void
+MultiAttrDFW<DataType>::insertField(uint32_t docid, GetDocsumsState* state, ResType, Inserter& target)
+{
+    const auto& attr = get_attribute(*state);
+    uint32_t entries = attr.getValueCount(docid);
     if (entries == 0) {
         return;  // Don't insert empty fields
     }
 
     Cursor &arr = target.insertArray();
-    BasicType::Type t = v.getBasicType();
-    switch (t) {
+    std::vector<DataType> elements(entries);
+    entries = std::min(entries, attr.get(docid, elements.data(), entries));
+    for (uint32_t i = 0; i < entries; ++i) {
+        insert_element(elements, i, _is_weighted_set, arr);
+    }
+}
+
+//-----------------------------------------------------------------------------
+
+namespace {
+
+std::unique_ptr<IDocsumFieldWriter>
+create_multi_writer(const IAttributeVector& attr)
+{
+    auto type = attr.getBasicType();
+    bool is_weighted_set = attr.hasWeightedSetType();
+    switch (type) {
     case BasicType::NONE:
     case BasicType::STRING: {
-        std::vector<IAttributeVector::WeightedString> elements(entries);
-        entries = std::min(entries, v.get(docid, &elements[0], entries));
-        for (uint32_t i = 0; i < entries; ++i) {
-            const vespalib::string &sv = elements[i].getValue();
-            Memory value(sv.c_str(), sv.size());
-            if (isWeightedSet) {
-                Cursor &elem = arr.addObject();
-                elem.setString("item", value);
-                elem.setLong("weight", elements[i].getWeight());
-            } else {
-                arr.addString(value);
-            }
-        }
-        return; }
+        return std::make_unique<MultiAttrDFW<IAttributeVector::WeightedString>>(attr.getName(), is_weighted_set);
+    }
     case BasicType::BOOL:
     case BasicType::UINT2:
     case BasicType::UINT4:
@@ -180,54 +228,34 @@ MultiAttrDFW::insertField(uint32_t docid, GetDocsumsState *state, ResType, Inser
     case BasicType::INT16:
     case BasicType::INT32:
     case BasicType::INT64: {
-        std::vector<IAttributeVector::WeightedInt> elements(entries);
-        entries = std::min(entries, v.get(docid, &elements[0], entries));
-        for (uint32_t i = 0; i < entries; ++i) {
-            if (isWeightedSet) {
-                Cursor &elem = arr.addObject();
-                elem.setLong("item", elements[i].getValue());
-                elem.setLong("weight", elements[i].getWeight());
-            } else {
-                arr.addLong(elements[i].getValue());
-            }
-        }
-        return; }
+        return std::make_unique<MultiAttrDFW<IAttributeVector::WeightedInt>>(attr.getName(), is_weighted_set);
+    }
     case BasicType::FLOAT:
     case BasicType::DOUBLE: {
-        std::vector<IAttributeVector::WeightedFloat> elements(entries);
-        entries = std::min(entries, v.get(docid, &elements[0], entries));
-        for (uint32_t i = 0; i < entries; ++i) {
-            if (isWeightedSet) {
-                Cursor &elem = arr.addObject();
-                elem.setDouble("item", elements[i].getValue());
-                elem.setLong("weight", elements[i].getWeight());
-            } else {
-                arr.addDouble(elements[i].getValue());
-            }
-        }
-        return; }
+        return std::make_unique<MultiAttrDFW<IAttributeVector::WeightedFloat>>(attr.getName(), is_weighted_set);
+    }
     default:
         // should not happen
-        LOG(error, "bad value for type: %u\n", t);
+        LOG(error, "Bad value for attribute type: %u", type);
         LOG_ASSERT(false);
     }
 }
 
-//-----------------------------------------------------------------------------
+}
 
-IDocsumFieldWriter *
-AttributeDFWFactory::create(IAttributeManager & vecMan, const char *vecName)
+std::unique_ptr<IDocsumFieldWriter>
+AttributeDFWFactory::create(IAttributeManager& attr_mgr, const vespalib::string& attr_name)
 {
-    IAttributeContext::UP ctx = vecMan.createContext();
-    const IAttributeVector * vec = ctx->getAttribute(vecName);
-    if (vec == nullptr) {
-        LOG(warning, "No valid attribute vector found: %s", vecName);
-        return nullptr;
-    }
-    if (vec->hasMultiValue()) {
-        return new MultiAttrDFW(vec->getName());
+    auto ctx = attr_mgr.createContext();
+    const auto* attr = ctx->getAttribute(attr_name);
+    if (attr == nullptr) {
+        LOG(warning, "No valid attribute vector found: '%s'", attr_name.c_str());
+        return std::unique_ptr<IDocsumFieldWriter>();
+    }
+    if (attr->hasMultiValue()) {
+        return create_multi_writer(*attr);
     } else {
-        return new SingleAttrDFW(vec->getName());
+        return std::make_unique<SingleAttrDFW>(attr->getName());
     }
 }
 
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.h b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.h
index 211becea16c..03de2cc76b4 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.h
@@ -8,12 +8,20 @@ namespace search::attribute { class IAttributeVector; }
 
 namespace search::docsummary {
 
+/**
+ * Factory to create an IDocsumFieldWriter to write an attribute vector to slime.
+ */
+class AttributeDFWFactory {
+public:
+    static std::unique_ptr<IDocsumFieldWriter> create(IAttributeManager& attr_mgr, const vespalib::string& attr_name);
+};
+
 class AttrDFW : public ISimpleDFW
 {
 private:
     vespalib::string _attrName;
 protected:
-    const attribute::IAttributeVector & vec(const GetDocsumsState & s) const;
+    const attribute::IAttributeVector& get_attribute(const GetDocsumsState& s) const;
     const vespalib::string & getAttributeName() const override { return _attrName; }
 public:
     AttrDFW(const vespalib::string & attrName);
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp
index 2c3366cb94d..f41ada8b2e8 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumconfig.cpp
@@ -88,11 +88,9 @@ DynamicDocsumConfig::createFieldWriter(const string & fieldName, const string &
             rc = fieldWriter.get();
         }
     } else if (overrideName == "attribute") {
-        const char *vectorName = argument.c_str();
         if (getEnvironment() && getEnvironment()->getAttributeManager()) {
-            IDocsumFieldWriter *fw = AttributeDFWFactory::create(*getEnvironment()->getAttributeManager(), vectorName);
-            fieldWriter.reset(fw);
-            rc = fw != NULL;
+            fieldWriter = AttributeDFWFactory::create(*getEnvironment()->getAttributeManager(), argument);
+            rc = static_cast<bool>(fieldWriter);
         }
     } else if (overrideName == "attributecombiner") {
         if (getEnvironment() && getEnvironment()->getAttributeManager()) {
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumfieldwriter.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumfieldwriter.h
index 43375fb47f3..a40f105a1cb 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumfieldwriter.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumfieldwriter.h
@@ -84,14 +84,5 @@ public:
                      vespalib::slime::Inserter &target) override;
 };
 
-//--------------------------------------------------------------------------
-
-class AttributeDFWFactory
-{
-private:
-    AttributeDFWFactory();
-public:
-    static IDocsumFieldWriter *create(IAttributeManager & vecMan, const char *vecName);
-};
 
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.cpp
index ae3d6acde43..df510d5bcbc 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/geoposdfw.cpp
@@ -47,7 +47,7 @@ GeoPositionDFW::insertField(uint32_t docid, GetDocsumsState * dsState, ResType,
     using vespalib::slime::Symbol;
     using vespalib::slime::ArrayInserter;
 
-    const IAttributeVector & attribute = vec(*dsState);
+    const auto& attribute = get_attribute(*dsState);
     if (attribute.hasMultiValue()) {
         uint32_t entries = attribute.getValueCount(docid);
         Cursor &arr = target.insertArray();
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp
index 6b003553f49..ecdde13b919 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp
@@ -30,7 +30,7 @@ uint64_t
 AbsDistanceDFW::findMinDistance(uint32_t docid, GetDocsumsState *state)
 {
     search::common::Location &location = *state->_parsedLocation;
-    const IAttributeVector & attribute(vec(*state));
+    const auto& attribute = get_attribute(*state);
 
     uint64_t absdist = std::numeric_limits<int64_t>::max();
     int32_t docx = 0;
@@ -221,10 +221,10 @@ void
 PositionsDFW::insertField(uint32_t docid, GetDocsumsState * dsState, ResType type, vespalib::slime::Inserter &target)
 {
     if (type == RES_XMLSTRING) {
-        insertFromAttr(vec(*dsState), docid, target);
+        insertFromAttr(get_attribute(*dsState), docid, target);
         return;
     }
-    vespalib::asciistream val(formatField(vec(*dsState), docid, type));
+    vespalib::asciistream val(formatField(get_attribute(*dsState), docid, type));
     target.insertString(vespalib::Memory(val.c_str(), val.size()));
 }