diff options
author | Tor Egge <Tor.Egge@online.no> | 2023-10-24 16:48:37 +0200 |
---|---|---|
committer | Tor Egge <Tor.Egge@online.no> | 2023-10-24 16:48:37 +0200 |
commit | 6644150903efbe6b61b8144bc60fe68eae00eac5 (patch) | |
tree | 71f38454c0b877ed7f1d0c4be7453a325665bec2 | |
parent | 7f6f44d95e95706c0a31937c5dc89c20d09051ca (diff) |
Prepare for emitting warning if summary field type is specified.
Improve resolving of summary field type from source field type.
29 files changed, 303 insertions, 80 deletions
diff --git a/config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedFields.java b/config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedFields.java index ffb86f8ecf2..bc0e16abbe3 100644 --- a/config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedFields.java +++ b/config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedFields.java @@ -20,6 +20,7 @@ import com.yahoo.vespa.documentmodel.SummaryTransform; import java.util.Locale; import java.util.Map; +import java.util.logging.Level; /** * Helper for converting ParsedField etc to SDField with settings @@ -137,7 +138,7 @@ public class ConvertParsedFields { } // from grammar, things that can be inside struct-field block - private void convertCommonFieldSettings(SDField field, ParsedField parsed) { + private void convertCommonFieldSettings(Schema schema, SDField field, ParsedField parsed) { convertMatchSettings(field, parsed.matchSettings()); var indexing = parsed.getIndexing(); if (indexing.isPresent()) { @@ -152,7 +153,12 @@ public class ConvertParsedFields { for (var summaryField : parsed.getSummaryFields()) { var dataType = field.getDataType(); var otherType = summaryField.getType(); - if (otherType != null) { + if (otherType != null && summaryField.getHasExplicitType()) { + schema.getDeployLogger().log(Level.FINE, () -> "For " + schema.getName() + + ", field '" + field.getName() + + "', summary '" + summaryField.name() + + "': Specifying the type is deprecated, ignored and will be an error in Vespa 9." + + " Remove the type specification to silence this warning."); dataType = context.resolveType(otherType); } convertSummaryField(field, summaryField, dataType); @@ -161,7 +167,7 @@ public class ConvertParsedFields { field.addQueryCommand(command); } for (var structField : parsed.getStructFields()) { - convertStructField(field, structField); + convertStructField(schema, field, structField); } if (parsed.hasLiteral()) { field.getRanking().setLiteral(true); @@ -174,13 +180,13 @@ public class ConvertParsedFields { } } - private void convertStructField(SDField field, ParsedField parsed) { + private void convertStructField(Schema schema, SDField field, ParsedField parsed) { SDField structField = field.getStructField(parsed.name()); if (structField == null ) { throw new IllegalArgumentException("Struct field '" + parsed.name() + "' has not been defined in struct " + "for field '" + field.getName() + "'."); } - convertCommonFieldSettings(structField, parsed); + convertCommonFieldSettings(schema, structField, parsed); } private void convertExtraFieldSettings(SDField field, ParsedField parsed) { @@ -280,7 +286,7 @@ public class ConvertParsedFields { String name = parsed.name(); DataType dataType = context.resolveType(parsed.getType()); var field = new SDField(document, name, dataType); - convertCommonFieldSettings(field, parsed); + convertCommonFieldSettings(schema, field, parsed); convertExtraFieldSettings(field, parsed); document.addField(field); return field; @@ -290,7 +296,7 @@ public class ConvertParsedFields { String name = parsed.name(); DataType dataType = context.resolveType(parsed.getType()); var field = new SDField(schema.getDocument(), name, dataType); - convertCommonFieldSettings(field, parsed); + convertCommonFieldSettings(schema, field, parsed); convertExtraFieldSettings(field, parsed); schema.addExtraField(field); } @@ -307,7 +313,7 @@ public class ConvertParsedFields { for (var parsedField : parsed.getFields()) { var fieldType = context.resolveType(parsedField.getType()); var field = new SDField(document, parsedField.name(), fieldType); - convertCommonFieldSettings(field, parsedField); + convertCommonFieldSettings(schema, field, parsedField); structProxy.addField(field); if (parsedField.hasIdOverride()) { structProxy.setFieldId(field, parsedField.idOverride()); diff --git a/config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedSchemas.java b/config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedSchemas.java index 6ca7537205c..689353319c0 100644 --- a/config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedSchemas.java +++ b/config-model/src/main/java/com/yahoo/schema/parser/ConvertParsedSchemas.java @@ -11,11 +11,13 @@ import com.yahoo.config.model.deploy.TestProperties; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.document.DataType; import com.yahoo.document.DocumentTypeManager; +import com.yahoo.document.PositionDataType; import com.yahoo.schema.DefaultRankProfile; import com.yahoo.schema.DocumentOnlySchema; import com.yahoo.schema.RankProfileRegistry; import com.yahoo.schema.Schema; import com.yahoo.schema.UnrankedRankProfile; +import com.yahoo.schema.derived.SummaryClass; import com.yahoo.schema.document.SDDocumentType; import com.yahoo.schema.document.SDField; import com.yahoo.schema.document.TemporaryImportedField; @@ -25,7 +27,9 @@ import com.yahoo.vespa.documentmodel.SummaryField; import java.util.ArrayList; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; +import java.util.logging.Level; import java.util.Map; import java.util.Optional; @@ -137,7 +141,82 @@ public class ConvertParsedSchemas { schema.addDocument(document); } - private void convertDocumentSummary(Schema schema, ParsedDocumentSummary parsed, TypeResolver typeContext) { + /* + * Helper class for resolving data type for a document summary. Summary type is still + * used internally in config model when generating and processing indexing scripts. + * See DynamicSummaryTransformUtils class comment for more details. + */ + private class SummaryFieldTypeResolver { + + private final Schema schema; + private final Map<String, ParsedSummaryField> summaryFields = new LinkedHashMap<String, ParsedSummaryField>(); + private static final String zCurveSuffix = new String("_zcurve"); + + public SummaryFieldTypeResolver(Schema schema, List<ParsedDocumentSummary> parsed) { + this.schema = schema; + for (var docsum : parsed) { + for (var field : docsum.getSummaryFields()) { + summaryFields.put(field.name(), field); + } + } + } + + private boolean isPositionAttribute(Schema schema, String sourceFieldName) { + if (!sourceFieldName.endsWith(zCurveSuffix)) { + return false; + } + var name = sourceFieldName.substring(0, sourceFieldName.length() - zCurveSuffix.length()); + var field = schema.getField(name); + return (field.getDataType().equals(PositionDataType.INSTANCE)); + } + + + private String getSingleSource(ParsedSummaryField parsedField) { + if (parsedField.getSources().size() == 1) { + return parsedField.getSources().get(0); + } + return parsedField.name(); + } + + public DataType resolve(ParsedDocumentSummary docsum, ParsedSummaryField parsedField) { + var seen = new LinkedHashSet<String>(); + var origName = parsedField.name(); + while (true) { + if (seen.contains(parsedField.name())) { + throw new IllegalArgumentException("For schema '" + schema.getName() + + "' summary class '" + docsum.name() + + "' summary field '" + origName + + "': Source loop detected for summary field '" + parsedField.name() + "'"); + } + seen.add(parsedField.name()); + if (parsedField.getSources().size() >= 2) { + return DataType.STRING; // Flattening, streaming search + } + var source = getSingleSource(parsedField); + if (source.equals(SummaryClass.DOCUMENT_ID_FIELD)) { + return DataType.STRING; // Reserved source field name + } else if (isPositionAttribute(schema, source)) { + return DataType.LONG; // Extra field with suffix is added later for positions + } + var field = schema.getField(source); + if (field != null) { + return field.getDataType(); + } else if (schema.temporaryImportedFields().isPresent() && + schema.temporaryImportedFields().get().hasField(source)) { + return null; // Imported field, cannot resolve now + } else if (source.equals(parsedField.name()) || !summaryFields.containsKey(source)) { + throw new IllegalArgumentException("For schema '" + schema.getName() + + "', summary class '" + docsum.name() + + "', summary field '" + parsedField.name() + + "': there is no valid source '" + source + "'."); + } + parsedField = summaryFields.get(source); + } + } + } + + private void convertDocumentSummary(Schema schema, ParsedDocumentSummary parsed, TypeResolver typeContext, + SummaryFieldTypeResolver sfResolver) { var docsum = new DocumentSummary(parsed.name(), schema); parsed.getInherited().forEach(inherited -> docsum.addInherited(inherited)); if (parsed.getFromDisk()) { @@ -148,16 +227,17 @@ public class ConvertParsedSchemas { } for (var parsedField : parsed.getSummaryFields()) { var parsedType = parsedField.getType(); - DataType dataType = (parsedType != null) ? typeContext.resolveType(parsedType) : null; - var existingField = schema.getField(parsedField.name()); - if (existingField == null && parsedField.getSources().size() == 1) { - var sourceName = parsedField.getSources().get(0); - if (!sourceName.equals(parsedField.name())) { - existingField = schema.getField(sourceName); - } + if (parsedType != null) { + var log = schema.getDeployLogger(); + log.log(Level.FINE, () -> "For " + schema.getName() + + ", document-summary '" + parsed.name() + + "', summary field '" + parsedField.name() + + "': Specifying the type is deprecated, ignored and will be an error in Vespa 9." + + " Remove the type specification to silence this warning."); } - if (existingField != null) { - var existingType = existingField.getDataType(); + DataType dataType = (parsedType != null) ? typeContext.resolveType(parsedType) : null; + DataType existingType = sfResolver.resolve(parsed, parsedField); + if (existingType != null) { if (dataType == null) { dataType = existingType; } else if (!dataType.equals(existingType)) { @@ -167,10 +247,9 @@ public class ConvertParsedSchemas { } } } - if (dataType == null) { - throw new IllegalArgumentException("Missing data-type for summary field " + parsedField.name() + " in document-summary " + parsed.name()); - } - var summaryField = new SummaryField(parsedField.name(), dataType); + var summaryField = (dataType == null) ? + SummaryField.createWithUnresolvedType(parsedField.name()) : + new SummaryField(parsedField.name(), dataType); // XXX does not belong here: summaryField.setVsmCommand(SummaryField.VsmCommand.FLATTENSPACE); ConvertParsedFields.convertSummaryFieldSettings(summaryField, parsedField); @@ -212,6 +291,7 @@ public class ConvertParsedSchemas { } parsed.getRawAsBase64().ifPresent(value -> schema.enableRawAsBase64(value)); var typeContext = typeConverter.makeContext(parsed.getDocument()); + var sfResolver = new SummaryFieldTypeResolver(schema, parsed.getDocumentSummaries()); var fieldConverter = new ConvertParsedFields(typeContext, convertedStructs); convertDocument(schema, parsed.getDocument(), fieldConverter); for (var field : parsed.getFields()) { @@ -220,12 +300,12 @@ public class ConvertParsedSchemas { for (var index : parsed.getIndexes()) { fieldConverter.convertExtraIndex(schema, index); } - for (var docsum : parsed.getDocumentSummaries()) { - convertDocumentSummary(schema, docsum, typeContext); - } for (var importedField : parsed.getImportedFields()) { convertImportField(schema, importedField); } + for (var docsum : parsed.getDocumentSummaries()) { + convertDocumentSummary(schema, docsum, typeContext, sfResolver); + } for (var fieldSet : parsed.getFieldSets()) { convertFieldSet(schema, fieldSet); } diff --git a/config-model/src/main/java/com/yahoo/schema/parser/ParsedSummaryField.java b/config-model/src/main/java/com/yahoo/schema/parser/ParsedSummaryField.java index 8b732c358f5..8f9733d2595 100644 --- a/config-model/src/main/java/com/yahoo/schema/parser/ParsedSummaryField.java +++ b/config-model/src/main/java/com/yahoo/schema/parser/ParsedSummaryField.java @@ -19,6 +19,7 @@ class ParsedSummaryField extends ParsedBlock { private boolean isFull = false; private boolean isBold = false; private boolean isTokens = false; + private boolean hasExplicitType = false; private final List<String> sources = new ArrayList<>(); private final List<String> destinations = new ArrayList<>(); @@ -39,6 +40,7 @@ class ParsedSummaryField extends ParsedBlock { boolean getFull() { return isFull; } boolean getMatchedElementsOnly() { return isMEO; } boolean getTokens() { return isTokens; } + boolean getHasExplicitType() { return hasExplicitType; } void addDestination(String dst) { destinations.add(dst); } void addSource(String src) { sources.add(src); } @@ -47,6 +49,7 @@ class ParsedSummaryField extends ParsedBlock { void setFull() { this.isFull = true; } void setMatchedElementsOnly() { this.isMEO = true; } void setTokens() { this.isTokens = true; } + void setHasExplicitType() { this.hasExplicitType = true; } void setType(ParsedType value) { verifyThat(type == null, "Cannot change type from ", type, "to", value); this.type = value; diff --git a/config-model/src/main/java/com/yahoo/schema/processing/AddAttributeTransformToSummaryOfImportedFields.java b/config-model/src/main/java/com/yahoo/schema/processing/AddDataTypeAndTransformToSummaryOfImportedFields.java index 5b72381bfb1..762279e3871 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/AddAttributeTransformToSummaryOfImportedFields.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/AddDataTypeAndTransformToSummaryOfImportedFields.java @@ -2,6 +2,8 @@ package com.yahoo.schema.processing; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.document.DataType; +import com.yahoo.document.PositionDataType; import com.yahoo.schema.RankProfileRegistry; import com.yahoo.schema.Schema; import com.yahoo.schema.document.ImmutableImportedComplexSDField; @@ -13,17 +15,17 @@ import com.yahoo.vespa.model.container.search.QueryProfiles; import java.util.stream.Stream; /** - * Adds the attribute summary transform ({@link SummaryTransform#ATTRIBUTE} to all {@link SummaryField} having an imported + * Adds the data type and attribute summary transform ({@link SummaryTransform#ATTRIBUTE} to all {@link SummaryField} having an imported * field as source. * * @author bjorncs */ -public class AddAttributeTransformToSummaryOfImportedFields extends Processor { +public class AddDataTypeAndTransformToSummaryOfImportedFields extends Processor { - public AddAttributeTransformToSummaryOfImportedFields(Schema schema, - DeployLogger deployLogger, - RankProfileRegistry rankProfileRegistry, - QueryProfiles queryProfiles) { + public AddDataTypeAndTransformToSummaryOfImportedFields(Schema schema, + DeployLogger deployLogger, + RankProfileRegistry rankProfileRegistry, + QueryProfiles queryProfiles) { super(schema, deployLogger, rankProfileRegistry, queryProfiles); } @@ -39,19 +41,29 @@ public class AddAttributeTransformToSummaryOfImportedFields extends Processor { private void setTransform(ImmutableSDField field) { if (field instanceof ImmutableImportedComplexSDField) { - getSummaryFieldsForImportedField(field).forEach(AddAttributeTransformToSummaryOfImportedFields::setAttributeCombinerTransform); + getSummaryFieldsForImportedField(field).forEach(summaryField -> setAttributeCombinerTransform(field, summaryField)); } else { - getSummaryFieldsForImportedField(field).forEach(AddAttributeTransformToSummaryOfImportedFields::setAttributeTransform); + getSummaryFieldsForImportedField(field).forEach(summaryField -> setAttributeTransform(field, summaryField)); } } - private static void setAttributeTransform(SummaryField summaryField) { + private static void setAttributeTransform(ImmutableSDField field, SummaryField summaryField) { + if (summaryField.hasUnresolvedType()) { + if (field.getDataType().equals(DataType.LONG) && summaryField.getTransform().equals(SummaryTransform.GEOPOS)) { + summaryField.setResolvedDataType(PositionDataType.INSTANCE); + } else { + summaryField.setResolvedDataType(field.getDataType()); + } + } if (summaryField.getTransform() == SummaryTransform.NONE) { summaryField.setTransform(SummaryTransform.ATTRIBUTE); } } - private static void setAttributeCombinerTransform(SummaryField summaryField) { + private static void setAttributeCombinerTransform(ImmutableSDField field, SummaryField summaryField) { + if (summaryField.hasUnresolvedType()) { + summaryField.setResolvedDataType(field.getDataType()); + } if (summaryField.getTransform() == SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER) { // This field already has the correct transform. return; diff --git a/config-model/src/main/java/com/yahoo/schema/processing/Processing.java b/config-model/src/main/java/com/yahoo/schema/processing/Processing.java index 89e6a1533d0..d9b90ad661d 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/Processing.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/Processing.java @@ -49,6 +49,7 @@ public class Processing { DictionaryProcessor::new, WordMatch::new, ImportedFieldsResolver::new, + AddDataTypeAndTransformToSummaryOfImportedFields::new, ImplicitSummaries::new, ImplicitSummaryFields::new, AdjustPositionSummaryFields::new, @@ -57,7 +58,6 @@ public class Processing { SummaryNamesFieldCollisions::new, SummaryFieldsMustHaveValidSource::new, MatchedElementsOnlyResolver::new, - AddAttributeTransformToSummaryOfImportedFields::new, MakeDefaultSummaryTheSuperSet::new, Bolding::new, AttributeProperties::new, diff --git a/config-model/src/main/java/com/yahoo/schema/processing/ValidateFieldTypes.java b/config-model/src/main/java/com/yahoo/schema/processing/ValidateFieldTypes.java index dd2fd72b280..662f3fc970b 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/ValidateFieldTypes.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/ValidateFieldTypes.java @@ -49,7 +49,9 @@ public class ValidateFieldTypes extends Processor { final protected void verifySummaryFields(String searchName, Map<String, DataType> seenFields) { for (DocumentSummary summary : schema.getSummaries().values()) { for (SummaryField field : summary.getSummaryFields().values()) { - checkFieldType(searchName, "summary field", field.getName(), field.getDataType(), seenFields); + if (!field.hasUnresolvedType()) { + checkFieldType(searchName, "summary field", field.getName(), field.getDataType(), seenFields); + } } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java b/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java index 2a316c8af60..1c53ee36497 100644 --- a/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java +++ b/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java @@ -66,6 +66,7 @@ public class SummaryField extends Field implements Cloneable, TypedKey { /** True if this field was defined implicitly */ private boolean implicit = false; + private boolean unresolvedType = false; /** Creates a summary field with NONE as transform */ public SummaryField(String name, DataType type) { @@ -87,10 +88,24 @@ public class SummaryField extends Field implements Cloneable, TypedKey { this.transform=transform; } + public static SummaryField createWithUnresolvedType(String name) { + /* + * Data type is not available during conversion of + * parsed schema to schema. Use a placeholder data type and tag the summary + * field as having an unresolved type. + */ + var summaryField = new SummaryField(name, DataType.NONE); + summaryField.unresolvedType = true; + return summaryField; + } + + public void setImplicit(boolean implicit) { this.implicit=implicit; } public boolean isImplicit() { return implicit; } + public boolean hasUnresolvedType() { return unresolvedType; } + public void setTransform(SummaryTransform transform) { this.transform = transform; if (SummaryTransform.DYNAMICTEASER.equals(transform) || SummaryTransform.BOLDED.equals(transform)) { @@ -246,6 +261,7 @@ public class SummaryField extends Field implements Cloneable, TypedKey { clone.sources = new LinkedHashSet<>(this.sources); if (this.destinations != null) clone.destinations = new LinkedHashSet<>(destinations); + clone.unresolvedType = unresolvedType; return clone; } catch (CloneNotSupportedException e) { @@ -272,6 +288,14 @@ public class SummaryField extends Field implements Cloneable, TypedKey { return true; } + public void setResolvedDataType(DataType type) { + this.dataType = type; + if (!hasForcedId()) { + this.fieldId = calculateIdV7(null); + } + unresolvedType = false; + } + public VsmCommand getVsmCommand() { return vsmCommand; } diff --git a/config-model/src/main/javacc/SchemaParser.jj b/config-model/src/main/javacc/SchemaParser.jj index f186caacb5f..aef91e34239 100644 --- a/config-model/src/main/javacc/SchemaParser.jj +++ b/config-model/src/main/javacc/SchemaParser.jj @@ -1090,6 +1090,9 @@ void summaryInDocument(ParsedDocumentSummary docsum) : (<TYPE> type = dataType())? lbrace() { psf = new ParsedSummaryField(name, type); + if (type != null) { + psf.setHasExplicitType(); + } } (summaryItem(psf) (<NL>)*)* <RBRACE> { @@ -1140,13 +1143,17 @@ void summaryInFieldLong(ParsedField field) : { String name = field.name(); ParsedType type = field.getType(); + boolean explicitType = false; ParsedSummaryField psf; } { - ( [ name = identifier() [ <TYPE> type = dataType() ] ] + ( [ name = identifier() [ <TYPE> { type = dataType(); explicitType = true; } ] ] lbrace() { psf = field.summaryFieldFor(name, type); + if (explicitType) { + psf.setHasExplicitType(); + } } (summaryItem(psf) (<NL>)*)* <RBRACE> ) } diff --git a/config-model/src/test/derived/flickr/flickrphotos.sd b/config-model/src/test/derived/flickr/flickrphotos.sd index 514df5e76ed..df80ea3f96f 100755 --- a/config-model/src/test/derived/flickr/flickrphotos.sd +++ b/config-model/src/test/derived/flickr/flickrphotos.sd @@ -4,7 +4,7 @@ schema flickrphotos{ #Document summary to use for attribute-prefetching with many hits document-summary mapcluster { - summary distance type int {} + summary distance {} } document flickrphotos{ diff --git a/config-model/src/test/derived/imported_position_field_summary/child.sd b/config-model/src/test/derived/imported_position_field_summary/child.sd index 34aa6a19920..0efa2a8905c 100644 --- a/config-model/src/test/derived/imported_position_field_summary/child.sd +++ b/config-model/src/test/derived/imported_position_field_summary/child.sd @@ -8,6 +8,6 @@ schema child { import field parent_ref.pos as my_pos {} document-summary mysummary { - summary my_pos type position { } + summary my_pos { } } } diff --git a/config-model/src/test/derived/imported_struct_fields/child.sd b/config-model/src/test/derived/imported_struct_fields/child.sd index 63a4efee6f2..09eb08d3534 100644 --- a/config-model/src/test/derived/imported_struct_fields/child.sd +++ b/config-model/src/test/derived/imported_struct_fields/child.sd @@ -10,22 +10,22 @@ schema child { import field parent_ref.str_int_map as my_str_int_map {} document-summary mysummary { - summary documentid type string {} - summary my_elem_array type array<elem> {} - summary my_elem_map type map<string, elem> {} - summary my_str_int_map type map<string, int> {} + summary documentid {} + summary my_elem_array {} + summary my_elem_map {} + summary my_str_int_map {} } document-summary filtered { - summary elem_array_filtered type array<elem> { + summary elem_array_filtered { source: my_elem_array matched-elements-only } - summary elem_map_filtered type map<string, elem> { + summary elem_map_filtered { source: my_elem_map matched-elements-only } - summary str_int_map_filtered type map<string, int> { + summary str_int_map_filtered { source: my_str_int_map matched-elements-only } diff --git a/config-model/src/test/derived/importedfields/child.sd b/config-model/src/test/derived/importedfields/child.sd index 540c3b87751..d3be011f5fb 100644 --- a/config-model/src/test/derived/importedfields/child.sd +++ b/config-model/src/test/derived/importedfields/child.sd @@ -22,14 +22,14 @@ schema child { } document-summary mysummary { - summary a_ref type reference<parent_a> {} - summary b_ref_with_summary type reference<parent_b> {} - summary my_int_field type int {} - summary my_string_field type string {} - summary my_int_array_field type array<int> {} - summary my_int_wset_field type weightedset<int> {} - summary my_ancient_int_field type int {} - summary my_filtered_int_array_field type array<int> { + summary a_ref {} + summary b_ref_with_summary {} + summary my_int_field {} + summary my_string_field {} + summary my_int_array_field {} + summary my_int_wset_field {} + summary my_ancient_int_field {} + summary my_filtered_int_array_field { source: my_int_array_field matched-elements-only } diff --git a/config-model/src/test/derived/multiplesummaries/multiplesummaries.sd b/config-model/src/test/derived/multiplesummaries/multiplesummaries.sd index 221e888adc7..28eaec22784 100644 --- a/config-model/src/test/derived/multiplesummaries/multiplesummaries.sd +++ b/config-model/src/test/derived/multiplesummaries/multiplesummaries.sd @@ -170,7 +170,7 @@ schema multiplesummaries { summary c { } - summary loc_position type long { + summary loc_position { source: loc_pos_zcurve } @@ -185,7 +185,7 @@ schema multiplesummaries { source: a } - summary loc_pos_zcurve type long { + summary loc_pos_zcurve { } } diff --git a/config-model/src/test/derived/reference_from_several/bar.sd b/config-model/src/test/derived/reference_from_several/bar.sd index 02c912153ef..47cd9b117aa 100644 --- a/config-model/src/test/derived/reference_from_several/bar.sd +++ b/config-model/src/test/derived/reference_from_several/bar.sd @@ -11,6 +11,6 @@ schema bar { import field bpref.x as barsximp {} document-summary other { summary bartitle {} - summary barsximp type int {} + summary barsximp {} } } diff --git a/config-model/src/test/derived/reference_from_several/foo.sd b/config-model/src/test/derived/reference_from_several/foo.sd index 26ba07ea1fd..ead26bcbba3 100644 --- a/config-model/src/test/derived/reference_from_several/foo.sd +++ b/config-model/src/test/derived/reference_from_several/foo.sd @@ -10,7 +10,7 @@ schema foo { } import field myref.x as myx {} document-summary small { - summary myx type int {} + summary myx {} summary foo {} } } diff --git a/config-model/src/test/derived/streamingstruct/streamingstruct.sd b/config-model/src/test/derived/streamingstruct/streamingstruct.sd index 70c0a55b833..8606ec0a7f7 100644 --- a/config-model/src/test/derived/streamingstruct/streamingstruct.sd +++ b/config-model/src/test/derived/streamingstruct/streamingstruct.sd @@ -174,11 +174,11 @@ schema streamingstruct { } document-summary summ { - summary snippet type string { + summary snippet { dynamic source: a.f1, b.f2 } - summary snippet2 type string { + summary snippet2 { source: a.f1, b.f1, b.f2 } } diff --git a/config-model/src/test/derived/streamingstructdefault/streamingstructdefault.sd b/config-model/src/test/derived/streamingstructdefault/streamingstructdefault.sd index 7727875b17d..837678ed174 100644 --- a/config-model/src/test/derived/streamingstructdefault/streamingstructdefault.sd +++ b/config-model/src/test/derived/streamingstructdefault/streamingstructdefault.sd @@ -14,7 +14,7 @@ schema streamingstructdefault { } } document-summary default { - summary sum1 type string { + summary sum1 { source: f1, f2.s1 dynamic } diff --git a/config-model/src/test/derived/twostreamingstructs/streamingstruct.sd b/config-model/src/test/derived/twostreamingstructs/streamingstruct.sd index d0084703bca..8e70fdfc8d1 100644 --- a/config-model/src/test/derived/twostreamingstructs/streamingstruct.sd +++ b/config-model/src/test/derived/twostreamingstructs/streamingstruct.sd @@ -176,11 +176,11 @@ schema streamingstruct { } document-summary summ { - summary snippet type string { + summary snippet { dynamic source: a.f1, b.f2 } - summary snippet2 type string { + summary snippet2 { source: a.f1, b.f1, b.f2 } } diff --git a/config-model/src/test/examples/documentidinsummary.sd b/config-model/src/test/examples/documentidinsummary.sd index b85506d4eef..23bca092cad 100644 --- a/config-model/src/test/examples/documentidinsummary.sd +++ b/config-model/src/test/examples/documentidinsummary.sd @@ -4,6 +4,6 @@ search documentidinsummary { field a type string { } } document-summary withid { - summary w type string { source: documentid } + summary w { source: documentid } } } diff --git a/config-model/src/test/examples/invalidimplicitsummarysource.sd b/config-model/src/test/examples/invalidimplicitsummarysource.sd index 2a7310d848a..309d5f6e9a6 100644 --- a/config-model/src/test/examples/invalidimplicitsummarysource.sd +++ b/config-model/src/test/examples/invalidimplicitsummarysource.sd @@ -6,6 +6,6 @@ search invalidsummarysource { } } document-summary baz { - summary cox type string { } + summary cox { } } } diff --git a/config-model/src/test/examples/invalidselfreferringsummary.sd b/config-model/src/test/examples/invalidselfreferringsummary.sd index 3ee4d2c457e..6a2f278ad1d 100644 --- a/config-model/src/test/examples/invalidselfreferringsummary.sd +++ b/config-model/src/test/examples/invalidselfreferringsummary.sd @@ -4,6 +4,6 @@ search invalidselfreferringsummary { field a type string { } } document-summary withid { - summary w type string { source: w } + summary w { source: w } } } diff --git a/config-model/src/test/examples/invalidsummarysource.sd b/config-model/src/test/examples/invalidsummarysource.sd index 86d6918ac7b..abb8ac0c4ae 100644 --- a/config-model/src/test/examples/invalidsummarysource.sd +++ b/config-model/src/test/examples/invalidsummarysource.sd @@ -6,6 +6,6 @@ search invalidsummarysource { } } document-summary baz { - summary cox type string { source: nonexistingfield } + summary cox { source: nonexistingfield } } } diff --git a/config-model/src/test/examples/nextgen/implicitstructtypes.sd b/config-model/src/test/examples/nextgen/implicitstructtypes.sd index 82fd1bb0121..182805a0ae7 100644 --- a/config-model/src/test/examples/nextgen/implicitstructtypes.sd +++ b/config-model/src/test/examples/nextgen/implicitstructtypes.sd @@ -10,8 +10,8 @@ search implicitstructtypes { } } document-summary docsum { - summary docsum_str type string { - source: doc_str_sum + summary docsum_str { + source: doc_str } } } diff --git a/config-model/src/test/examples/nextgen/simple.sd b/config-model/src/test/examples/nextgen/simple.sd index 6d371328453..897371f2991 100644 --- a/config-model/src/test/examples/nextgen/simple.sd +++ b/config-model/src/test/examples/nextgen/simple.sd @@ -7,8 +7,8 @@ search simple { } } document-summary explicit_summary { - summary summary_field type string { - source: doc_field_summary + summary summary_field { + source: doc_field } } field extern_field type string { diff --git a/config-model/src/test/examples/nextgen/summaryfield.sd b/config-model/src/test/examples/nextgen/summaryfield.sd index 5a5747359a0..906d5441e55 100644 --- a/config-model/src/test/examples/nextgen/summaryfield.sd +++ b/config-model/src/test/examples/nextgen/summaryfield.sd @@ -10,8 +10,8 @@ search summaryfield { } } document-summary baz { - summary cox type string { - source: bar + summary cox { + source: foo } summary alltags { source: mytags diff --git a/config-model/src/test/java/com/yahoo/schema/SummaryTestCase.java b/config-model/src/test/java/com/yahoo/schema/SummaryTestCase.java index 3c83e79157a..e1dcfe70e91 100644 --- a/config-model/src/test/java/com/yahoo/schema/SummaryTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/SummaryTestCase.java @@ -6,6 +6,7 @@ import com.yahoo.vespa.documentmodel.DocumentSummary; import com.yahoo.vespa.model.test.utils.DeployLoggerStub; import com.yahoo.vespa.objects.FieldBase; import com.yahoo.yolean.Exceptions; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static com.yahoo.config.model.test.TestUtil.joinLines; @@ -259,12 +260,12 @@ public class SummaryTestCase { } } document-summary parent1 { - summary s1 type string { + summary s1 { source: field1 } } document-summary parent2 { - summary field1 type int { + summary field1 { source: field2 } } @@ -343,6 +344,94 @@ public class SummaryTestCase { DeployLoggerStub logger = new DeployLoggerStub(); ApplicationBuilder.createFromStrings(logger, parent, child); assertTrue(logger.entries.isEmpty()); + + } + private void testSummaryTypeInField(boolean explicit) throws ParseException { + String sd = joinLines("schema test {", + " document test {", + " field foo type string {", + " indexing: summary", + " summary bar " + (explicit ? "type string ": "") + "{ }", + " }", + " }", + "}"); + DeployLoggerStub logger = new DeployLoggerStub(); + ApplicationBuilder.createFromStrings(logger, sd); + if (explicit) { + assertEquals(1, logger.entries.size()); + assertEquals(Level.FINE, logger.entries.get(0).level); + assertEquals("For test, field 'foo', summary 'bar':" + + " Specifying the type is deprecated, ignored and will be an error in Vespa 9." + + " Remove the type specification to silence this warning.", logger.entries.get(0).message); + } else { + assertTrue(logger.entries.isEmpty()); + } + } + + @Test + void testSummaryInFieldWithoutTypeEmitsNoWarning() throws ParseException { + testSummaryTypeInField(false); + } + + @Test + void testSummaryInFieldWithTypeEmitsWarning() throws ParseException { + testSummaryTypeInField(true); + } + + private void testSummaryField(boolean explicit) throws ParseException { + String sd = joinLines("schema test {", + " document test {", + " field foo type string { indexing: summary }", + " }", + " document-summary bar {", + " summary foo " + (explicit ? "type string" : "") + "{ }", + " from-disk", + " }", + "}"); + DeployLoggerStub logger = new DeployLoggerStub(); + ApplicationBuilder.createFromStrings(logger, sd); + if (explicit) { + assertEquals(1, logger.entries.size()); + assertEquals(Level.FINE, logger.entries.get(0).level); + assertEquals("For test, document-summary 'bar', summary field 'foo':" + + " Specifying the type is deprecated, ignored and will be an error in Vespa 9." + + " Remove the type specification to silence this warning.", logger.entries.get(0).message); + } else { + assertTrue(logger.entries.isEmpty()); + } + } + + @Test + void testSummaryFieldWithoutTypeEmitsNoWarning() throws ParseException { + testSummaryField(false); + } + + @Test + void testSummaryFieldWithTypeEmitsWarning() throws ParseException { + testSummaryField(true); + } + + @Test + void testSummarySourceLoop() throws ParseException { + String sd = joinLines("schema test {", + " document test {", + " field foo type string { indexing: summary }", + " }", + " document-summary bar {", + " summary foo { source: foo2 }", + " summary foo2 { source: foo3 }", + " summary foo3 { source: foo2 }", + " from-disk", + " }", + "}"); + DeployLoggerStub logger = new DeployLoggerStub(); + try { + ApplicationBuilder.createFromStrings(logger, sd); + fail("expected exception"); + } catch (IllegalArgumentException e) { + assertEquals("For schema 'test' summary class 'bar' summary field 'foo'" + + ": Source loop detected for summary field 'foo2'", e.getMessage()); + } } private static class TestValue { diff --git a/config-model/src/test/java/com/yahoo/schema/derived/SummaryTestCase.java b/config-model/src/test/java/com/yahoo/schema/derived/SummaryTestCase.java index 2fb7955546c..1c03c66d17b 100644 --- a/config-model/src/test/java/com/yahoo/schema/derived/SummaryTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/derived/SummaryTestCase.java @@ -221,7 +221,7 @@ public class SummaryTestCase extends AbstractSchemaTestCase { void documentid_summary_field_has_corresponding_summary_transform() throws ParseException { var schema = buildSchema("field foo type string { indexing: summary }", joinLines("document-summary bar {", - " summary documentid type string {}", + " summary documentid {}", "}")); assertOverride(schema, "documentid", SummaryTransform.DOCUMENT_ID.getName(), "", "bar"); } diff --git a/config-model/src/test/java/com/yahoo/schema/processing/AddAttributeTransformToSummaryOfImportedFieldsTest.java b/config-model/src/test/java/com/yahoo/schema/processing/AddDataTypeAndTransformToSummaryOfImportedFieldsTest.java index 3b4a6c0a67b..f09a95b89a0 100644 --- a/config-model/src/test/java/com/yahoo/schema/processing/AddAttributeTransformToSummaryOfImportedFieldsTest.java +++ b/config-model/src/test/java/com/yahoo/schema/processing/AddDataTypeAndTransformToSummaryOfImportedFieldsTest.java @@ -26,7 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; /** * @author bjorncs */ -public class AddAttributeTransformToSummaryOfImportedFieldsTest { +public class AddDataTypeAndTransformToSummaryOfImportedFieldsTest { private static final String IMPORTED_FIELD_NAME = "imported_myfield"; private static final String DOCUMENT_NAME = "mydoc"; @@ -38,7 +38,7 @@ public class AddAttributeTransformToSummaryOfImportedFieldsTest { schema.setImportedFields(createSingleImportedField(IMPORTED_FIELD_NAME)); schema.addSummary(createDocumentSummary(IMPORTED_FIELD_NAME, schema)); - AddAttributeTransformToSummaryOfImportedFields processor = new AddAttributeTransformToSummaryOfImportedFields( + AddDataTypeAndTransformToSummaryOfImportedFields processor = new AddDataTypeAndTransformToSummaryOfImportedFields( schema, null, null, null); processor.process(true, false); SummaryField summaryField = schema.getSummaries().get(SUMMARY_NAME).getSummaryField(IMPORTED_FIELD_NAME); diff --git a/config-model/src/test/java/com/yahoo/schema/processing/SummaryDiskAccessValidatorTestCase.java b/config-model/src/test/java/com/yahoo/schema/processing/SummaryDiskAccessValidatorTestCase.java index a5145588136..bac29b52949 100644 --- a/config-model/src/test/java/com/yahoo/schema/processing/SummaryDiskAccessValidatorTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/processing/SummaryDiskAccessValidatorTestCase.java @@ -58,7 +58,7 @@ public class SummaryDiskAccessValidatorTestCase { " }", " import field ref.str_map as ref_str_map {}", " document-summary my_sum {", - " summary ref_str_map type map<string, string> { source: ref_str_map }", + " summary ref_str_map { source: ref_str_map }", " }", "}"); var logger = new TestableDeployLogger(); |