diff options
25 files changed, 465 insertions, 317 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/ApplicationBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/ApplicationBuilder.java index ac4ac649a28..98c6897dbb7 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/ApplicationBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/ApplicationBuilder.java @@ -223,9 +223,6 @@ public class ApplicationBuilder { private void addRankProfileFiles(Schema schema, String schemaPath) { if (applicationPackage == null || schemaPath == null) return; Path rankProfilePath = Path.fromString(schemaPath).append(schema.getName()); - System.out.println("Adding external rank profiles for " + schema + ": Have " + - applicationPackage.getFiles(rankProfilePath, ".profile").size() + - " files in " + rankProfilePath); for (NamedReader reader : applicationPackage.getFiles(rankProfilePath, ".profile")) { parseRankProfile(reader, schema); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DefaultRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/DefaultRankProfile.java index 4444636fbe8..94d2dfc3cc3 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/DefaultRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/DefaultRankProfile.java @@ -4,52 +4,51 @@ package com.yahoo.searchdefinition; import com.yahoo.searchdefinition.document.ImmutableSDField; import java.util.LinkedHashSet; +import java.util.List; import java.util.Set; /** * The rank profile containing default settings. This is derived from the fields * whenever this is accessed. * - * @author bratseth + * @author bratseth */ public class DefaultRankProfile extends RankProfile { /** * Creates a new rank profile * - * @param rankProfileRegistry The {@link com.yahoo.searchdefinition.RankProfileRegistry} to use for storing and looking up rank profiles. + * @param rankProfileRegistry the {@link com.yahoo.searchdefinition.RankProfileRegistry} + * to use for storing and looking up rank profiles */ public DefaultRankProfile(Schema schema, RankProfileRegistry rankProfileRegistry, RankingConstants rankingConstants) { super("default", schema, rankProfileRegistry, rankingConstants); } - /** - * Does nothing, the default rank profile can not inherit anything - */ - // TODO: Why not? If that's the case, then fail attempts at it - public void setInherited(String inheritedName) { - } + /** Does nothing, the default rank profile can not inherit anything. */ + // TODO: Why not? If that's the case, then fail attempts at i + @Override + public void inherit(String inheritedName) {} - /** Returns null, the default rank profile can not inherit anything */ - public String getInheritedName() { - return null; - } + /** Returns empty, the default rank profile can not inherit anything */ + @Override + public List<String> inheritedNames() { return List.of(); } - /** Returns the rank boost value of the given field */ - public RankSetting getRankSetting(String fieldOrIndex,RankSetting.Type type) { - RankSetting setting = super.getRankSetting(fieldOrIndex,type); + @Override + public RankSetting getRankSetting(String fieldOrIndex, RankSetting.Type type) { + RankSetting setting = super.getRankSetting(fieldOrIndex, type); if (setting != null) return setting; ImmutableSDField field = schema().getConcreteField(fieldOrIndex); if (field != null) { - setting = toRankSetting(field,type); + setting = toRankSetting(field, type); if (setting != null) return setting; } Index index = schema().getIndex(fieldOrIndex); if (index != null) { - setting = toRankSetting(index,type); + setting = toRankSetting(index, type); if (setting != null) return setting; } @@ -57,13 +56,13 @@ public class DefaultRankProfile extends RankProfile { return null; } - private RankSetting toRankSetting(ImmutableSDField field,RankSetting.Type type) { - if (type.equals(RankSetting.Type.WEIGHT) && field.getWeight()>0 && field.getWeight()!=100) - return new RankSetting(field.getName(),type,field.getWeight()); + private RankSetting toRankSetting(ImmutableSDField field, RankSetting.Type type) { + if (type.equals(RankSetting.Type.WEIGHT) && field.getWeight() > 0 && field.getWeight() != 100) + return new RankSetting(field.getName(), type, field.getWeight()); if (type.equals(RankSetting.Type.RANKTYPE)) - return new RankSetting(field.getName(),type,field.getRankType()); - if (type.equals(RankSetting.Type.LITERALBOOST) && field.getLiteralBoost()>0) - return new RankSetting(field.getName(),type,field.getLiteralBoost()); + return new RankSetting(field.getName(), type, field.getRankType()); + if (type.equals(RankSetting.Type.LITERALBOOST) && field.getLiteralBoost() > 0) + return new RankSetting(field.getName(), type, field.getLiteralBoost()); // Index level setting really if (type.equals(RankSetting.Type.PREFERBITVECTOR) && field.getRanking().isFilter()) { @@ -86,6 +85,7 @@ public class DefaultRankProfile extends RankProfile { * Returns the names of the fields which have a rank boost setting * explicitly in this profile or in fields */ + @Override public Set<RankSetting> rankSettings() { Set<RankSetting> settings = new LinkedHashSet<>(20); settings.addAll(this.rankSettings); @@ -96,7 +96,7 @@ public class DefaultRankProfile extends RankProfile { addSetting(field, RankSetting.Type.PREFERBITVECTOR, settings); } - // Foer settings that really pertains to indexes do the explicit indexes too + // For settings that really pertains to indexes do the explicit indexes too for (Index index : schema().getExplicitIndices()) { addSetting(index, RankSetting.Type.PREFERBITVECTOR, settings); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/OnnxModel.java b/config-model/src/main/java/com/yahoo/searchdefinition/OnnxModel.java index d40fe975721..4b849af9662 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/OnnxModel.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/OnnxModel.java @@ -18,8 +18,8 @@ import java.util.Optional; public class OnnxModel extends DistributableResource { private OnnxModelInfo modelInfo = null; - private Map<String, String> inputMap = new HashMap<>(); - private Map<String, String> outputMap = new HashMap<>(); + private final Map<String, String> inputMap = new HashMap<>(); + private final Map<String, String> outputMap = new HashMap<>(); private String statelessExecutionMode = null; private Integer statelessInterOpThreads = null; diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java index 1b6c4aa9459..53e9cfd5601 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java @@ -43,6 +43,7 @@ import java.util.Objects; import java.util.Optional; import java.util.OptionalDouble; import java.util.Set; +import java.util.function.Predicate; import java.util.function.Supplier; import java.util.logging.Level; import java.util.stream.Collectors; @@ -65,8 +66,9 @@ public class RankProfile implements Cloneable { private final ImmutableSchema schema; /** The name of the rank profile inherited by this */ - private String inheritedName = null; - private RankProfile inherited = null; + private final List<String> inheritedNames = new ArrayList<>(); + /** Stores the resolved inherited profiles, or null when not resolved. */ + private List<RankProfile> inherited; /** The match settings of this profile */ private MatchPhaseSettings matchPhaseSettings = null; @@ -96,10 +98,10 @@ public class RankProfile implements Cloneable { private double rankScoreDropLimit = -Double.MAX_VALUE; private Set<ReferenceNode> summaryFeatures; - private String inheritedSummaryFeatures; + private String inheritedSummaryFeaturesProfileName; private Set<ReferenceNode> matchFeatures; - private String inheritedMatchFeatures; + private String inheritedMatchFeaturesProfileName; private Set<ReferenceNode> rankFeatures; @@ -199,31 +201,28 @@ public class RankProfile implements Cloneable { } /** - * Sets the name of the rank profile this inherits. Both rank profiles must be present in the same search - * definition + * Adds a profile to those inherited by this. + * The profile must belong to this schema (directly or by inheritance). */ - public void setInherited(String inheritedName) { - this.inheritedName = inheritedName; - } - - /** Returns the name of the profile this one inherits, or null if none is inherited */ - public String getInheritedName() { return inheritedName; } - - /** Returns the inherited rank profile, or null if there is none */ - private RankProfile getInherited() { - if (inheritedName == null) return null; - if (inherited == null) { - inherited = resolveInherited(); - if (inherited == null) { - String msg = "rank-profile '" + name() + "' inherits '" + inheritedName + - "', but this is not found in " + - ((schema() != null) ? schema() : " global rank profiles"); - throw new IllegalArgumentException(msg); - } else { - List<String> children = new ArrayList<>(); - children.add(createFullyQualifiedName()); - verifyNoInheritanceCycle(children, inherited); - } + public void inherit(String inheritedName) { + inheritedNames.add(inheritedName); + } + + /** Returns the names of the profiles this inherits, if any. */ + public List<String> inheritedNames() { return inheritedNames; } + + /** Returns the rank profiles inherited by this. */ + private List<RankProfile> inherited() { + if (inheritedNames.isEmpty()) return List.of(); + if (inherited != null) return inherited; + + for (String inheritedName : inheritedNames) { + inherited = (schema() != null) ? resolveInheritedProfiles(schema) + : List.of(rankProfileRegistry.getGlobal(inheritedName)); + + List<String> children = new ArrayList<>(); + children.add(createFullyQualifiedName()); + inherited.forEach(profile -> verifyNoInheritanceCycle(children, profile)); } return inherited; } @@ -237,15 +236,26 @@ public class RankProfile implements Cloneable { private void verifyNoInheritanceCycle(List<String> children, RankProfile parent) { children.add(parent.createFullyQualifiedName()); String root = children.get(0); - if (root.equals(parent.createFullyQualifiedName())) { + if (root.equals(parent.createFullyQualifiedName())) throw new IllegalArgumentException("There is a cycle in the inheritance for rank-profile '" + root + "' = " + children); + for (RankProfile parentInherited : parent.inherited()) + verifyNoInheritanceCycle(children, parentInherited); + } + + private List<RankProfile> resolveInheritedProfiles(ImmutableSchema schema) { + List<RankProfile> inherited = new ArrayList<>(); + for (String inheritedName : inheritedNames) { + RankProfile inheritedProfile = resolveInheritedProfile(schema, inheritedName); + if (inheritedProfile == null) + throw new IllegalArgumentException("rank-profile '" + name() + "' inherits '" + inheritedName + + "', but this is not found in " + + ((schema() != null) ? schema() : " global rank profiles")); + inherited.add(inheritedProfile); } - if (parent.getInherited() != null) { - verifyNoInheritanceCycle(children, parent.getInherited()); - } + return inherited; } - private RankProfile resolveInherited(ImmutableSchema schema) { + private RankProfile resolveInheritedProfile(ImmutableSchema schema, String inheritedName) { SDDocumentType documentType = schema.getDocument(); if (documentType != null) { if (name.equals(inheritedName)) { @@ -260,25 +270,11 @@ public class RankProfile implements Cloneable { return rankProfileRegistry.get(schema.getName(), inheritedName); } - private RankProfile resolveInherited() { - if (inheritedName == null) return null; - return (schema() != null) - ? resolveInherited(schema) - : rankProfileRegistry.getGlobal(inheritedName); - } - - /** - * Returns whether this profile inherits (directly or indirectly) the given profile - * - * @param name the profile name to compare this to. - * @return whether or not this inherits from the named profile. - */ + /** Returns whether this profile inherits (directly or indirectly) the given profile name. */ public boolean inherits(String name) { - RankProfile parent = getInherited(); - while (parent != null) { - if (parent.name().equals(name)) - return true; - parent = parent.getInherited(); + for (RankProfile inheritedProfile : inherited()) { + if (inheritedProfile.name().equals(name)) return true; + if (inheritedProfile.inherits(name)) return true; } return false; } @@ -289,10 +285,20 @@ public class RankProfile implements Cloneable { } public MatchPhaseSettings getMatchPhaseSettings() { - MatchPhaseSettings settings = this.matchPhaseSettings; - if (settings != null) return settings; - if (getInherited() != null) return getInherited().getMatchPhaseSettings(); - return null; + if (matchPhaseSettings != null) return matchPhaseSettings; + return inheritedWith(p -> p.getMatchPhaseSettings() != null,"match phase settings") + .map(p -> p.getMatchPhaseSettings()).orElse(null); + } + + /** Returns the single profile having the property checked by the given filter, or empty if none */ + private Optional<RankProfile> inheritedWith(Predicate<RankProfile> property, String propertyDescription) { + List<RankProfile> matchingInherited = + inherited().stream().filter(profile -> property.test(profile)).collect(Collectors.toList()); + if (matchingInherited.isEmpty()) return Optional.empty(); + if (matchingInherited.size() == 1) return Optional.of(matchingInherited.get(0)); + throw new IllegalArgumentException("Only one of the profiles inherited by " + this + " can contain " + + propertyDescription + ", but it is present in all of " + matchingInherited); + } public void addRankSetting(RankSetting rankSetting) { @@ -306,15 +312,14 @@ public class RankProfile implements Cloneable { /** * Returns the a rank setting of a field, or null if there is no such rank setting in this profile * - * @param field the field whose settings to return. - * @param type the type that the field is required to be. - * @return the rank setting found, or null. + * @param field the field whose settings to return + * @param type the type that the field is required to be + * @return the rank setting found, or null */ RankSetting getDeclaredRankSetting(String field, RankSetting.Type type) { for (Iterator<RankSetting> i = declaredRankSettingIterator(); i.hasNext(); ) { RankSetting setting = i.next(); - if (setting.getFieldName().equals(field) && - setting.getType().equals(type)) { + if (setting.getFieldName().equals(field) && setting.getType() == type) { return setting; } } @@ -333,9 +338,8 @@ public class RankProfile implements Cloneable { RankSetting rankSetting = getDeclaredRankSetting(field, type); if (rankSetting != null) return rankSetting; - if (getInherited() != null) return getInherited().getRankSetting(field, type); - - return null; + return inheritedWith(p -> p.getRankSetting(field, type) != null, "rank setting " + type + " on " + field) + .map(p -> p.getRankSetting(field, type)).orElse(null); } /** @@ -361,12 +365,20 @@ public class RankProfile implements Cloneable { * Changes to the returned set will not be reflected in this rank profile. */ public Set<RankSetting> rankSettings() { - Set<RankSetting> allSettings = new LinkedHashSet<>(rankSettings); - RankProfile parent = getInherited(); - if (parent != null) - allSettings.addAll(parent.rankSettings()); + Set<RankSetting> settings = new LinkedHashSet<>(); + for (RankProfile inheritedProfile : inherited()) { + for (RankSetting setting : inheritedProfile.rankSettings()) { + if (settings.contains(setting)) + throw new IllegalArgumentException(setting + " is present in " + inheritedProfile + " inherited by " + + this + ", but is also present in another profile inherited by it"); + settings.add(setting); + } + } - return allSettings; + // TODO: Here we do things in the wrong order to not break tests. Reverse this. + Set<RankSetting> finalSettings = new LinkedHashSet<>(rankSettings); + finalSettings.addAll(settings); + return finalSettings; } public void addConstant(String name, Value value) { @@ -385,14 +397,20 @@ public class RankProfile implements Cloneable { /** Returns an unmodifiable view of the constants available in this */ public Map<String, Value> getConstants() { - if (constants.isEmpty()) - return getInherited() != null ? getInherited().getConstants() : Collections.emptyMap(); - if (getInherited() == null || getInherited().getConstants().isEmpty()) - return Collections.unmodifiableMap(constants); - - Map<String, Value> combinedConstants = new HashMap<>(getInherited().getConstants()); - combinedConstants.putAll(constants); - return combinedConstants; + if (inherited().isEmpty()) return new HashMap<>(constants); + + Map<String, Value> allConstants = new HashMap<>(); + for (var inheritedProfile : inherited()) { + for (var constant : inheritedProfile.getConstants().entrySet()) { + if (allConstants.containsKey(constant.getKey())) + throw new IllegalArgumentException("Constant '" + constant.getKey() + "' is present in " + + inheritedProfile + " inherited by " + + this + ", but is also present in another profile inherited by it"); + allConstants.put(constant.getKey(), constant.getValue()); + } + } + allConstants.putAll(constants); + return allConstants; } public void addAttributeType(String attributeName, String attributeType) { @@ -423,9 +441,8 @@ public class RankProfile implements Cloneable { public RankingExpressionFunction getFirstPhase() { if (firstPhaseRanking != null) return firstPhaseRanking; - RankProfile inherited = getInherited(); - if (inherited != null) return inherited.getFirstPhase(); - return null; + return inheritedWith(p -> p.getFirstPhase() != null, "first-phase expression") + .map(p -> p.getFirstPhase()).orElse(null); } void setFirstPhaseRanking(RankingExpression rankingExpression) { @@ -452,9 +469,8 @@ public class RankProfile implements Cloneable { public RankingExpressionFunction getSecondPhase() { if (secondPhaseRanking != null) return secondPhaseRanking; - RankProfile inherited = getInherited(); - if (inherited != null) return inherited.getSecondPhase(); - return null; + return inheritedWith(p -> p.getSecondPhase() != null, "second-phase expression") + .map(p -> p.getSecondPhase()).orElse(null); } public void setSecondPhaseRanking(String expression) { @@ -466,75 +482,80 @@ public class RankProfile implements Cloneable { } } - /** Returns a read-only view of the summary features to use in this profile. This is never null */ - public Set<ReferenceNode> getSummaryFeatures() { - if (inheritedSummaryFeatures != null && summaryFeatures != null) { - Set<ReferenceNode> combined = new HashSet<>(); - combined.addAll(getInherited().getSummaryFeatures()); - combined.addAll(summaryFeatures); - return Collections.unmodifiableSet(combined); - } - if (summaryFeatures != null) return Collections.unmodifiableSet(summaryFeatures); - if (getInherited() != null) return getInherited().getSummaryFeatures(); - return Set.of(); - } - - private void addSummaryFeature(ReferenceNode feature) { - if (summaryFeatures == null) - summaryFeatures = new LinkedHashSet<>(); - summaryFeatures.add(feature); - } - - /** Adds the content of the given feature list to the internal list of summary features. */ - public void addSummaryFeatures(FeatureList features) { - for (ReferenceNode feature : features) { - addSummaryFeature(feature); - } - } + // TODO: Below we have duplicate methods for summary and match features: Encapsulate this in a single parametrized + // class instead (and probably make rank features work the same). /** * Sets the name this should inherit the summary features of. - * Without setting this, this will either have the summary features of the parent, + * Without setting this, this will either have the summary features of the single parent setting them, * or if summary features are set in this, only have the summary features in this. * With this set the resulting summary features of this will be the superset of those defined in this and * the final (with inheritance included) summary features of the given parent. - * The profile must be the profile which is directly inherited by this. - * + * The profile must be one which is directly inherited by this. */ public void setInheritedSummaryFeatures(String parentProfile) { - if ( ! parentProfile.equals(inheritedName)) - throw new IllegalArgumentException("This can only inherit the summary features of its parent, '" + - inheritedName + ", but attempting to inherit '" + parentProfile); - this.inheritedSummaryFeatures = parentProfile; + if ( ! inheritedNames().contains(parentProfile)) + throw new IllegalArgumentException("This can only inherit the summary features of a directly inherited profile, '" + + ", but attempting to inherit '" + parentProfile); + this.inheritedSummaryFeaturesProfileName = parentProfile; } /** * Sets the name of a profile this should inherit the match features of. - * Without setting this, this will either have the match features of the parent, + * Without setting this, this will either have the match features of the single parent setting them, * or if match features are set in this, only have the match features in this. * With this set the resulting match features of this will be the superset of those defined in this and * the final (with inheritance included) match features of the given parent. - * The profile must be the profile which is directly inherited by this. + * The profile must be one which which is directly inherited by this. * */ public void setInheritedMatchFeatures(String parentProfile) { - if ( ! parentProfile.equals(inheritedName)) - throw new IllegalArgumentException("This rank profile ("+name+") can only inherit the match features of its parent, '" + - inheritedName + ", but attemtping to inherit '" + parentProfile); - this.inheritedMatchFeatures = parentProfile; + if ( ! inheritedNames().contains(parentProfile)) + throw new IllegalArgumentException("This can only inherit the match features of a directly inherited profile, '" + + ", but attempting to inherit '" + parentProfile); + this.inheritedMatchFeaturesProfileName = parentProfile; + } + + /** Returns a read-only view of the summary features to use in this profile. This is never null */ + public Set<ReferenceNode> getSummaryFeatures() { + if (inheritedSummaryFeaturesProfileName != null && summaryFeatures != null) { + Set<ReferenceNode> combined = new HashSet<>(); + RankProfile inherited = inherited().stream() + .filter(p -> p.name().equals(inheritedSummaryFeaturesProfileName)) + .findAny() + .orElseThrow(); + combined.addAll(inherited.getSummaryFeatures()); + combined.addAll(summaryFeatures); + return Collections.unmodifiableSet(combined); + } + if (summaryFeatures != null) return Collections.unmodifiableSet(summaryFeatures); + return inheritedWith(p -> ! p.getSummaryFeatures().isEmpty(), "summary features") + .map(p -> p.getSummaryFeatures()) + .orElse(Set.of()); } /** Returns a read-only view of the match features to use in this profile. This is never null */ public Set<ReferenceNode> getMatchFeatures() { - if (inheritedMatchFeatures != null && matchFeatures != null) { + if (inheritedMatchFeaturesProfileName != null && matchFeatures != null) { Set<ReferenceNode> combined = new HashSet<>(); - combined.addAll(getInherited().getMatchFeatures()); + RankProfile inherited = inherited().stream() + .filter(p -> p.name().equals(inheritedMatchFeaturesProfileName)) + .findAny() + .orElseThrow(); + combined.addAll(inherited.getMatchFeatures()); combined.addAll(matchFeatures); return Collections.unmodifiableSet(combined); } if (matchFeatures != null) return Collections.unmodifiableSet(matchFeatures); - if (getInherited() != null) return getInherited().getMatchFeatures(); - return Set.of(); + return inheritedWith(p -> ! p.getMatchFeatures().isEmpty(), "match features") + .map(p -> p.getMatchFeatures()) + .orElse(Set.of()); + } + + private void addSummaryFeature(ReferenceNode feature) { + if (summaryFeatures == null) + summaryFeatures = new LinkedHashSet<>(); + summaryFeatures.add(feature); } private void addMatchFeature(ReferenceNode feature) { @@ -543,6 +564,13 @@ public class RankProfile implements Cloneable { matchFeatures.add(feature); } + /** Adds the content of the given feature list to the internal list of summary features. */ + public void addSummaryFeatures(FeatureList features) { + for (ReferenceNode feature : features) { + addSummaryFeature(feature); + } + } + /** Adds the content of the given feature list to the internal list of match features. */ public void addMatchFeatures(FeatureList features) { for (ReferenceNode feature : features) { @@ -553,8 +581,8 @@ public class RankProfile implements Cloneable { /** Returns a read-only view of the rank features to use in this profile. This is never null */ public Set<ReferenceNode> getRankFeatures() { if (rankFeatures != null) return Collections.unmodifiableSet(rankFeatures); - if (getInherited() != null) return getInherited().getRankFeatures(); - return Collections.emptySet(); + return inheritedWith(p -> ! p.getRankFeatures().isEmpty(), "summary-features") + .map(p -> p.getRankFeatures()).orElse(Set.of()); } private void addRankFeature(ReferenceNode feature) { @@ -585,12 +613,15 @@ public class RankProfile implements Cloneable { /** Returns a read only map view of the rank properties to use in this profile. This is never null. */ public Map<String, List<RankProperty>> getRankPropertyMap() { - if (rankProperties.size() == 0 && getInherited() == null) return Collections.emptyMap(); - if (rankProperties.size() == 0) return getInherited().getRankPropertyMap(); - if (getInherited() == null) return Collections.unmodifiableMap(rankProperties); + if (rankProperties.size() == 0 && inherited().isEmpty()) return Map.of(); + if (inherited().isEmpty()) return Collections.unmodifiableMap(rankProperties); + + var inheritedProperties = inheritedWith(p -> ! p.getRankPropertyMap().isEmpty(), "rank-properties") + .map(p -> p.getRankPropertyMap()).orElse(Map.of()); + if (rankProperties.isEmpty()) return inheritedProperties; // Neither is null - Map<String, List<RankProperty>> combined = new LinkedHashMap<>(getInherited().getRankPropertyMap()); + Map<String, List<RankProperty>> combined = new LinkedHashMap<>(inheritedProperties); combined.putAll(rankProperties); // Don't combine values across inherited properties return Collections.unmodifiableMap(combined); } @@ -604,57 +635,44 @@ public class RankProfile implements Cloneable { rankProperties.computeIfAbsent(rankProperty.getName(), (String key) -> new ArrayList<>(1)).add(rankProperty); } - @Override - public String toString() { - return "rank profile '" + name() + "'"; - } + public void setRerankCount(int rerankCount) { this.rerankCount = rerankCount; } public int getRerankCount() { - return (rerankCount < 0 && (getInherited() != null)) - ? getInherited().getRerankCount() - : rerankCount; + if (rerankCount >= 0) return rerankCount; + return inheritedWith(p -> p.getRerankCount() >= 0, "rerank-count") + .map(p -> p.getRerankCount()).orElse(-1); } + public void setNumThreadsPerSearch(int numThreads) { this.numThreadsPerSearch = numThreads; } + public int getNumThreadsPerSearch() { - return (numThreadsPerSearch < 0 && (getInherited() != null)) - ? getInherited().getNumThreadsPerSearch() - : numThreadsPerSearch; + if (numThreadsPerSearch >= 0) return numThreadsPerSearch; + return inheritedWith(p -> p.getNumThreadsPerSearch() >= 0, "num-threads-per-search") + .map(p -> p.getNumThreadsPerSearch()).orElse(-1); } - public void setNumThreadsPerSearch(int numThreads) { - this.numThreadsPerSearch = numThreads; - } + public void setMinHitsPerThread(int minHits) { this.minHitsPerThread = minHits; } public int getMinHitsPerThread() { - return (minHitsPerThread < 0 && (getInherited() != null)) - ? getInherited().getMinHitsPerThread() - : minHitsPerThread; - } - - public void setMinHitsPerThread(int minHits) { - this.minHitsPerThread = minHits; + if (minHitsPerThread >= 0) return minHitsPerThread; + return inheritedWith(p -> p.getMinHitsPerThread() >= 0, "min-hits-per-search") + .map(p -> p.getMinHitsPerThread()).orElse(-1); } - public void setNumSearchPartitions(int numSearchPartitions) { - this.numSearchPartitions = numSearchPartitions; - } + public void setNumSearchPartitions(int numSearchPartitions) { this.numSearchPartitions = numSearchPartitions; } public int getNumSearchPartitions() { - return (numSearchPartitions < 0 && (getInherited() != null)) - ? getInherited().getNumSearchPartitions() - : numSearchPartitions; + if (numSearchPartitions >= 0) return numSearchPartitions; + return inheritedWith(p -> p.getNumSearchPartitions() >= 0, "num-search-partitions") + .map(p -> p.getNumSearchPartitions()).orElse(-1); } - public OptionalDouble getTermwiseLimit() { - return ((termwiseLimit == null) && (getInherited() != null)) - ? getInherited().getTermwiseLimit() - : (termwiseLimit != null) ? OptionalDouble.of(termwiseLimit) : OptionalDouble.empty(); - } public void setTermwiseLimit(double termwiseLimit) { this.termwiseLimit = termwiseLimit; } - /** Sets the rerank count. Set to -1 to use inherited */ - public void setRerankCount(int rerankCount) { - this.rerankCount = rerankCount; + public OptionalDouble getTermwiseLimit() { + if (termwiseLimit != null) return OptionalDouble.of(termwiseLimit); + return inheritedWith(p -> p.getTermwiseLimit().isPresent(), "termwise-limit") + .map(p -> p.getTermwiseLimit()).orElse(OptionalDouble.empty()); } /** Whether we should ignore the default rank features. Set to null to use inherited */ @@ -664,10 +682,26 @@ public class RankProfile implements Cloneable { public boolean getIgnoreDefaultRankFeatures() { if (ignoreDefaultRankFeatures != null) return ignoreDefaultRankFeatures; - return (getInherited() != null) && getInherited().getIgnoreDefaultRankFeatures(); + return inheritedWith(p -> p.ignoreDefaultRankFeatures != null, "ignore-default-rank-features") + .map(p -> p.getIgnoreDefaultRankFeatures()).orElse(false); + } + + public void setKeepRankCount(int rerankArraySize) { this.keepRankCount = rerankArraySize; } + + public int getKeepRankCount() { + if (keepRankCount >= 0) return keepRankCount; + return inheritedWith(p -> p.getKeepRankCount() >= 0, "keep-rank-count") + .map(p -> p.getKeepRankCount()).orElse(-1); + } + + public void setRankScoreDropLimit(double rankScoreDropLimit) { this.rankScoreDropLimit = rankScoreDropLimit; } + + public double getRankScoreDropLimit() { + if (rankScoreDropLimit > -Double.MAX_VALUE) return rankScoreDropLimit; + return inheritedWith(p -> p.getRankScoreDropLimit() > -Double.MAX_VALUE, "rank.score-drop-limit") + .map(p -> p.getRankScoreDropLimit()).orElse(rankScoreDropLimit); } - /** Adds a function */ public void addFunction(String name, List<String> arguments, String expression, boolean inline) { try { addFunction(parseRankingExpression(name, arguments, expression), inline); @@ -729,10 +763,11 @@ public class RankProfile implements Cloneable { public RankingExpressionFunction findFunction(String name) { RankingExpressionFunction function = functions.get(name); - return ((function == null) && (getInherited() != null)) - ? getInherited().findFunction(name) - : function; + if (function != null) return function; + return inheritedWith(p -> p.findFunction(name) != null, "function '" + name + "'") + .map(p -> p.findFunction(name)).orElse(null); } + /** Returns an unmodifiable snapshot of the functions in this */ public Map<String, RankingExpressionFunction> getFunctions() { updateCachedFunctions(); @@ -749,59 +784,42 @@ public class RankProfile implements Cloneable { } private Map<String, RankingExpressionFunction> gatherAllFunctions() { - if (functions.isEmpty() && getInherited() == null) return Collections.emptyMap(); - if (functions.isEmpty()) return getInherited().getFunctions(); - if (getInherited() == null) return Collections.unmodifiableMap(new LinkedHashMap<>(functions)); - - // Neither is null - Map<String, RankingExpressionFunction> allFunctions = new LinkedHashMap<>(getInherited().getFunctions()); + if (functions.isEmpty() && inherited().isEmpty()) return Map.of(); + if (inherited().isEmpty()) return Collections.unmodifiableMap(new LinkedHashMap<>(functions)); + + // Combine + Map<String, RankingExpressionFunction> allFunctions = new LinkedHashMap<>(); + for (var inheritedProfile : inherited()) { + for (var function : inheritedProfile.getFunctions().entrySet()) { + if (allFunctions.containsKey(function.getKey())) + throw new IllegalArgumentException(this + " inherits " + inheritedProfile + " which contains " + + function.getValue() + ", but this function is already " + + "defined in another profile this inherits"); + allFunctions.put(function.getKey(), function.getValue()); + } + } allFunctions.putAll(functions); return Collections.unmodifiableMap(allFunctions); } private boolean needToUpdateFunctionCache() { - if (getInherited() != null) - return (allFunctionsCached == null) || getInherited().needToUpdateFunctionCache(); + if (inherited().stream().anyMatch(profile -> profile.needToUpdateFunctionCache())) return true; return allFunctionsCached == null; } - public int getKeepRankCount() { - if (keepRankCount >= 0) return keepRankCount; - if (getInherited() != null) return getInherited().getKeepRankCount(); - return -1; - } - - public void setKeepRankCount(int rerankArraySize) { - this.keepRankCount = rerankArraySize; - } - - public double getRankScoreDropLimit() { - if (rankScoreDropLimit >- Double.MAX_VALUE) return rankScoreDropLimit; - if (getInherited() != null) return getInherited().getRankScoreDropLimit(); - return rankScoreDropLimit; - } + public Set<String> filterFields() { return filterFields; } - public void setRankScoreDropLimit(double rankScoreDropLimit) { - this.rankScoreDropLimit = rankScoreDropLimit; - } + /** Returns all filter fields in this profile and any profile it inherits. */ + public Set<String> allFilterFields() { + Set<String> inheritedFilterFields = + inheritedWith(p -> ! p.allFilterFields().isEmpty(), "filter fields") + .map(p -> p.allFilterFields()).orElse(Set.of()); - public Set<String> filterFields() { - return filterFields; - } + if (inheritedFilterFields.isEmpty()) return Collections.unmodifiableSet(filterFields); - /** - * Returns all filter fields in this profile and any profile it inherits. - * - * @return the set of all filter fields - */ - public Set<String> allFilterFields() { - RankProfile parent = getInherited(); - Set<String> retval = new LinkedHashSet<>(); - if (parent != null) { - retval.addAll(parent.allFilterFields()); - } - retval.addAll(filterFields()); - return retval; + Set<String> combined = new LinkedHashSet<>(inheritedFilterFields); + combined.addAll(filterFields()); + return combined; } private ExpressionFunction parseRankingExpression(String name, List<String> arguments, String expression) throws ParseException { @@ -892,7 +910,7 @@ public class RankProfile implements Cloneable { secondPhaseRanking = compile(this.getSecondPhase(), queryProfiles, featureTypes, importedModels, getConstants(), inlineFunctions, expressionTransforms); // Function compiling second pass: compile all functions and insert previously compiled inline functions - // TODO This merges all functions from inherited profiles too and erases inheritance information. Not good. + // TODO: This merges all functions from inherited profiles too and erases inheritance information. Not good. functions = compileFunctions(this::getFunctions, queryProfiles, featureTypes, importedModels, inlineFunctions, expressionTransforms); allFunctionsCached = null; } @@ -947,6 +965,7 @@ public class RankProfile implements Cloneable { Map<String, RankingExpressionFunction> inlineFunctions, ExpressionTransforms expressionTransforms) { if (function == null) return null; + RankProfileTransformContext context = new RankProfileTransformContext(this, queryProfiles, featureTypes, @@ -1063,6 +1082,11 @@ public class RankProfile implements Cloneable { }); } + @Override + public String toString() { + return "rank profile '" + name() + "'"; + } + /** * A rank setting. The identity of a rank setting is its field name and type (not value). * A rank setting is immutable. @@ -1105,8 +1129,9 @@ public class RankProfile implements Cloneable { return name; } + @Override public String toString() { - return "type: " + name; + return "type " + name; } } @@ -1219,7 +1244,7 @@ public class RankProfile implements Cloneable { @Override public String toString() { - return "function " + function; + return function.toString(); } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/FieldRankSettings.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/FieldRankSettings.java index c6f59fbe596..2a2950f71e9 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/FieldRankSettings.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/FieldRankSettings.java @@ -4,11 +4,9 @@ package com.yahoo.searchdefinition.derived; import com.yahoo.collections.Pair; import java.util.ArrayList; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.logging.Logger; /** * The rank settings of a field used for native rank features. @@ -17,9 +15,7 @@ import java.util.logging.Logger; */ public class FieldRankSettings { - private static final Logger logger = Logger.getLogger(FieldRankSettings.class.getName()); - - private String fieldName; + private final String fieldName; private final Map<String, NativeTable> tables = new LinkedHashMap<>(); @@ -30,7 +26,7 @@ public class FieldRankSettings { public void addTable(NativeTable table) { NativeTable existing = tables.get(table.getType().getName()); if (existing != null) { - logger.info("Using already specified rank table " + existing + " for field " + fieldName + ", not " + table); + // TODO: Throw? return; } tables.put(table.getType().getName(), table); @@ -60,8 +56,7 @@ public class FieldRankSettings { public List<Pair<String, String>> deriveRankProperties() { List<Pair<String, String>> properties = new ArrayList<>(); - for (Iterator<NativeTable> i = tables.values().iterator(); i.hasNext();) { - NativeTable table = i.next(); + for (NativeTable table : tables.values()) { if (isFieldMatchTable(table)) properties.add(new Pair<>("nativeFieldMatch." + table.getType().getName() + "." + fieldName, table.getName())); if (isAttributeMatchTable(table)) @@ -72,6 +67,7 @@ public class FieldRankSettings { return properties; } + @Override public String toString() { return "rank settings of field " + fieldName; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java index 86c1d478974..bcebdf3a916 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java @@ -73,9 +73,9 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ } private boolean areDependenciesReady(RankProfile rank, RankProfileRegistry registry) { - return (rank.getInheritedName() == null) || - rankProfiles.containsKey(rank.getInheritedName()) || - (rank.schema() != null && registry.resolve(rank.schema().getDocument(), rank.getInheritedName()) != null); + return rank.inheritedNames().isEmpty() || + rankProfiles.keySet().containsAll(rank.inheritedNames()) || + (rank.schema() != null && rank.inheritedNames().stream().allMatch(name -> registry.resolve(rank.schema().getDocument(), name) != null)); } private void deriveRankProfiles(RankProfileRegistry rankProfileRegistry, diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RawRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RawRankProfile.java index a64f0939677..75703c33f07 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RawRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RawRankProfile.java @@ -156,8 +156,7 @@ public class RawRankProfile implements RankProfilesConfig.Producer { /** * Creates a raw rank profile from the given rank profile */ - Deriver(RankProfile compiled, AttributeFields attributeFields, ModelContext.Properties deployProperties) - { + Deriver(RankProfile compiled, AttributeFields attributeFields, ModelContext.Properties deployProperties) { rankprofileName = compiled.name(); attributeTypes = compiled.getAttributeTypes(); queryFeatureTypes = compiled.getQueryFeatureTypes(); @@ -277,14 +276,15 @@ public class RawRankProfile implements RankProfilesConfig.Producer { private void deriveRankTypeSetting(RankProfile rankProfile, AttributeFields attributeFields) { for (Iterator<RankProfile.RankSetting> i = rankProfile.rankSettingIterator(); i.hasNext(); ) { RankProfile.RankSetting setting = i.next(); - if (!setting.getType().equals(RankProfile.RankSetting.Type.RANKTYPE)) continue; + if (setting.getType() != RankProfile.RankSetting.Type.RANKTYPE) continue; deriveNativeRankTypeSetting(setting.getFieldName(), (RankType) setting.getValue(), attributeFields, - hasDefaultRankTypeSetting(rankProfile, setting.getFieldName())); + hasDefaultRankTypeSetting(rankProfile, setting.getFieldName())); } } - private void deriveNativeRankTypeSetting(String fieldName, RankType rankType, AttributeFields attributeFields, boolean isDefaultSetting) { + private void deriveNativeRankTypeSetting(String fieldName, RankType rankType, AttributeFields attributeFields, + boolean isDefaultSetting) { if (isDefaultSetting) return; NativeRankTypeDefinition definition = nativeRankTypeDefinitions.getRankTypeDefinition(rankType); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConstantTensorTransformer.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConstantTensorTransformer.java index 166255799bd..f6ed5abaa7f 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConstantTensorTransformer.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/ConstantTensorTransformer.java @@ -63,8 +63,8 @@ public class ConstantTensorTransformer extends ExpressionTransformer<RankProfile TensorValue tensorValue = (TensorValue)value; String tensorType = tensorValue.asTensor().type().toString(); - context.rankProperties().put(constantReference.toString() + ".value", tensorValue.toString()); - context.rankProperties().put(constantReference.toString() + ".type", tensorType); + context.rankProperties().put(constantReference + ".value", tensorValue.toString()); + context.rankProperties().put(constantReference + ".type", tensorType); return new ReferenceNode(constantReference); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java index ae8f918dd4b..6944a1f9dd1 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/OnnxFeatureConverter.java @@ -39,7 +39,6 @@ public class OnnxFeatureConverter extends ExpressionTransformer<RankProfileTrans private ExpressionNode transformFeature(ReferenceNode feature, RankProfileTransformContext context) { if ( ! feature.getName().equals("onnx_vespa")) return feature; - try { FeatureArguments arguments = asFeatureArguments(feature.getArguments()); ConvertedModel convertedModel = diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index b4d2af42f55..f4a6ac1b03d 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -2036,15 +2036,15 @@ Object rankProfileItem(RankProfile profile) : { } /** * This rule consumes an inherits statement of a rank-profile. * - * @param profile The profile to modify. + * @param profile the profile to modify */ void inheritsRankProfile(RankProfile profile) : { - String str; + String name; } { - <INHERITS> str = identifierWithDash() - { profile.setInherited(str); } + <INHERITS> name = identifierWithDash() { profile.inherit(name); } + ( <COMMA> name = identifierWithDash() { profile.inherit(name);; } )* } /** diff --git a/config-model/src/test/derived/attributerank/attributerank.sd b/config-model/src/test/derived/attributerank/attributerank.sd index 86f96e4817c..4989e1795cd 100644 --- a/config-model/src/test/derived/attributerank/attributerank.sd +++ b/config-model/src/test/derived/attributerank/attributerank.sd @@ -38,4 +38,5 @@ search attributerank { rank-type singledouble: identity rank-type singlestring: identity } + } diff --git a/config-model/src/test/derived/rankprofilemodularity/rank-profiles.cfg b/config-model/src/test/derived/rankprofilemodularity/rank-profiles.cfg index 8953a8b6bcf..9364e7e1369 100644 --- a/config-model/src/test/derived/rankprofilemodularity/rank-profiles.cfg +++ b/config-model/src/test/derived/rankprofilemodularity/rank-profiles.cfg @@ -8,6 +8,15 @@ rankprofile[].fef.property[].name "vespa.hitcollector.arraysize" rankprofile[].fef.property[].value "0" rankprofile[].fef.property[].name "vespa.dump.ignoredefaultfeatures" rankprofile[].fef.property[].value "true" +rankprofile[].name "in_schema0" +rankprofile[].fef.property[].name "rankingExpression(fo2).rankingScript" +rankprofile[].fef.property[].value "random" +rankprofile[].fef.property[].name "rankingExpression(f2).rankingScript" +rankprofile[].fef.property[].value "fieldMatch(title) + rankingExpression(fo2)" +rankprofile[].fef.property[].name "rankingExpression(fo1).rankingScript" +rankprofile[].fef.property[].value "now" +rankprofile[].fef.property[].name "vespa.rank.firstphase" +rankprofile[].fef.property[].value "rankingExpression(f2)" rankprofile[].name "in_schema1" rankprofile[].fef.property[].name "vespa.rank.firstphase" rankprofile[].fef.property[].value "nativeRank" @@ -18,6 +27,15 @@ rankprofile[].fef.property[].name "rankingExpression(f2).rankingScript" rankprofile[].fef.property[].value "fieldMatch(title) + rankingExpression(fo2)" rankprofile[].fef.property[].name "vespa.rank.firstphase" rankprofile[].fef.property[].value "rankingExpression(f2)" +rankprofile[].name "in_schema3" +rankprofile[].fef.property[].name "rankingExpression(fo2).rankingScript" +rankprofile[].fef.property[].value "random" +rankprofile[].fef.property[].name "rankingExpression(f2).rankingScript" +rankprofile[].fef.property[].value "fieldMatch(title) + rankingExpression(fo2)" +rankprofile[].fef.property[].name "rankingExpression(fo1).rankingScript" +rankprofile[].fef.property[].value "now" +rankprofile[].fef.property[].name "vespa.rank.firstphase" +rankprofile[].fef.property[].value "rankingExpression(f2)" rankprofile[].name "outside_schema1" rankprofile[].fef.property[].name "rankingExpression(fo1).rankingScript" rankprofile[].fef.property[].value "now" diff --git a/config-model/src/test/derived/rankprofilemodularity/test.sd b/config-model/src/test/derived/rankprofilemodularity/test.sd index 720e2143f17..b017a70fef1 100644 --- a/config-model/src/test/derived/rankprofilemodularity/test.sd +++ b/config-model/src/test/derived/rankprofilemodularity/test.sd @@ -8,6 +8,9 @@ schema test { } + rank-profile in_schema0 inherits in_schema3 { + } + rank-profile in_schema1 { first-phase { @@ -28,4 +31,16 @@ schema test { } + rank-profile in_schema3 inherits outside_schema1, outside_schema2 { + + function f2() { + expression: fieldMatch(title) + fo2 + } + + first-phase { + expression: f2 + } + + } + }
\ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java index c88e8149e46..df64d65aec3 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java @@ -52,7 +52,7 @@ public class RankProfileTestCase extends AbstractSchemaTestCase { document.addField("b", DataType.STRING); schema.addDocument(document); RankProfile child = new RankProfile("child", schema, rankProfileRegistry, schema.rankingConstants()); - child.setInherited("default"); + child.inherit("default"); rankProfileRegistry.add(child); Iterator<RankProfile.RankSetting> i = child.rankSettingIterator(); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaImporterTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaImporterTestCase.java index 8a9300a83eb..9c895b452d5 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaImporterTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaImporterTestCase.java @@ -18,6 +18,7 @@ import org.junit.Test; import java.io.IOException; import java.util.Iterator; +import static com.google.common.collect.testing.Helpers.assertEmpty; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -53,14 +54,14 @@ public class SchemaImporterTestCase extends AbstractSchemaTestCase { new MakeAliases(schema, new BaseDeployLogger(), rankProfileRegistry, new QueryProfiles()).process(true, false); // First field - field=(SDField) document.getField("title"); - assertEquals(DataType.STRING,field.getDataType()); + field = (SDField) document.getField("title"); + assertEquals(DataType.STRING, field.getDataType()); assertEquals("{ input title | tokenize normalize stem:\"BEST\" | summary title | index title; }", field.getIndexingScript().toString()); assertFalse(schema.getIndex("default").isPrefix()); assertTrue(schema.getIndex("title").isPrefix()); - Iterator<String> titleAliases= schema.getIndex("title").aliasIterator(); - assertEquals("aliaz",titleAliases.next()); - assertEquals("analias.totitle",titleAliases.next()); + Iterator<String> titleAliases = schema.getIndex("title").aliasIterator(); + assertEquals("aliaz", titleAliases.next()); + assertEquals("analias.totitle", titleAliases.next()); assertEquals("analias.todefault", schema.getIndex("default").aliasIterator().next()); assertEquals(RankType.IDENTITY, field.getRankType()); @@ -70,7 +71,7 @@ public class SchemaImporterTestCase extends AbstractSchemaTestCase { assertTrue(field.isHeader()); // Second field - field=(SDField) document.getField("description"); + field = (SDField) document.getField("description"); assertEquals(RankType.ABOUT, field.getRankType()); assertEquals(SummaryTransform.NONE, field.getSummaryField("description").getTransform()); @@ -81,30 +82,30 @@ public class SchemaImporterTestCase extends AbstractSchemaTestCase { assertEquals("hallo", schema.getIndex("description").aliasIterator().next()); // Third field - field=(SDField) document.getField("chatter"); + field = (SDField) document.getField("chatter"); assertEquals(RankType.ABOUT, field.getRankType()); assertNull(field.getStemming()); assertTrue(field.getNormalizing().doRemoveAccents()); // Fourth field - field=(SDField) document.getField("category"); + field = (SDField) document.getField("category"); assertEquals(0, field.getAttributes().size()); assertEquals(Stemming.NONE, field.getStemming()); assertFalse(field.getNormalizing().doRemoveAccents()); // Fifth field - field=(SDField) document.getField("popularity"); + field = (SDField) document.getField("popularity"); assertEquals("{ input popularity | attribute popularity; }", field.getIndexingScript().toString()); // Sixth field - field=(SDField) document.getField("measurement"); - assertEquals(DataType.INT,field.getDataType()); + field = (SDField) document.getField("measurement"); + assertEquals(DataType.INT, field.getDataType()); assertEquals(RankType.EMPTY, field.getRankType()); assertEquals(1, field.getAttributes().size()); // Seventh field - field= schema.getConcreteField("categories"); + field = schema.getConcreteField("categories"); assertEquals("{ input categories_src | lowercase | normalize | tokenize normalize stem:\"BEST\" | index categories; }", field.getIndexingScript().toString()); assertTrue(field.isHeader()); @@ -139,27 +140,27 @@ public class SchemaImporterTestCase extends AbstractSchemaTestCase { assertEquals(Attribute.CollectionType.ARRAY, attribute.getCollectionType()); // Rank Profiles - RankProfile profile=rankProfileRegistry.get(schema, "default"); + RankProfile profile = rankProfileRegistry.get(schema, "default"); assertNotNull(profile); - assertNull(profile.getInheritedName()); + assertEmpty(profile.inheritedNames()); assertNull(profile.getDeclaredRankSetting("measurement", RankProfile.RankSetting.Type.RANKTYPE)); assertEquals(RankType.EMPTY, profile.getRankSetting("measurement", RankProfile.RankSetting.Type.RANKTYPE).getValue()); - profile=rankProfileRegistry.get(schema, "experimental"); + profile = rankProfileRegistry.get(schema, "experimental"); assertNotNull(profile); - assertEquals("default",profile.getInheritedName()); + assertEquals("default", profile.inheritedNames().get(0)); assertEquals(RankType.IDENTITY, profile.getDeclaredRankSetting("measurement", RankProfile.RankSetting.Type.RANKTYPE).getValue()); - profile=rankProfileRegistry.get(schema, "other"); + profile = rankProfileRegistry.get(schema, "other"); assertNotNull(profile); - assertEquals("experimental",profile.getInheritedName()); + assertEquals("experimental", profile.inheritedNames().get(0)); // The extra-document field - SDField exact= schema.getConcreteField("exact"); - assertNotNull("Extra field was parsed",exact); - assertEquals("exact",exact.getName()); - assertEquals(Stemming.NONE,exact.getStemming()); + SDField exact = schema.getConcreteField("exact"); + assertNotNull("Extra field was parsed", exact); + assertEquals("exact", exact.getName()); + assertEquals(Stemming.NONE, exact.getStemming()); assertFalse(exact.getNormalizing().doRemoveAccents()); assertEquals("{ input title . \" \" . input category | tokenize | summary exact | index exact; }", exact.getIndexingScript().toString()); @@ -179,10 +180,10 @@ public class SchemaImporterTestCase extends AbstractSchemaTestCase { @Test public void testIdImporting() throws IOException, ParseException { Schema schema = ApplicationBuilder.buildFromFile("src/test/examples/strange.sd"); - SDField idecidemyide=(SDField) schema.getDocument().getField("idecidemyide"); - assertEquals(5,idecidemyide.getId()); - SDField sodoi=(SDField) schema.getDocument().getField("sodoi"); - assertEquals(7,sodoi.getId()); + SDField idecidemyide = (SDField)schema.getDocument().getField("idecidemyide"); + assertEquals(5, idecidemyide.getId()); + SDField sodoi = (SDField) schema.getDocument().getField("sodoi"); + assertEquals(7, sodoi.getId()); } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java index d012f330328..112dbd2587e 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java @@ -34,7 +34,7 @@ public class SchemaTestCase { ApplicationBuilder.createFromStrings(logger, schema); assertEquals("schema 'test' inherits 'nonesuch', but this schema does not exist", logger.entries.get(0).message); - fail("Expected failure"); + fail("Expected exception"); } catch (IllegalArgumentException e) { assertEquals("schema 'test' inherits 'nonesuch', but this schema does not exist", e.getMessage()); @@ -61,6 +61,7 @@ public class SchemaTestCase { " }" + "}"); ApplicationBuilder.createFromStrings(new DeployLoggerStub(), parent, child); + fail("Expected exception"); } catch (IllegalArgumentException e) { assertEquals("schema 'child' inherits 'parent', " + @@ -187,7 +188,7 @@ public class SchemaTestCase { assertNotNull(child1.getExtraField("child1_field")); assertNotNull(builder.getRankProfileRegistry().get(child1, "parent_profile")); assertNotNull(builder.getRankProfileRegistry().get(child1, "child1_profile")); - assertEquals("parent_profile", builder.getRankProfileRegistry().get(child1, "child1_profile").getInheritedName()); + assertEquals("parent_profile", builder.getRankProfileRegistry().get(child1, "child1_profile").inheritedNames().get(0)); assertNotNull(child1.rankingConstants().get("parent_constant")); assertNotNull(child1.rankingConstants().get("child1_constant")); assertTrue(child1.rankingConstants().asMap().containsKey("parent_constant")); @@ -222,7 +223,7 @@ public class SchemaTestCase { assertNotNull(child2.getExtraField("child2_field")); assertNotNull(builder.getRankProfileRegistry().get(child2, "parent_profile")); assertNotNull(builder.getRankProfileRegistry().get(child2, "child2_profile")); - assertEquals("parent_profile", builder.getRankProfileRegistry().get(child2, "child2_profile").getInheritedName()); + assertEquals("parent_profile", builder.getRankProfileRegistry().get(child2, "child2_profile").inheritedNames().get(0)); assertNotNull(child2.rankingConstants().get("parent_constant")); assertNotNull(child2.rankingConstants().get("child2_constant")); assertTrue(child2.rankingConstants().asMap().containsKey("parent_constant")); @@ -320,6 +321,98 @@ public class SchemaTestCase { assertInheritedFromParent(application.schemas().get("grandchild"), application, builder.getRankProfileRegistry()); } + @Test + public void testInheritingMultipleRankProfilesWithOverlappingConstructsIsDisallowed1() throws ParseException { + try { + String profile = joinLines( + "schema test {" + + " document test {" + + " field title type string {" + + " indexing: summary" + + " }" + + " }" + + " rank-profile r1 {" + + " first-phase {" + + " expression: fieldMatch(title)" + + " }" + + " }" + + " rank-profile r2 {" + + " first-phase {" + + " expression: fieldMatch(title)" + + " }" + + " }" + + " rank-profile r3 inherits r1, r2 {" + + " }" + + "}"); + ApplicationBuilder.createFromStrings(new DeployLoggerStub(), profile); + fail("Expected exception"); + } + catch (IllegalArgumentException e) { + assertEquals("Only one of the profiles inherited by rank profile 'r3' can contain first-phase expression, but it is present in all of [rank profile 'r1', rank profile 'r2']", + e.getMessage()); + } + } + + @Test + public void testInheritingMultipleRankProfilesWithOverlappingConstructsIsAllowedWhenDefinedInChild() throws ParseException { + String profile = joinLines( + "schema test {" + + " document test {" + + " field title type string {" + + " indexing: summary" + + " }" + + " }" + + " rank-profile r1 {" + + " first-phase {" + + " expression: fieldMatch(title)" + + " }" + + " }" + + " rank-profile r2 {" + + " first-phase {" + + " expression: fieldMatch(title)" + + " }" + + " }" + + " rank-profile r3 inherits r1, r2 {" + + " first-phase {" + // Redefined here so this does not cause failure + " expression: nativeRank" + + " }" + + " }" + + "}"); + ApplicationBuilder.createFromStrings(new DeployLoggerStub(), profile); + } + + @Test + public void testInheritingMultipleRankProfilesWithOverlappingConstructsIsDisallowed2() throws ParseException { + try { + String profile = joinLines( + "schema test {" + + " document test {" + + " field title type string {" + + " indexing: summary" + + " }" + + " }" + + " rank-profile r1 {" + + " function f1() {" + + " expression: fieldMatch(title)" + + " }" + + " }" + + " rank-profile r2 {" + + " function f1() {" + + " expression: fieldMatch(title)" + + " }" + + " }" + + " rank-profile r3 inherits r1, r2 {" + + " }" + + "}"); + ApplicationBuilder.createFromStrings(new DeployLoggerStub(), profile); + fail("Expected exception"); + } + catch (IllegalArgumentException e) { + assertEquals("rank profile 'r3' inherits rank profile 'r2' which contains function 'f1', but this function is already defined in another profile this inherits", + e.getMessage()); + } + } + private void assertInheritedFromParent(Schema schema, Application application, RankProfileRegistry rankProfileRegistry) { assertEquals("pf1", schema.fieldSets().userFieldSets().get("parent_set").getFieldNames().stream().findFirst().get()); assertEquals(Stemming.NONE, schema.getStemming()); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java index 2a9d64767ba..d44c102a5c3 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithOnnxTestCase.java @@ -259,7 +259,7 @@ public class RankingExpressionWithOnnxTestCase { public void testFunctionGeneration() { final String name = "small_constants_and_functions"; final String rankProfiles = - " rank-profile my_profile {\n" + + " rank-profile my_profile {\n" + " function input() {\n" + " expression: tensor<float>(d0[3])(0.0)\n" + " }\n" + diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java index 7c7f01b414a..094367dc140 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java @@ -377,7 +377,7 @@ public class FastHit extends Hit { } } - private static void appendAsHex(byte [] gid, StringBuilder sb) { + private static void appendAsHex(byte[] gid, StringBuilder sb) { for (byte b : gid) { String hex = Integer.toHexString(0xFF & b); if (hex.length() == 1) { diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java b/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java index 03b0f092abb..bd0415fa449 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java @@ -11,17 +11,17 @@ import java.util.Arrays; */ public class LeanHit implements Comparable<LeanHit> { - private final byte [] gid; + private final byte[] gid; private final double relevance; - private final byte [] sortData; + private final byte[] sortData; private final int partId; private final int distributionKey; private FeatureData matchFeatures; - public LeanHit(byte [] gid, int partId, int distributionKey, double relevance) { + public LeanHit(byte[] gid, int partId, int distributionKey, double relevance) { this(gid, partId, distributionKey, relevance, null); } - public LeanHit(byte [] gid, int partId, int distributionKey, double relevance, byte [] sortData) { + public LeanHit(byte[] gid, int partId, int distributionKey, double relevance, byte[] sortData) { this.gid = gid; this.relevance = Double.isNaN(relevance) ? Double.NEGATIVE_INFINITY : relevance; this.sortData = sortData; @@ -31,8 +31,8 @@ public class LeanHit implements Comparable<LeanHit> { } public double getRelevance() { return relevance; } - public byte [] getGid() { return gid; } - public byte [] getSortData() { return sortData; } + public byte[] getGid() { return gid; } + public byte[] getSortData() { return sortData; } public boolean hasSortData() { return sortData != null; } public int getPartId() { return partId; } public int getDistributionKey() { return distributionKey; } @@ -51,7 +51,7 @@ public class LeanHit implements Comparable<LeanHit> { return (res != 0) ? res : compareData(gid, o.gid); } - private static int compareData(byte [] left, byte [] right) { + private static int compareData(byte[] left, byte[] right) { int i = Arrays.mismatch(left, right); if (i < 0) { return 0; diff --git a/integration/intellij/README.md b/integration/intellij/README.md index fec547f8a06..d293b320028 100644 --- a/integration/intellij/README.md +++ b/integration/intellij/README.md @@ -36,17 +36,19 @@ open a project with some sd file and see how the plugin works on it. ## Some useful links: -1. JetBrains official tutorials: https://plugins.jetbrains.com/docs/intellij/custom-language-support.html and - https://plugins.jetbrains.com/docs/intellij/custom-language-support-tutorial.html +1. Plugin development documentation: https://plugins.jetbrains.com/docs/intellij/welcome.html -2. Grammar-Kit HOWTO: Helps to understand the BNF syntax. +2. JetBrains official tutorials: https://plugins.jetbrains.com/docs/intellij/custom-language-support.html and + https://plugins.jetbrains.com/docs/intellij/custom-language-support-tutorial.html + +3. Grammar-Kit HOWTO: Helps to understand the BNF syntax. https://github.com/JetBrains/Grammar-Kit/blob/master/HOWTO.md -3. How to deal with left-recursion in the grammar (in SD for example it happens in expressions). Last answer here: +4. How to deal with left-recursion in the grammar (in SD for example it happens in expressions). Last answer here: https://intellij-support.jetbrains.com/hc/en-us/community/posts/360001258300-What-s-the-alternative-to-left-recursion-in-GrammarKit- -4. Great tutorial for a custom-language-plugin, but only for the basics (mainly the parser and lexer): +5. Great tutorial for a custom-language-plugin, but only for the basics (mainly the parser and lexer): https://medium.com/@shan1024/custom-language-plugin-development-for-intellij-idea-part-01-d6a41ab96bc9 -5. Code of Dart (some custom language) plugin for IntelliJ: +6. Code of Dart (some custom language) plugin for IntelliJ: https://github.com/JetBrains/intellij-plugins/tree/0f07ca63355d5530b441ca566c98f17c560e77f8/Dart
\ No newline at end of file diff --git a/integration/intellij/build.gradle b/integration/intellij/build.gradle index 2d5ad1f33d6..5529d3d7c15 100644 --- a/integration/intellij/build.gradle +++ b/integration/intellij/build.gradle @@ -36,7 +36,7 @@ compileJava { } group 'ai.vespa' -version '1.0.2' // Also update pom.xml version if this is changed +version '1.0.3' // Also update pom.xml version if this is changed sourceCompatibility = 11 diff --git a/integration/intellij/pom.xml b/integration/intellij/pom.xml index 984968b2834..94d8fd90f99 100644 --- a/integration/intellij/pom.xml +++ b/integration/intellij/pom.xml @@ -9,7 +9,7 @@ <relativePath>../parent/pom.xml</relativePath> </parent> <artifactId>vespa-intellij</artifactId> <!-- Not used - plugin is build by gradle --> - <version>1.0.2</version> <!-- See copy-zip below, which depends on this being the same as the v. in build.gradle --> + <version>1.0.3</version> <!-- See copy-zip below, which depends on this being the same as the v. in build.gradle --> <description> Maven wrapper for the gradle build of this IntelliJ plugin. </description> diff --git a/integration/intellij/src/main/bnf/ai/vespa/intellij/schema/parser/sd.bnf b/integration/intellij/src/main/bnf/ai/vespa/intellij/schema/parser/sd.bnf index 6be7302a28c..04368bf29af 100644 --- a/integration/intellij/src/main/bnf/ai/vespa/intellij/schema/parser/sd.bnf +++ b/integration/intellij/src/main/bnf/ai/vespa/intellij/schema/parser/sd.bnf @@ -40,7 +40,7 @@ ] } -SdFile ::= SchemaDefinition | DocumentDefinition +SdFile ::= SchemaDefinition | DocumentDefinition | RankProfileDefinition SchemaDefinition ::= (search | schema) IdentifierVal? (inherits IdentifierVal)? '{' SchemaBody '}' SchemaBody ::= SchemaBodyOptions* DocumentDefinition SchemaBodyOptions* // Does not support zero-or-one occurrences private SchemaBodyOptions ::= SchemaFieldDefinition | ImportFieldDefinition | DocumentSummaryDefinition | @@ -129,7 +129,7 @@ PrimitiveExpr ::= (('-')? INTEGER_REG) | (('-')? FLOAT_REG) | IdentifierVal | Ra //------------------------- //-- Rank Profile rules --- //------------------------- -RankProfileDefinition ::= (rank-profile | model) IdentifierWithDashVal (inherits IdentifierWithDashVal)? '{' RankProfileBody '}' +RankProfileDefinition ::= (rank-profile | model) IdentifierWithDashVal (inherits IdentifierWithDashVal (',' IdentifierWithDashVal)*)? '{' RankProfileBody '}' { mixin="ai.vespa.intellij.schema.psi.impl.SdNamedElementImpl" implements=["ai.vespa.intellij.schema.psi.SdDeclaration"] } diff --git a/integration/intellij/src/main/java/ai/vespa/intellij/schema/SdUtil.java b/integration/intellij/src/main/java/ai/vespa/intellij/schema/SdUtil.java index d9f769d07cc..cba1bd04ce4 100644 --- a/integration/intellij/src/main/java/ai/vespa/intellij/schema/SdUtil.java +++ b/integration/intellij/src/main/java/ai/vespa/intellij/schema/SdUtil.java @@ -132,7 +132,8 @@ public class SdUtil { for (VirtualFile vfile : virtualFiles) { SdFile sdFile = (SdFile) PsiManager.getInstance(project).findFile(vfile); - if (sdFile != null && !sdFile.getName().equals(docName + ".sd")) { + if (sdFile != null && + ( !sdFile.getName().equals(docName + ".sd") && !sdFile.getName().equals(docName + ".profile"))) { continue; } result.addAll(SdUtil.findDeclarationsByName(sdFile, name)); diff --git a/integration/intellij/src/main/resources/META-INF/plugin.xml b/integration/intellij/src/main/resources/META-INF/plugin.xml index 673a66f3228..ea6fe3f2f15 100644 --- a/integration/intellij/src/main/resources/META-INF/plugin.xml +++ b/integration/intellij/src/main/resources/META-INF/plugin.xml @@ -29,7 +29,7 @@ <!-- Extension points defined by the plugin --> <extensions defaultExtensionNs="com.intellij"> <fileType name="Sd File" implementationClass="ai.vespa.intellij.schema.SdFileType" fieldName="INSTANCE" - language="Sd" extensions="sd"/> + language="Sd" extensions="sd;profile"/> <lang.parserDefinition language="Sd" implementationClass="ai.vespa.intellij.schema.parser.SdParserDefinition"/> <lang.syntaxHighlighterFactory language="Sd" implementationClass="ai.vespa.intellij.schema.SdSyntaxHighlighterFactory"/> <completion.contributor language="Sd" implementationClass="ai.vespa.intellij.schema.SdCompletionContributor"/> |