diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2018-08-29 15:19:49 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-08-29 15:19:49 +0200 |
commit | 06323aff51bf054d64ef2bea001917a22433717f (patch) | |
tree | 64d37e3d33041392e9512733de791847556e0fba /config-model | |
parent | 27279eb6f2b23bcce4c8a755e4b7ca3456110803 (diff) | |
parent | 49fde9abcf86600bcbfedc46f6fd4ec0740ec41a (diff) |
Merge pull request #6718 from vespa-engine/geirst/relax-validation-of-complex-attribute-fields
Relax validation of complex attribute fields by only considering the …
Diffstat (limited to 'config-model')
7 files changed, 144 insertions, 105 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 a3580a404a3..a8f6dd09497 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 @@ -6,9 +6,9 @@ import com.yahoo.document.DataType; import com.yahoo.document.PositionDataType; import com.yahoo.searchdefinition.Search; import com.yahoo.searchdefinition.document.Attribute; +import com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils; import com.yahoo.searchdefinition.document.ImmutableSDField; import com.yahoo.searchdefinition.document.Ranking; -import com.yahoo.searchdefinition.document.SDDocumentType; import com.yahoo.searchdefinition.document.Sorting; import com.yahoo.vespa.config.search.AttributesConfig; import com.yahoo.vespa.indexinglanguage.expressions.ToPositionExpression; @@ -51,14 +51,14 @@ public class AttributeFields extends Derived implements AttributesConfig.Produce /** Derives everything from a field */ @Override protected void derive(ImmutableSDField field, Search search) { - if (unsupportedFieldType(field, search.getDocument())) { + if (unsupportedFieldType(field)) { return; // Ignore complex struct and map fields for indexed search (only supported for streaming search) } if (field.isImportedField()) { deriveImportedAttributes(field); - } else if (isArrayOfSimpleStruct(field, search.getDocument())) { + } else if (isArrayOfSimpleStruct(field)) { deriveArrayOfSimpleStruct(field); - } else if (isMapOfSimpleStruct(field, search.getDocument())) { + } else if (isMapOfSimpleStruct(field)) { deriveMapOfSimpleStruct(field); } else if (isMapOfPrimitiveType(field)) { deriveMapOfPrimitiveType(field); @@ -67,9 +67,9 @@ public class AttributeFields extends Derived implements AttributesConfig.Produce } } - private static boolean unsupportedFieldType(ImmutableSDField field, SDDocumentType docType) { + private static boolean unsupportedFieldType(ImmutableSDField field) { return (field.usesStructOrMap() && - !isSupportedComplexField(field, docType) && + !isSupportedComplexField(field) && !field.getDataType().equals(PositionDataType.INSTANCE) && !field.getDataType().equals(DataType.getArray(PositionDataType.INSTANCE))); } 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 72eb1c96e0f..c3ac8c77173 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 @@ -3,15 +3,9 @@ package com.yahoo.searchdefinition.document; import com.yahoo.document.ArrayDataType; import com.yahoo.document.DataType; -import com.yahoo.document.Field; import com.yahoo.document.MapDataType; import com.yahoo.document.PositionDataType; import com.yahoo.document.StructDataType; -import com.yahoo.document.TemporaryStructuredDataType; - -import java.util.Collection; -import java.util.Collections; -import java.util.Optional; /** * Utils used to check whether a complex field supports being represented as struct field attributes. @@ -21,64 +15,42 @@ import java.util.Optional; * - map of primitive type to simple struct * - map of primitive type to primitive type * + * A simple struct can contain fields of any type, but only fields of primitive type can be defined as + * struct field attributes in the complex field using the simple struct. + * * @author geirst */ public class ComplexAttributeFieldUtils { - public static boolean isSupportedComplexField(ImmutableSDField field, SDDocumentType docType) { - return (isArrayOfSimpleStruct(field, docType) || - isMapOfSimpleStruct(field, docType) || + public static boolean isSupportedComplexField(ImmutableSDField field) { + return (isArrayOfSimpleStruct(field) || + isMapOfSimpleStruct(field) || isMapOfPrimitiveType(field)); } - public static boolean isSupportedComplexField(DataType fieldType) { - return (isArrayOfSimpleStruct(fieldType) || - isMapOfSimpleStruct(fieldType) || - isMapOfPrimitiveType(fieldType)); - } - - public static boolean isArrayOfSimpleStruct(ImmutableSDField field, SDDocumentType docType) { - return isArrayOfSimpleStruct(field.getDataType(), Optional.of(docType)); - } - - public static boolean isArrayOfSimpleStruct(DataType fieldType) { - return isArrayOfSimpleStruct(fieldType, Optional.empty()); - } - - private static boolean isArrayOfSimpleStruct(DataType fieldType, Optional<SDDocumentType> docType) { - if (fieldType instanceof ArrayDataType) { - ArrayDataType arrayType = (ArrayDataType)fieldType; - return isSimpleStruct(arrayType.getNestedType(), docType); + public static boolean isArrayOfSimpleStruct(ImmutableSDField field) { + if (field.getDataType() instanceof ArrayDataType) { + ArrayDataType arrayType = (ArrayDataType)field.getDataType(); + return isStructWithPrimitiveStructFieldAttributes(arrayType.getNestedType(), field); } else { return false; } } - public static boolean isMapOfSimpleStruct(ImmutableSDField field, SDDocumentType docType) { - return isMapOfSimpleStruct(field.getDataType(), Optional.of(docType)); - } - - public static boolean isMapOfSimpleStruct(DataType fieldType) { - return isMapOfSimpleStruct(fieldType, Optional.empty()); - } - - private static boolean isMapOfSimpleStruct(DataType fieldType, Optional<SDDocumentType> docType) { - if (fieldType instanceof MapDataType) { - MapDataType mapType = (MapDataType)fieldType; + public static boolean isMapOfSimpleStruct(ImmutableSDField field) { + if (field.getDataType() instanceof MapDataType) { + MapDataType mapType = (MapDataType)field.getDataType(); return isPrimitiveType(mapType.getKeyType()) && - isSimpleStruct(mapType.getValueType(), docType); + isStructWithPrimitiveStructFieldAttributes(mapType.getValueType(), + field.getStructField("value")); } else { return false; } } public static boolean isMapOfPrimitiveType(ImmutableSDField field) { - return isMapOfPrimitiveType(field.getDataType()); - } - - public static boolean isMapOfPrimitiveType(DataType fieldType) { - if (fieldType instanceof MapDataType) { - MapDataType mapType = (MapDataType)fieldType; + if (field.getDataType() instanceof MapDataType) { + MapDataType mapType = (MapDataType)field.getDataType(); return isPrimitiveType(mapType.getKeyType()) && isPrimitiveType(mapType.getValueType()); } else { @@ -86,17 +58,15 @@ public class ComplexAttributeFieldUtils { } } - private static boolean isSimpleStruct(DataType type, Optional<SDDocumentType> docType) { + private static boolean isStructWithPrimitiveStructFieldAttributes(DataType type, ImmutableSDField field) { if (type instanceof StructDataType && !(type.equals(PositionDataType.INSTANCE))) { - StructDataType structType = (StructDataType) type; - Collection<Field> structFields = getStructFields(structType, docType); - if (structFields.isEmpty()) { - return false; - } - for (Field innerField : structFields) { - if (!isPrimitiveType(innerField.getDataType())) { - return false; + for (ImmutableSDField structField : field.getStructFields()) { + Attribute attribute = structField.getAttributes().get(structField.getName()); + if (attribute != null) { + if (!isPrimitiveType(attribute)) { + return false; + } } } return true; @@ -105,20 +75,12 @@ public class ComplexAttributeFieldUtils { } } - private static Collection<Field> getStructFields(StructDataType structType, Optional<SDDocumentType> docType) { - // The struct data type might be unresolved at this point. If so we use the document type to resolve it. - if (docType.isPresent() && (structType instanceof TemporaryStructuredDataType)) { - SDDocumentType realStructType = docType.get().getOwnedType(structType.getName()); - if (structType != null) { - return realStructType.getDocumentType().getFields(); - } - return Collections.emptyList(); - } else { - return structType.getFields(); - } + private static boolean isPrimitiveType(Attribute attribute) { + return attribute.getCollectionType().equals(Attribute.CollectionType.SINGLE) && + isPrimitiveType(attribute.getDataType()); } - private static boolean isPrimitiveType(DataType dataType) { + public static boolean isPrimitiveType(DataType dataType) { return dataType.equals(DataType.BYTE) || dataType.equals(DataType.INT) || dataType.equals(DataType.LONG) || @@ -127,10 +89,10 @@ public class ComplexAttributeFieldUtils { dataType.equals(DataType.STRING); } - public static boolean isComplexFieldWithOnlyStructFieldAttributes(ImmutableSDField field, SDDocumentType docType) { - if (isArrayOfSimpleStruct(field, docType)) { + public static boolean isComplexFieldWithOnlyStructFieldAttributes(ImmutableSDField field) { + if (isArrayOfSimpleStruct(field)) { return hasOnlyStructFieldAttributes(field); - } else if (isMapOfSimpleStruct(field, docType)) { + } else if (isMapOfSimpleStruct(field)) { return hasSingleAttribute(field.getStructField("key")) && hasOnlyStructFieldAttributes(field.getStructField("value")); } else if (isMapOfPrimitiveType(field)) { diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImplicitSummaries.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImplicitSummaries.java index 2d2fc10d9c5..b51524b7e62 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImplicitSummaries.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImplicitSummaries.java @@ -83,7 +83,7 @@ public class ImplicitSummaries extends Processor { } } - if (addedSummaryField != null && isComplexFieldWithOnlyStructFieldAttributes(field, search.getDocument())) { + if (addedSummaryField != null && isComplexFieldWithOnlyStructFieldAttributes(field)) { addedSummaryField.setTransform(SummaryTransform.ATTRIBUTECOMBINER); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidator.java index 005c1dd8b2e..bc89d0dfcf5 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidator.java @@ -61,7 +61,7 @@ public class ComplexAttributeFieldsValidator extends Validator { } private static boolean isSupportedComplexField(ImmutableSDField field) { - return (ComplexAttributeFieldUtils.isSupportedComplexField(field.getDataType()) || + return (ComplexAttributeFieldUtils.isSupportedComplexField(field) || field.getDataType().equals(PositionDataType.INSTANCE) || field.getDataType().equals(DataType.getArray(PositionDataType.INSTANCE))); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/StructFieldAttributeChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/StructFieldAttributeChangeValidator.java index 520494f1697..4dfeb808e31 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/StructFieldAttributeChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/StructFieldAttributeChangeValidator.java @@ -10,6 +10,7 @@ import com.yahoo.document.StructDataType; import com.yahoo.documentmodel.NewDocumentType; import com.yahoo.searchdefinition.derived.AttributeFields; import com.yahoo.searchdefinition.document.Attribute; +import com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils; import com.yahoo.vespa.model.application.validation.change.VespaConfigChangeAction; import com.yahoo.vespa.model.application.validation.change.VespaRefeedAction; @@ -20,10 +21,6 @@ import java.util.List; import java.util.StringTokenizer; import java.util.stream.Collectors; -import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isArrayOfSimpleStruct; -import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isMapOfPrimitiveType; -import static com.yahoo.searchdefinition.document.ComplexAttributeFieldUtils.isMapOfSimpleStruct; - /** * Validates the changes between the current and next set of struct field attributes in a document database. @@ -66,7 +63,7 @@ public class StructFieldAttributeChangeValidator { private List<VespaConfigChangeAction> validateAddAttributeAspect(Context current, Context next, ValidationOverrides overrides, Instant now) { return next.structFieldAttributes.stream() - .filter(nextAttr -> current.hasFieldFor(nextAttr) && + .filter(nextAttr -> current.hasFieldForStructFieldAttribute(nextAttr) && !current.hasStructFieldAttribute(nextAttr)) .map(nextAttr -> VespaRefeedAction.of("field-type-change", overrides, @@ -94,23 +91,23 @@ public class StructFieldAttributeChangeValidator { .anyMatch(attr -> attr.getName().equals(structFieldAttribute.getName())); } - public boolean hasFieldFor(Attribute structFieldAttribute) { + public boolean hasFieldForStructFieldAttribute(Attribute structFieldAttribute) { StringTokenizer fieldNames = new StringTokenizer(structFieldAttribute.getName(), "."); if (!fieldNames.nextToken().equals(field.getName())) { return false; } - if (isArrayOfSimpleStruct(dataType())) { + if (isArrayOfStructType(dataType())) { StructDataType nestedType = (StructDataType)((ArrayDataType)dataType()).getNestedType(); - if (hasLastFieldInStructType(fieldNames, nestedType)) { + if (structTypeContainsLastFieldNameComponent(nestedType, fieldNames)) { return true; } - } else if (isMapOfSimpleStruct(dataType())) { + } else if (isMapOfStructType(dataType())) { MapDataType mapType = (MapDataType)dataType(); StructDataType valueType = (StructDataType)mapType.getValueType(); String subFieldName = fieldNames.nextToken(); if (subFieldName.equals("key") && !fieldNames.hasMoreTokens()) { return true; - } else if (subFieldName.equals("value") && hasLastFieldInStructType(fieldNames, valueType)) { + } else if (subFieldName.equals("value") && structTypeContainsLastFieldNameComponent(valueType, fieldNames)) { return true; } } else if (isMapOfPrimitiveType(dataType())) { @@ -123,10 +120,42 @@ public class StructFieldAttributeChangeValidator { return false; } - private static boolean hasLastFieldInStructType(StringTokenizer fieldNames, StructDataType structType) { - return structType.getField(fieldNames.nextToken()) != null && !fieldNames.hasMoreTokens(); + private static boolean isArrayOfStructType(DataType type) { + if (type instanceof ArrayDataType) { + ArrayDataType arrayType = (ArrayDataType)type; + return isStructType(arrayType.getNestedType()); + } else { + return false; + } + } + + private static boolean isMapOfStructType(DataType type) { + if (type instanceof MapDataType) { + MapDataType mapType = (MapDataType)type; + return ComplexAttributeFieldUtils.isPrimitiveType(mapType.getKeyType()) && + isStructType(mapType.getValueType()); + } else { + return false; + } } + public static boolean isMapOfPrimitiveType(DataType type) { + if (type instanceof MapDataType) { + MapDataType mapType = (MapDataType)type; + return ComplexAttributeFieldUtils.isPrimitiveType(mapType.getKeyType()) && + ComplexAttributeFieldUtils.isPrimitiveType(mapType.getValueType()); + } else { + return false; + } + } + + private static boolean isStructType(DataType type) { + return (type instanceof StructDataType); + } + + private static boolean structTypeContainsLastFieldNameComponent(StructDataType structType, StringTokenizer fieldNames) { + return structType.getField(fieldNames.nextToken()) != null && !fieldNames.hasMoreTokens(); + } } } 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 91a89c204c9..3f45b28b994 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 @@ -25,20 +25,16 @@ public class ComplexAttributeFieldUtilsTestCase { return field; } - public SDDocumentType docType() { - return search.getDocument(); - } - public boolean isSupportedComplexField() { - return ComplexAttributeFieldUtils.isSupportedComplexField(field(), docType()); + return ComplexAttributeFieldUtils.isSupportedComplexField(field()); } public boolean isArrayOfSimpleStruct() { - return ComplexAttributeFieldUtils.isArrayOfSimpleStruct(field(), docType()); + return ComplexAttributeFieldUtils.isArrayOfSimpleStruct(field()); } public boolean isMapOfSimpleStruct() { - return ComplexAttributeFieldUtils.isMapOfSimpleStruct(field(), docType()); + return ComplexAttributeFieldUtils.isMapOfSimpleStruct(field()); } public boolean isMapOfPrimitiveType() { @@ -46,7 +42,7 @@ public class ComplexAttributeFieldUtilsTestCase { } public boolean isComplexFieldWithOnlyStructFieldAttributes() { - return ComplexAttributeFieldUtils.isComplexFieldWithOnlyStructFieldAttributes(field(), docType()); + return ComplexAttributeFieldUtils.isComplexFieldWithOnlyStructFieldAttributes(field()); } } @@ -57,7 +53,7 @@ public class ComplexAttributeFieldUtilsTestCase { " document test {", " struct elem {", " field name type string {}", - " field weight type string {}", + " field weight type int {}", " }", sdFieldContent, " }", @@ -72,7 +68,7 @@ public class ComplexAttributeFieldUtilsTestCase { " document test {", " struct elem {", " field name type string {}", - " field weight type array<string> {}", + " field weights type array<int> {}", " }", sdFieldContent, " }", @@ -194,7 +190,7 @@ public class ComplexAttributeFieldUtilsTestCase { ComplexFixture f = new ComplexFixture("elem_array", joinLines("field elem_array type array<elem> {", " struct-field name { indexing: attribute }", - " struct-field weight { indexing: attribute }", + " struct-field weights { indexing: attribute }", "}")); assertFalse(f.isSupportedComplexField()); assertFalse(f.isArrayOfSimpleStruct()); @@ -207,7 +203,7 @@ public class ComplexAttributeFieldUtilsTestCase { joinLines("field elem_map type map<int, elem> {", " indexing: summary", " struct-field key { indexing: attribute }", - " struct-field value.weight { indexing: attribute }", + " struct-field value.weights { indexing: attribute }", "}")); assertFalse(f.isSupportedComplexField()); assertFalse(f.isArrayOfSimpleStruct()); @@ -217,4 +213,32 @@ public class ComplexAttributeFieldUtilsTestCase { } } + @Test + public void only_struct_field_attributes_are_considered_when_tagging_a_complex_field() throws ParseException { + { + ComplexFixture f = new ComplexFixture("elem_array", + joinLines("field elem_array type array<elem> {", + " struct-field name { indexing: attribute }", + "}")); + assertTrue(f.isSupportedComplexField()); + assertTrue(f.isArrayOfSimpleStruct()); + assertFalse(f.isMapOfSimpleStruct()); + assertFalse(f.isMapOfPrimitiveType()); + assertFalse(f.isComplexFieldWithOnlyStructFieldAttributes()); + } + { + ComplexFixture f = new ComplexFixture("elem_map", + joinLines("field elem_map type map<int, elem> {", + " indexing: summary", + " struct-field key { indexing: attribute }", + " struct-field value.name { indexing: attribute }", + "}")); + assertTrue(f.isSupportedComplexField()); + assertFalse(f.isArrayOfSimpleStruct()); + assertTrue(f.isMapOfSimpleStruct()); + assertFalse(f.isMapOfPrimitiveType()); + assertFalse(f.isComplexFieldWithOnlyStructFieldAttributes()); + } + } + } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java index 6483933385d..3ba3745f46e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java @@ -31,18 +31,42 @@ public class ComplexAttributeFieldsValidatorTestCase { exceptionRule.expect(IllegalArgumentException.class); exceptionRule.expectMessage("For cluster 'mycluster', search 'test': " + "The following complex fields do not support using struct field attributes: " + - "struct_array (struct_array.s1), struct_map (struct_map.key, struct_map.value.s1). " + + "struct_array (struct_array.f1), struct_map (struct_map.key, struct_map.value.f1). " + "Only supported for the following complex field types: array or map of struct with primitive types, map of primitive types"); createModelAndValidate(joinLines("search test {", " document test {", - " struct s { field s1 type array<int> {} }", + " struct s { field f1 type array<int> {} }", " field struct_array type array<s> {", - " struct-field s1 { indexing: attribute }", + " struct-field f1 { indexing: attribute }", " }", " field struct_map type map<string,s> {", " struct-field key { indexing: attribute }", - " struct-field value.s1 { indexing: attribute }", + " struct-field value.f1 { indexing: attribute }", + " }", + " }", + "}")); + } + + @Test + public void validation_passes_when_only_supported_struct_field_attributes_are_used() throws IOException, SAXException { + createModelAndValidate(joinLines("search test {", + " document test {", + " struct s1 {", + " field f1 type string {}", + " field f2 type int {}", + " }", + " struct s2 {", + " field f3 type string {}", + " field f4 type array<int> {}", + " field f5 type array<s1> {}", + " }", + " field struct_array type array<s2> {", + " struct-field f3 { indexing: attribute }", + " }", + " field struct_map type map<string,s2> {", + " struct-field key { indexing: attribute }", + " struct-field value.f3 { indexing: attribute }", " }", " }", "}")); |