Merge pull request #6141 from vespa-engine/geirst/add-support-for-primitive-map-attribute-fields

Geirst/add support for primitive map attribute fields

3 files changed, 93 insertions, 46 deletions

diff --git a/searchcore/src/tests/proton/attribute/document_field_extractor/document_field_extractor_test.cpp b/searchcore/src/tests/proton/attribute/document_field_extractor/document_field_extractor_test.cpp
index bad27938d4b..33a3de0c5b6 100644
--- a/searchcore/src/tests/proton/attribute/document_field_extractor/document_field_extractor_test.cpp
+++ b/searchcore/src/tests/proton/attribute/document_field_extractor/document_field_extractor_test.cpp
@@ -36,8 +36,7 @@ using document::FieldValue;
 using document::FieldNotFoundException;
 using proton::DocumentFieldExtractor;
 
-namespace
-{
+namespace {
 
 const ArrayDataType arrayTypeInt(*DataType::INT);
 const ArrayDataType arrayTypeString(*DataType::STRING);
@@ -335,6 +334,41 @@ TEST_F("require that struct map field gives array values", StructMapFixture)
     TEST_DO(f.assertExtracted("s.value.name", makeStringArray({ "name10", noString, "name12", noString })));
 }
 
+struct PrimitiveMapFixture : public FixtureBase
+{
+    MapDataType mapFieldType;
+    Field mapField;
+    using MapVector = std::vector<std::pair<vespalib::string, int>>;
+
+    PrimitiveMapFixture()
+        : FixtureBase(false),
+          mapFieldType(nameField.getDataType(), weightField.getDataType()),
+          mapField("map", mapFieldType, true)
+    {
+        type.addField(mapField);
+    }
+
+    std::unique_ptr<MapFieldValue> makeMap(const MapVector &input) {
+        auto result = std::make_unique<MapFieldValue>(mapFieldType);
+        for (const auto &elem : input) {
+            result->put(StringFieldValue(elem.first), IntFieldValue(elem.second));
+        }
+        return result;
+    }
+
+    void makeDoc(const MapVector &input) {
+        FixtureBase::makeDoc()->setValue(mapField, *makeMap(input));
+    }
+
+};
+
+TEST_F("require that primitive map field gives array values", PrimitiveMapFixture)
+{
+    f.makeDoc({ {"foo", 10}, {"", 20}, {"bar", noInt} });
+    TEST_DO(f.assertExtracted("map.key", makeStringArray({ "foo", "", "bar" })));
+    TEST_DO(f.assertExtracted("map.value", makeIntArray({ 10, 20, noInt })));
+}
+
 TEST_F("require that unknown field gives null value", FixtureBase(false))
 {
     f.makeDoc();
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/document_field_extractor.cpp b/searchcore/src/vespa/searchcore/proton/attribute/document_field_extractor.cpp
index 46f3fdeff67..64c3455be84 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/document_field_extractor.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/document_field_extractor.cpp
@@ -11,12 +11,10 @@
 #include <vespa/document/fieldvalue/longfieldvalue.h>
 #include <vespa/document/fieldvalue/shortfieldvalue.h>
 #include <vespa/document/fieldvalue/stringfieldvalue.h>
-#include <vespa/document/fieldvalue/structfieldvalue.h>
 #include <vespa/document/fieldvalue/mapfieldvalue.h>
 #include <vespa/searchcommon/common/undefinedvalues.h>
 #include <vespa/vespalib/stllike/hash_map.hpp>
 #include <vespa/vespalib/util/exceptions.h>
-#include <vespa/vespalib/util/stringfmt.h>
 
 using document::FieldValue;
 using document::ByteFieldValue;
@@ -132,7 +130,8 @@ DocumentFieldExtractor::isSupported(const FieldPath &fieldPath)
     }
     if (fieldPath.size() == 2) {
         if (fieldPath[1].getType() != FieldPathEntry::Type::STRUCT_FIELD &&
-            fieldPath[1].getType() != FieldPathEntry::Type::MAP_ALL_KEYS) {
+            fieldPath[1].getType() != FieldPathEntry::Type::MAP_ALL_KEYS &&
+            fieldPath[1].getType() != FieldPathEntry::Type::MAP_ALL_VALUES) {
             return false;
         }
     } else if (fieldPath.size() == 3) {
@@ -165,56 +164,36 @@ DocumentFieldExtractor::getSimpleFieldValue(const FieldPath &fieldPath)
     return _doc.getNestedFieldValue(fieldPath.getFullRange());
 }
 
-std::unique_ptr<FieldValue>
-DocumentFieldExtractor::getStructArrayFieldValue(const FieldPath &fieldPath)
-{
-    const auto outerFieldValue = getCachedFieldValue(fieldPath[0]);
-    if (outerFieldValue != nullptr && checkInherits(*outerFieldValue, ArrayFieldValue::classId)) {
-        const auto outerArray = static_cast<const ArrayFieldValue *>(outerFieldValue);
-        const auto &innerFieldPathEntry = fieldPath[1];
-        auto array = makeArray(innerFieldPathEntry, outerArray->size());
-        uint32_t arrayIndex = 0;
-        for (const auto &outerElemBase : *outerArray) {
-            auto &arrayElem = (*array)[arrayIndex++];
-            const auto &structElem = static_cast<const StructFieldValue &>(outerElemBase);
-            if (!structElem.getValue(innerFieldPathEntry.getFieldRef(), arrayElem)) {
-                arrayElem.accept(setUndefinedValueVisitor);
-            }
-        }
-        return array;
-    }
-    return std::unique_ptr<FieldValue>();
-}
+namespace {
 
+template <typename ExtractorFunc>
 std::unique_ptr<FieldValue>
-DocumentFieldExtractor::getStructMapKeyFieldValue(const FieldPath &fieldPath)
+extractFieldFromMap(const FieldValue *outerFieldValue, const FieldPathEntry &innerEntry, ExtractorFunc &&extractor)
 {
-    const auto outerFieldValue = getCachedFieldValue(fieldPath[0]);
     if (outerFieldValue != nullptr && checkInherits(*outerFieldValue, MapFieldValue::classId)) {
         const auto outerMap = static_cast<const MapFieldValue *>(outerFieldValue);
-        auto array = makeArray(fieldPath[1], outerMap->size());
+        auto array = makeArray(innerEntry, outerMap->size());
         uint32_t arrayIndex = 0;
         for (const auto &mapElem : *outerMap) {
-            (*array)[arrayIndex++].assign(*mapElem.first);
+            (*array)[arrayIndex++].assign(*extractor(mapElem));
         }
         return array;
     }
     return std::unique_ptr<FieldValue>();
 }
 
-std::unique_ptr<document::FieldValue>
-DocumentFieldExtractor::getStructMapFieldValue(const FieldPath &fieldPath)
+template <typename CollectionFieldValueT, typename ExtractorFunc>
+std::unique_ptr<FieldValue>
+extractFieldFromStructCollection(const FieldValue *outerFieldValue, const FieldPathEntry &innerEntry, ExtractorFunc &&extractor)
 {
-    const auto outerFieldValue = getCachedFieldValue(fieldPath[0]);
-    if (outerFieldValue != nullptr && checkInherits(*outerFieldValue, MapFieldValue::classId)) {
-        const auto outerMap = static_cast<const MapFieldValue *>(outerFieldValue);
-        const auto &innerFieldPathEntry = fieldPath[2];
-        auto array = makeArray(innerFieldPathEntry, outerMap->size());
+    if (outerFieldValue != nullptr && checkInherits(*outerFieldValue, CollectionFieldValueT::classId)) {
+        const auto *outerCollection = static_cast<const CollectionFieldValueT *>(outerFieldValue);
+        auto array = makeArray(innerEntry, outerCollection->size());
         uint32_t arrayIndex = 0;
-        for (const auto &mapElem : *outerMap) {
+        for (const auto &outerElem : *outerCollection) {
             auto &arrayElem = (*array)[arrayIndex++];
-            const auto &structElem = static_cast<const StructFieldValue &>(*mapElem.second);
-            if (!structElem.getValue(innerFieldPathEntry.getFieldRef(), arrayElem)) {
+            const auto &structElem = static_cast<const StructFieldValue &>(*extractor(&outerElem));
+            if (!structElem.getValue(innerEntry.getFieldRef(), arrayElem)) {
                 arrayElem.accept(setUndefinedValueVisitor);
             }
         }
@@ -223,19 +202,52 @@ DocumentFieldExtractor::getStructMapFieldValue(const FieldPath &fieldPath)
     return std::unique_ptr<FieldValue>();
 }
 
+}
+
+std::unique_ptr<FieldValue>
+DocumentFieldExtractor::extractFieldFromStructArray(const FieldPath &fieldPath)
+{
+    return extractFieldFromStructCollection<ArrayFieldValue>(getCachedFieldValue(fieldPath[0]), fieldPath[1],
+                                                             [](const auto *elem){ return elem; });
+}
+
+std::unique_ptr<FieldValue>
+DocumentFieldExtractor::extractKeyFieldFromMap(const FieldPath &fieldPath)
+{
+    return extractFieldFromMap(getCachedFieldValue(fieldPath[0]), fieldPath[1],
+                               [](const auto &elem){ return elem.first; });
+}
+
+std::unique_ptr<document::FieldValue>
+DocumentFieldExtractor::extractValueFieldFromPrimitiveMap(const FieldPath &fieldPath)
+{
+    return extractFieldFromMap(getCachedFieldValue(fieldPath[0]), fieldPath[1],
+                               [](const auto &elem){ return elem.second; });
+}
+
+std::unique_ptr<document::FieldValue>
+DocumentFieldExtractor::extractValueFieldFromStructMap(const FieldPath &fieldPath)
+{
+    return extractFieldFromStructCollection<MapFieldValue>(getCachedFieldValue(fieldPath[0]), fieldPath[2],
+                                                           [](const auto *elem){ return elem->second; });
+}
+
 std::unique_ptr<FieldValue>
 DocumentFieldExtractor::getFieldValue(const FieldPath &fieldPath)
 {
     if (fieldPath.size() == 1) {
         return getSimpleFieldValue(fieldPath);
     } else if (fieldPath.size() == 2) {
-        if (fieldPath[1].getType() == FieldPathEntry::Type::STRUCT_FIELD) {
-            return getStructArrayFieldValue(fieldPath);
+        auto lastElemType = fieldPath[1].getType();
+        if (lastElemType == FieldPathEntry::Type::STRUCT_FIELD) {
+            return extractFieldFromStructArray(fieldPath);
+        } else if (lastElemType == FieldPathEntry::Type::MAP_ALL_KEYS) {
+            return extractKeyFieldFromMap(fieldPath);
         } else {
-            return getStructMapKeyFieldValue(fieldPath);
+            return extractValueFieldFromPrimitiveMap(fieldPath);
         }
     } else if (fieldPath.size() == 3) {
-        return getStructMapFieldValue(fieldPath);
+        return extractValueFieldFromStructMap(fieldPath);
     }
     return std::unique_ptr<FieldValue>();
 }
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/document_field_extractor.h b/searchcore/src/vespa/searchcore/proton/attribute/document_field_extractor.h
index 48e2da9c4c6..b9543bd87c8 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/document_field_extractor.h
+++ b/searchcore/src/vespa/searchcore/proton/attribute/document_field_extractor.h
@@ -27,9 +27,10 @@ class DocumentFieldExtractor
 
     const document::FieldValue *getCachedFieldValue(const document::FieldPathEntry &fieldPathEntry);
     std::unique_ptr<document::FieldValue> getSimpleFieldValue(const document::FieldPath &fieldPath);
-    std::unique_ptr<document::FieldValue> getStructArrayFieldValue(const document::FieldPath &fieldPath);
-    std::unique_ptr<document::FieldValue> getStructMapKeyFieldValue(const document::FieldPath &fieldPath);
-    std::unique_ptr<document::FieldValue> getStructMapFieldValue(const document::FieldPath &fieldPath);
+    std::unique_ptr<document::FieldValue> extractFieldFromStructArray(const document::FieldPath &fieldPath);
+    std::unique_ptr<document::FieldValue> extractKeyFieldFromMap(const document::FieldPath &fieldPath);
+    std::unique_ptr<document::FieldValue> extractValueFieldFromPrimitiveMap(const document::FieldPath &fieldPath);
+    std::unique_ptr<document::FieldValue> extractValueFieldFromStructMap(const document::FieldPath &fieldPath);
 
 public:
     DocumentFieldExtractor(const document::Document &doc);