diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2018-06-08 17:13:31 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-06-08 17:13:31 +0200 |
commit | e14f7006a20d14f2445bcf22a7445267ffaaed40 (patch) | |
tree | fad36a022136edb9bcbbca1b2d0348640557d274 | |
parent | 8232481e8cf9c1df4c6f98fa8e3dacd0f226cf0e (diff) | |
parent | 8170cbe91308e4789f2a35f00d79af900beb0b20 (diff) |
Merge pull request #6141 from vespa-engine/geirst/add-support-for-primitive-map-attribute-fields
Geirst/add support for primitive map attribute fields
17 files changed, 358 insertions, 75 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/AttributeFields.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/AttributeFields.java index 6b4a9564e1e..0f1918f99d6 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/AttributeFields.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/AttributeFields.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isArrayOfSimpleStruct; +import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isMapOfPrimitiveType; import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isMapOfSimpleStruct; /** @@ -45,7 +46,7 @@ public class AttributeFields extends Derived implements AttributesConfig.Produce @Override protected void derive(ImmutableSDField field, Search search) { if (unsupportedFieldType(field)) { - return; // Ignore majority of struct fields for indexed search (only implemented for streaming search) + return; // Ignore complex struct and map fields for indexed search (only supported for streaming search) } if (field.isImportedField()) { deriveImportedAttributes(field); @@ -53,6 +54,8 @@ public class AttributeFields extends Derived implements AttributesConfig.Produce deriveArrayOfSimpleStruct(field); } else if (isMapOfSimpleStruct(field)) { deriveMapOfSimpleStruct(field); + } else if (isMapOfPrimitiveType(field)) { + deriveMapOfPrimitiveType(field); } else { deriveAttributes(field); } @@ -62,6 +65,7 @@ public class AttributeFields extends Derived implements AttributesConfig.Produce return (field.usesStructOrMap() && !isArrayOfSimpleStruct(field) && !isMapOfSimpleStruct(field) && + !isMapOfPrimitiveType(field) && !field.getDataType().equals(PositionDataType.INSTANCE) && !field.getDataType().equals(DataType.getArray(PositionDataType.INSTANCE))); } @@ -113,33 +117,33 @@ public class AttributeFields extends Derived implements AttributesConfig.Produce private void deriveArrayOfSimpleStruct(ImmutableSDField field) { for (ImmutableSDField structField : field.getStructFields()) { - deriveAttributesAsArrayType(structField); + deriveAttributeAsArrayType(structField); } } - private void deriveAttributesAsArrayType(ImmutableSDField field) { - for (Attribute attribute : field.getAttributes().values()) { - if (field.getName().equals(attribute.getName())) { - attributes.put(attribute.getName(), attribute.convertToArray()); - } + private void deriveAttributeAsArrayType(ImmutableSDField field) { + Attribute attribute = field.getAttributes().get(field.getName()); + if (attribute != null) { + attributes.put(attribute.getName(), attribute.convertToArray()); } } private void deriveMapOfSimpleStruct(ImmutableSDField field) { - deriveMapKeyField(field.getStructField("key")); + deriveAttributeAsArrayType(field.getStructField("key")); deriveMapValueField(field.getStructField("value")); } - private void deriveMapKeyField(ImmutableSDField keyField) { - deriveAttributesAsArrayType(keyField); - } - private void deriveMapValueField(ImmutableSDField valueField) { for (ImmutableSDField structField : valueField.getStructFields()) { - deriveAttributesAsArrayType(structField); + deriveAttributeAsArrayType(structField); } } + private void deriveMapOfPrimitiveType(ImmutableSDField field) { + deriveAttributeAsArrayType(field.getStructField("key")); + deriveAttributeAsArrayType(field.getStructField("value")); + } + /** Returns a read only attribute iterator */ public Iterator attributeIterator() { return attributes().iterator(); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/ComplexAttributeFieldUtils.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/ComplexAttributeFieldUtils.java index a46e94398a1..89d72756512 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/ComplexAttributeFieldUtils.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/ComplexAttributeFieldUtils.java @@ -39,6 +39,17 @@ public class ComplexAttributeFieldUtils { } } + public static boolean isMapOfPrimitiveType(ImmutableSDField field) { + DataType fieldType = field.getDataType(); + if (fieldType instanceof MapDataType) { + MapDataType mapType = (MapDataType)fieldType; + return isPrimitiveType(mapType.getKeyType()) && + isPrimitiveType(mapType.getValueType()); + } else { + return false; + } + } + private static boolean isSimpleStruct(DataType type) { if (type instanceof StructDataType && !(type.equals(PositionDataType.INSTANCE))) { @@ -69,6 +80,9 @@ public class ComplexAttributeFieldUtils { } else if (isMapOfSimpleStruct(field)) { return hasSingleAttribute(field.getStructField("key")) && hasOnlyStructFieldAttributes(field.getStructField("value")); + } else if (isMapOfPrimitiveType(field)) { + return hasSingleAttribute(field.getStructField("key")) && + hasSingleAttribute(field.getStructField("value")); } return false; } diff --git a/config-model/src/test/derived/map_attribute/attributes.cfg b/config-model/src/test/derived/map_attribute/attributes.cfg new file mode 100644 index 00000000000..cdd929a1818 --- /dev/null +++ b/config-model/src/test/derived/map_attribute/attributes.cfg @@ -0,0 +1,60 @@ +attribute[].name "str_map.key" +attribute[].datatype STRING +attribute[].collectiontype ARRAY +attribute[].removeifzero false +attribute[].createifnonexistent false +attribute[].fastsearch false +attribute[].huge false +attribute[].sortascending true +attribute[].sortfunction UCA +attribute[].sortstrength PRIMARY +attribute[].sortlocale "" +attribute[].enablebitvectors false +attribute[].enableonlybitvector false +attribute[].fastaccess false +attribute[].arity 8 +attribute[].lowerbound -9223372036854775808 +attribute[].upperbound 9223372036854775807 +attribute[].densepostinglistthreshold 0.4 +attribute[].tensortype "" +attribute[].imported false +attribute[].name "str_map.value" +attribute[].datatype STRING +attribute[].collectiontype ARRAY +attribute[].removeifzero false +attribute[].createifnonexistent false +attribute[].fastsearch false +attribute[].huge false +attribute[].sortascending true +attribute[].sortfunction UCA +attribute[].sortstrength PRIMARY +attribute[].sortlocale "" +attribute[].enablebitvectors false +attribute[].enableonlybitvector false +attribute[].fastaccess false +attribute[].arity 8 +attribute[].lowerbound -9223372036854775808 +attribute[].upperbound 9223372036854775807 +attribute[].densepostinglistthreshold 0.4 +attribute[].tensortype "" +attribute[].imported false +attribute[].name "int_map.key" +attribute[].datatype INT32 +attribute[].collectiontype ARRAY +attribute[].removeifzero false +attribute[].createifnonexistent false +attribute[].fastsearch false +attribute[].huge false +attribute[].sortascending true +attribute[].sortfunction UCA +attribute[].sortstrength PRIMARY +attribute[].sortlocale "" +attribute[].enablebitvectors false +attribute[].enableonlybitvector false +attribute[].fastaccess false +attribute[].arity 8 +attribute[].lowerbound -9223372036854775808 +attribute[].upperbound 9223372036854775807 +attribute[].densepostinglistthreshold 0.4 +attribute[].tensortype "" +attribute[].imported false diff --git a/config-model/src/test/derived/map_attribute/summary.cfg b/config-model/src/test/derived/map_attribute/summary.cfg new file mode 100644 index 00000000000..24d6cab7697 --- /dev/null +++ b/config-model/src/test/derived/map_attribute/summary.cfg @@ -0,0 +1,13 @@ +defaultsummaryid 1376056200 +classes[].id 1376056200 +classes[].name "default" +classes[].fields[].name "str_map" +classes[].fields[].type "jsonstring" +classes[].fields[].name "int_map" +classes[].fields[].type "jsonstring" +classes[].fields[].name "rankfeatures" +classes[].fields[].type "featuredata" +classes[].fields[].name "summaryfeatures" +classes[].fields[].type "featuredata" +classes[].fields[].name "documentid" +classes[].fields[].type "longstring" diff --git a/config-model/src/test/derived/map_attribute/summarymap.cfg b/config-model/src/test/derived/map_attribute/summarymap.cfg new file mode 100644 index 00000000000..8936b26ca64 --- /dev/null +++ b/config-model/src/test/derived/map_attribute/summarymap.cfg @@ -0,0 +1,10 @@ +defaultoutputclass -1 +override[].field "str_map" +override[].command "attributecombiner" +override[].arguments "" +override[].field "rankfeatures" +override[].command "rankfeatures" +override[].arguments "" +override[].field "summaryfeatures" +override[].command "summaryfeatures" +override[].arguments "" diff --git a/config-model/src/test/derived/map_attribute/test.sd b/config-model/src/test/derived/map_attribute/test.sd new file mode 100644 index 00000000000..da31f575396 --- /dev/null +++ b/config-model/src/test/derived/map_attribute/test.sd @@ -0,0 +1,19 @@ +search test { + document test { + field str_map type map<string, string> { + indexing: summary + struct-field key { + indexing: attribute + } + struct-field value { + indexing: attribute + } + } + field int_map type map<int, int> { + indexing: summary + struct-field key { + indexing: attribute + } + } + } +} diff --git a/config-model/src/test/derived/map_of_struct_attribute/attributes.cfg b/config-model/src/test/derived/map_of_struct_attribute/attributes.cfg index 9ada79e78bd..95cf8f761a0 100644 --- a/config-model/src/test/derived/map_of_struct_attribute/attributes.cfg +++ b/config-model/src/test/derived/map_of_struct_attribute/attributes.cfg @@ -1,4 +1,4 @@ -attribute[].name "elem_map.key" +attribute[].name "str_elem_map.key" attribute[].datatype STRING attribute[].collectiontype ARRAY attribute[].removeifzero false @@ -18,7 +18,7 @@ attribute[].upperbound 9223372036854775807 attribute[].densepostinglistthreshold 0.4 attribute[].tensortype "" attribute[].imported false -attribute[].name "elem_map.value.name" +attribute[].name "str_elem_map.value.name" attribute[].datatype STRING attribute[].collectiontype ARRAY attribute[].removeifzero false @@ -38,7 +38,7 @@ attribute[].upperbound 9223372036854775807 attribute[].densepostinglistthreshold 0.4 attribute[].tensortype "" attribute[].imported false -attribute[].name "elem_map.value.weight" +attribute[].name "str_elem_map.value.weight" attribute[].datatype INT32 attribute[].collectiontype ARRAY attribute[].removeifzero false @@ -58,3 +58,43 @@ attribute[].upperbound 9223372036854775807 attribute[].densepostinglistthreshold 0.4 attribute[].tensortype "" attribute[].imported false +attribute[].name "int_elem_map.key" +attribute[].datatype INT32 +attribute[].collectiontype ARRAY +attribute[].removeifzero false +attribute[].createifnonexistent false +attribute[].fastsearch false +attribute[].huge false +attribute[].sortascending true +attribute[].sortfunction UCA +attribute[].sortstrength PRIMARY +attribute[].sortlocale "" +attribute[].enablebitvectors false +attribute[].enableonlybitvector false +attribute[].fastaccess false +attribute[].arity 8 +attribute[].lowerbound -9223372036854775808 +attribute[].upperbound 9223372036854775807 +attribute[].densepostinglistthreshold 0.4 +attribute[].tensortype "" +attribute[].imported false +attribute[].name "int_elem_map.value.name" +attribute[].datatype STRING +attribute[].collectiontype ARRAY +attribute[].removeifzero false +attribute[].createifnonexistent false +attribute[].fastsearch false +attribute[].huge false +attribute[].sortascending true +attribute[].sortfunction UCA +attribute[].sortstrength PRIMARY +attribute[].sortlocale "" +attribute[].enablebitvectors false +attribute[].enableonlybitvector false +attribute[].fastaccess false +attribute[].arity 8 +attribute[].lowerbound -9223372036854775808 +attribute[].upperbound 9223372036854775807 +attribute[].densepostinglistthreshold 0.4 +attribute[].tensortype "" +attribute[].imported false diff --git a/config-model/src/test/derived/map_of_struct_attribute/summary.cfg b/config-model/src/test/derived/map_of_struct_attribute/summary.cfg index 4e1e3aec8aa..f70025c8f02 100644 --- a/config-model/src/test/derived/map_of_struct_attribute/summary.cfg +++ b/config-model/src/test/derived/map_of_struct_attribute/summary.cfg @@ -1,7 +1,9 @@ -defaultsummaryid 653486243 -classes[].id 653486243 +defaultsummaryid 1131098132 +classes[].id 1131098132 classes[].name "default" -classes[].fields[].name "elem_map" +classes[].fields[].name "str_elem_map" +classes[].fields[].type "jsonstring" +classes[].fields[].name "int_elem_map" classes[].fields[].type "jsonstring" classes[].fields[].name "rankfeatures" classes[].fields[].type "featuredata" diff --git a/config-model/src/test/derived/map_of_struct_attribute/summarymap.cfg b/config-model/src/test/derived/map_of_struct_attribute/summarymap.cfg index ccfba719a96..1540b821ae1 100644 --- a/config-model/src/test/derived/map_of_struct_attribute/summarymap.cfg +++ b/config-model/src/test/derived/map_of_struct_attribute/summarymap.cfg @@ -1,5 +1,5 @@ defaultoutputclass -1 -override[].field "elem_map" +override[].field "str_elem_map" override[].command "attributecombiner" override[].arguments "" override[].field "rankfeatures" diff --git a/config-model/src/test/derived/map_of_struct_attribute/test.sd b/config-model/src/test/derived/map_of_struct_attribute/test.sd index cb2eac4ed78..e139a7f8168 100644 --- a/config-model/src/test/derived/map_of_struct_attribute/test.sd +++ b/config-model/src/test/derived/map_of_struct_attribute/test.sd @@ -4,7 +4,7 @@ search test { field name type string {} field weight type int {} } - field elem_map type map<string,elem> { + field str_elem_map type map<string,elem> { indexing: summary struct-field key { indexing: attribute @@ -16,5 +16,14 @@ search test { indexing: attribute } } + field int_elem_map type map<int,elem> { + indexing: summary + struct-field key { + indexing: attribute + } + struct-field value.name { + indexing: attribute + } + } } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/AttributeListTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/AttributeListTestCase.java index c3cfcae66e6..bd762c64c00 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/AttributeListTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/AttributeListTestCase.java @@ -70,7 +70,7 @@ public class AttributeListTestCase extends SearchDefinitionTestCase { } @Test - public void array_of_struct_field_is_derived_into_array_attributes() throws IOException, ParseException { + public void fields_in_array_of_struct_are_derived_into_array_attributes() throws IOException, ParseException { Search search = SearchBuilder.buildFromFile("src/test/derived/array_of_struct_attribute/test.sd"); Iterator<Attribute> attributes = new AttributeFields(search).attributeIterator(); @@ -80,13 +80,15 @@ public class AttributeListTestCase extends SearchDefinitionTestCase { } @Test - public void map_of_struct_field_is_derived_into_array_attributes() throws IOException, ParseException { + public void fields_in_map_of_struct_are_derived_into_array_attributes() throws IOException, ParseException { Search search = SearchBuilder.buildFromFile("src/test/derived/map_of_struct_attribute/test.sd"); Iterator<Attribute> attributes = new AttributeFields(search).attributeIterator(); - assertAttribute("elem_map.key", Attribute.Type.STRING, Attribute.CollectionType.ARRAY, attributes.next()); - assertAttribute("elem_map.value.name", Attribute.Type.STRING, Attribute.CollectionType.ARRAY, attributes.next()); - assertAttribute("elem_map.value.weight", Attribute.Type.INTEGER, Attribute.CollectionType.ARRAY, attributes.next()); + assertAttribute("str_elem_map.key", Attribute.Type.STRING, Attribute.CollectionType.ARRAY, attributes.next()); + assertAttribute("str_elem_map.value.name", Attribute.Type.STRING, Attribute.CollectionType.ARRAY, attributes.next()); + assertAttribute("str_elem_map.value.weight", Attribute.Type.INTEGER, Attribute.CollectionType.ARRAY, attributes.next()); + assertAttribute("int_elem_map.key", Attribute.Type.INTEGER, Attribute.CollectionType.ARRAY, attributes.next()); + assertAttribute("int_elem_map.value.name", Attribute.Type.STRING, Attribute.CollectionType.ARRAY, attributes.next()); assertTrue(!attributes.hasNext()); } @@ -112,4 +114,15 @@ public class AttributeListTestCase extends SearchDefinitionTestCase { assertTrue(!attributes.hasNext()); } + @Test + public void fields_in_map_of_primitive_are_derived_into_array_attributes() throws IOException, ParseException { + Search search = SearchBuilder.buildFromFile("src/test/derived/map_attribute/test.sd"); + Iterator<Attribute> attributes = new AttributeFields(search).attributeIterator(); + + assertAttribute("str_map.key", Attribute.Type.STRING, Attribute.CollectionType.ARRAY, attributes.next()); + assertAttribute("str_map.value", Attribute.Type.STRING, Attribute.CollectionType.ARRAY, attributes.next()); + assertAttribute("int_map.key", Attribute.Type.INTEGER, Attribute.CollectionType.ARRAY, attributes.next()); + assertTrue(!attributes.hasNext()); + } + } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/AttributesTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/AttributesTestCase.java index 72c7aab4a39..2483d249125 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/AttributesTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/AttributesTestCase.java @@ -28,4 +28,9 @@ public class AttributesTestCase extends AbstractExportingTestCase { assertCorrectDeriving("map_of_struct_attribute"); } + @Test + public void testMapOfPrimitiveAttribute() throws IOException, ParseException { + assertCorrectDeriving("map_attribute"); + } + } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/document/ComplexAttributeFieldUtilsTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/document/ComplexAttributeFieldUtilsTestCase.java index 047ba73e784..4cdc48b330e 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/document/ComplexAttributeFieldUtilsTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/document/ComplexAttributeFieldUtilsTestCase.java @@ -8,6 +8,7 @@ import org.junit.Test; import static com.yahoo.config.model.test.TestUtil.joinLines; import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isArrayOfSimpleStruct; import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isComplexFieldWithOnlyStructFieldAttributes; +import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isMapOfPrimitiveType; import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isMapOfSimpleStruct; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -61,6 +62,7 @@ public class ComplexAttributeFieldUtilsTestCase { " struct-field value.weight { indexing: attribute }", "}")); assertTrue(isMapOfSimpleStruct(field)); + assertFalse(isMapOfPrimitiveType(field)); assertTrue(isComplexFieldWithOnlyStructFieldAttributes(field)); } @@ -68,22 +70,61 @@ public class ComplexAttributeFieldUtilsTestCase { public void map_of_struct_with_some_struct_field_attributes_is_tagged_as_such() throws ParseException { { ImmutableSDField field = createField("elem_map", - joinLines("field elem_map type map<string, elem> {", + joinLines("field elem_map type map<int, elem> {", " indexing: summary", " struct-field value.name { indexing: attribute }", " struct-field value.weight { indexing: attribute }", "}")); assertTrue(isMapOfSimpleStruct(field)); + assertFalse(isMapOfPrimitiveType(field)); assertFalse(isComplexFieldWithOnlyStructFieldAttributes(field)); } { ImmutableSDField field = createField("elem_map", - joinLines("field elem_map type map<string, elem> {", + joinLines("field elem_map type map<int, elem> {", " indexing: summary", " struct-field key { indexing: attribute }", " struct-field value.weight { indexing: attribute }", "}")); assertTrue(isMapOfSimpleStruct(field)); + assertFalse(isMapOfPrimitiveType(field)); + assertFalse(isComplexFieldWithOnlyStructFieldAttributes(field)); + } + } + + @Test + public void map_of_primitive_type_with_only_struct_field_attributes_is_tagged_as_such() throws ParseException { + ImmutableSDField field = createField("str_map", + joinLines("field str_map type map<string, string> {", + " indexing: summary", + " struct-field key { indexing: attribute }", + " struct-field value { indexing: attribute }", + "}")); + assertTrue(isMapOfPrimitiveType(field)); + assertFalse(isMapOfSimpleStruct(field)); + assertTrue(isComplexFieldWithOnlyStructFieldAttributes(field)); + } + + @Test + public void map_of_primitive_type_with_some_struct_field_attributes_is_tagged_as_such() throws ParseException { + { + ImmutableSDField field = createField("int_map", + joinLines("field int_map type map<int, int> {", + " indexing: summary", + " struct-field key { indexing: attribute }", + "}")); + assertTrue(isMapOfPrimitiveType(field)); + assertFalse(isMapOfSimpleStruct(field)); + assertFalse(isComplexFieldWithOnlyStructFieldAttributes(field)); + } + { + ImmutableSDField field = createField("int_map", + joinLines("field int_map type map<int, int> {", + " indexing: summary", + " struct-field value { indexing: attribute }", + "}")); + assertTrue(isMapOfPrimitiveType(field)); + assertFalse(isMapOfSimpleStruct(field)); assertFalse(isComplexFieldWithOnlyStructFieldAttributes(field)); } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImplicitSummariesTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImplicitSummariesTestCase.java index 9212617a870..da0732dfbaa 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImplicitSummariesTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImplicitSummariesTestCase.java @@ -67,6 +67,12 @@ public class ImplicitSummariesTestCase { @Test public void attribute_combiner_transform_is_set_on_map_of_struct_with_only_struct_field_attributes() throws IOException, ParseException { Search search = SearchBuilder.buildFromFile("src/test/derived/map_of_struct_attribute/test.sd"); - assertEquals(SummaryTransform.ATTRIBUTECOMBINER, search.getSummaryField("elem_map").getTransform()); + assertEquals(SummaryTransform.ATTRIBUTECOMBINER, search.getSummaryField("str_elem_map").getTransform()); + } + + @Test + public void attribute_combiner_transform_is_not_set_when_map_of_struct_has_some_struct_field_attributes() throws IOException, ParseException { + Search search = SearchBuilder.buildFromFile("src/test/derived/map_of_struct_attribute/test.sd"); + assertEquals(SummaryTransform.NONE, search.getSummaryField("int_elem_map").getTransform()); } } 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); |