From b379c30b870594defd4ef5631ac09b2897aefcd1 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Sat, 7 Oct 2023 09:50:15 +0200 Subject: Support inheriting multiple document summaries --- .../yahoo/schema/parser/ConvertParsedSchemas.java | 7 +-- .../yahoo/schema/parser/ParsedDocumentSummary.java | 4 +- .../schema/processing/ImplicitSummaryFields.java | 2 +- .../yahoo/vespa/documentmodel/DocumentSummary.java | 51 ++++++++++++---------- .../com/yahoo/vespa/documentmodel/FieldView.java | 7 +-- .../yahoo/vespa/documentmodel/SummaryField.java | 14 +++--- 6 files changed, 42 insertions(+), 43 deletions(-) (limited to 'config-model/src/main/java') 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 40ec84ec8bc..39a1d21965f 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 @@ -139,12 +139,7 @@ public class ConvertParsedSchemas { private void convertDocumentSummary(Schema schema, ParsedDocumentSummary parsed, TypeResolver typeContext) { var docsum = new DocumentSummary(parsed.name(), schema); - var inheritList = parsed.getInherited(); - if (inheritList.size() == 1) { - docsum.setInherited(inheritList.get(0)); - } else if (inheritList.size() != 0) { - throw new IllegalArgumentException("document-summary "+parsed.name()+" cannot inherit more than once"); - } + parsed.getInherited().forEach(inherited -> docsum.addInherited(inherited)); if (parsed.getFromDisk()) { docsum.setFromDisk(true); } diff --git a/config-model/src/main/java/com/yahoo/schema/parser/ParsedDocumentSummary.java b/config-model/src/main/java/com/yahoo/schema/parser/ParsedDocumentSummary.java index 7aaabaef865..d6648f609cd 100644 --- a/config-model/src/main/java/com/yahoo/schema/parser/ParsedDocumentSummary.java +++ b/config-model/src/main/java/com/yahoo/schema/parser/ParsedDocumentSummary.java @@ -10,8 +10,9 @@ import java.util.Map; * This class holds the extracted information after parsing a * "document-summary" block, using simple data structures as far as * possible. Do not put advanced logic here! + * * @author arnej27959 - **/ + */ class ParsedDocumentSummary extends ParsedBlock { private boolean omitSummaryFeatures; @@ -45,4 +46,5 @@ class ParsedDocumentSummary extends ParsedBlock { void inherit(String other) { inherited.add(other); } + } diff --git a/config-model/src/main/java/com/yahoo/schema/processing/ImplicitSummaryFields.java b/config-model/src/main/java/com/yahoo/schema/processing/ImplicitSummaryFields.java index b17efbfe8e8..adc7a88cc79 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/ImplicitSummaryFields.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/ImplicitSummaryFields.java @@ -23,7 +23,7 @@ public class ImplicitSummaryFields extends Processor { @Override public void process(boolean validate, boolean documentsOnly) { for (DocumentSummary docsum : schema.getSummariesInThis().values()) { - if (docsum.inherited().isPresent()) continue; // Implicit fields are added to inheriting summaries through their parent + if ( ! docsum.inherited().isEmpty()) continue; // Implicit fields are added to inheriting summaries through their parent addField(docsum, new SummaryField("rankfeatures", DataType.STRING, SummaryTransform.RANKFEATURES), validate); addField(docsum, new SummaryField("summaryfeatures", DataType.STRING, SummaryTransform.SUMMARYFEATURES), validate); } diff --git a/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java b/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java index 337f5e11329..359462c3715 100644 --- a/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java +++ b/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java @@ -2,9 +2,13 @@ package com.yahoo.vespa.documentmodel; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.schema.RankProfile; import com.yahoo.schema.Schema; +import com.yahoo.searchlib.rankingexpression.Reference; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -19,10 +23,9 @@ import java.util.logging.Level; */ public class DocumentSummary extends FieldView { - private int id; private boolean fromDisk = false; private boolean omitSummaryFeatures = false; - private Optional inherited = Optional.empty(); + private List inherited = new ArrayList<>(); private final Schema owner; @@ -32,8 +35,6 @@ public class DocumentSummary extends FieldView { this.owner = owner; } - public int id() { return id; } - public void setFromDisk(boolean fromDisk) { this.fromDisk = fromDisk; } /** Returns whether the user has noted explicitly that this summary accesses disk */ @@ -63,15 +64,23 @@ public class DocumentSummary extends FieldView { var field = (SummaryField)get(name); if (field != null) return field; if (inherited().isEmpty()) return null; - return inherited().get().getSummaryField(name); + for (var inheritedSummary : inherited()) { + var inheritedField = inheritedSummary.getSummaryField(name); + if (inheritedField != null) + return inheritedField; + } + return null; } public Map getSummaryFields() { - var fields = new LinkedHashMap(getFields().size()); - inherited().ifPresent(inherited -> fields.putAll(inherited.getSummaryFields())); + var allFields = new LinkedHashMap(getFields().size()); + for (var inheritedSummary : inherited()) { + if (inheritedSummary == null) continue; + allFields.putAll(inheritedSummary.getSummaryFields()); + } for (var field : getFields()) - fields.put(field.getName(), (SummaryField) field); - return fields; + allFields.put(field.getName(), (SummaryField) field); + return allFields; } /** @@ -99,14 +108,14 @@ public class DocumentSummary extends FieldView { } } - /** Sets the parent of this. Both summaries must be present in the same search definition */ - public void setInherited(String inherited) { - this.inherited = Optional.of(inherited); + /** Adds a parent of this. Both summaries must be present in the same schema, or a parent schema. */ + public void addInherited(String inherited) { + this.inherited.add(inherited); } /** Returns the parent of this, if any */ - public Optional inherited() { - return inherited.map(name -> owner.getSummary(name)); + public List inherited() { + return inherited.stream().map(name -> owner.getSummary(name)).toList(); } @Override @@ -115,16 +124,12 @@ public class DocumentSummary extends FieldView { } public void validate(DeployLogger logger) { - if (inherited.isPresent()) { - if ( ! owner.getSummaries().containsKey(inherited.get())) { - logger.log(Level.WARNING, - this + " inherits " + inherited.get() + " but this" + " is not present in " + owner); + for (var inheritedName : inherited) { + var inheritedSummary = owner.getSummary(inheritedName); + if (inheritedSummary == null) { + // TODO Vespa 9: Throw IllegalArgumentException instead logger.logApplicationPackage(Level.WARNING, - this + " inherits " + inherited.get() + " but this" + " is not present in " + owner); - // TODO: When safe, replace the above by - // throw new IllegalArgumentException(this + " inherits " + inherited.get() + " but this" + - // " is not present in " + owner); - // ... and update SummaryTestCase.testValidationOfInheritedSummary + this + " inherits '" + inheritedName + "' but this" + " is not present in " + owner); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/documentmodel/FieldView.java b/config-model/src/main/java/com/yahoo/vespa/documentmodel/FieldView.java index 73d699dda1b..34a7e786947 100644 --- a/config-model/src/main/java/com/yahoo/vespa/documentmodel/FieldView.java +++ b/config-model/src/main/java/com/yahoo/vespa/documentmodel/FieldView.java @@ -9,7 +9,7 @@ import java.util.LinkedHashMap; import java.util.Map; /** - * @author baldersheim + * @author baldersheim */ public class FieldView implements Serializable { @@ -30,8 +30,9 @@ public class FieldView implements Serializable { /** * This method will add a field to a view. All fields must come from the same document type. Not enforced here. - * @param field The field to add. - * @return Itself for chaining purposes. + * + * @param field the field to add. + * @return itself for chaining purposes. */ public FieldView add(Field field) { if (fields.containsKey(field.getName())) { 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 49cd36e4bc2..6f738cfe61f 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 @@ -7,6 +7,7 @@ import com.yahoo.schema.document.TypedKey; import java.io.Serializable; import java.util.*; +import java.util.stream.Collectors; import static com.yahoo.text.Lowercase.toLowerCase; @@ -223,23 +224,18 @@ public class SummaryField extends Field implements Cloneable, TypedKey { return true; } - private String getDestinationString() - { - StringBuilder destinationString = new StringBuilder("destinations("); - for (String destination : destinations) { - destinationString.append(destination).append(" "); - } - destinationString.append(")"); - return destinationString.toString(); + private String getDestinationString() { + return destinations.stream().map(destination -> "document summary '" + destination + "'").collect(Collectors.joining(", ")); } + @Override public String toString() { return "summary field '" + getName() + "'"; } /** Returns a string which aids locating this field in the source search definition */ public String toLocateString() { - return "'summary " + getName() + " type " + toLowerCase(getDataType().getName()) + "' in '" + getDestinationString() + "'"; + return "summary " + getName() + " type " + toLowerCase(getDataType().getName()) + " in " + getDestinationString(); } @Override -- cgit v1.2.3