From c3985a166aea6d03447b18e09f9071cc280a1c96 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 19 Jan 2024 11:34:58 +0000 Subject: fix semantics for empty feature lists * allow FeatureList to parse empty input and return empty list * if an empty feature list block is specified in a rank-profile, trigger that we no longer get the implicit inheritance --- .../main/java/com/yahoo/schema/RankProfile.java | 30 +++++++--------------- .../src/main/javacc/RankingExpressionParser.jj | 2 +- 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/schema/RankProfile.java b/config-model/src/main/java/com/yahoo/schema/RankProfile.java index 502b054f84e..9b3e236612a 100644 --- a/config-model/src/main/java/com/yahoo/schema/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/schema/RankProfile.java @@ -615,18 +615,6 @@ public class RankProfile implements Cloneable { .orElse(Set.of()); } - private void addSummaryFeature(ReferenceNode feature) { - if (summaryFeatures == null) - summaryFeatures = new LinkedHashSet<>(); - summaryFeatures.add(feature); - } - - private void addMatchFeature(ReferenceNode feature) { - if (matchFeatures == null) - matchFeatures = new LinkedHashSet<>(); - matchFeatures.add(feature); - } - private void addImplicitMatchFeatures(List list) { if (hiddenMatchFeatures == null) hiddenMatchFeatures = new LinkedHashSet<>(); @@ -642,15 +630,19 @@ public class RankProfile implements Cloneable { /** Adds the content of the given feature list to the internal list of summary features. */ public void addSummaryFeatures(FeatureList features) { + if (summaryFeatures == null) + summaryFeatures = new LinkedHashSet<>(); for (ReferenceNode feature : features) { - addSummaryFeature(feature); + summaryFeatures.add(feature); } } /** Adds the content of the given feature list to the internal list of match features. */ public void addMatchFeatures(FeatureList features) { + if (matchFeatures == null) + matchFeatures = new LinkedHashSet<>(); for (ReferenceNode feature : features) { - addMatchFeature(feature); + matchFeatures.add(feature); } } @@ -661,20 +653,16 @@ public class RankProfile implements Cloneable { .orElse(Set.of()); } - private void addRankFeature(ReferenceNode feature) { - if (rankFeatures == null) - rankFeatures = new LinkedHashSet<>(); - rankFeatures.add(feature); - } - /** * Adds the content of the given feature list to the internal list of rank features. * * @param features The features to add. */ public void addRankFeatures(FeatureList features) { + if (rankFeatures == null) + rankFeatures = new LinkedHashSet<>(); for (ReferenceNode feature : features) { - addRankFeature(feature); + rankFeatures.add(feature); } } diff --git a/searchlib/src/main/javacc/RankingExpressionParser.jj b/searchlib/src/main/javacc/RankingExpressionParser.jj index 591f0eb8b37..97aa42f79c9 100755 --- a/searchlib/src/main/javacc/RankingExpressionParser.jj +++ b/searchlib/src/main/javacc/RankingExpressionParser.jj @@ -177,7 +177,7 @@ List featureList() : ReferenceNode exp; } { - ( ( exp = feature() { ret.add(exp); } )+ ) + ( ( exp = feature() { ret.add(exp); } )* ) { return ret; } } -- cgit v1.2.3 From a28d483958b1d564957f8f7fa3070ae04bd4f431 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Sun, 21 Jan 2024 11:16:48 +0000 Subject: unit test reset of feature lists --- .../src/test/derived/rankprofileinheritance/child.sd | 11 +++++++++++ .../derived/rankprofileinheritance/rank-profiles.cfg | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/config-model/src/test/derived/rankprofileinheritance/child.sd b/config-model/src/test/derived/rankprofileinheritance/child.sd index 2517d0731f5..8348a62838c 100644 --- a/config-model/src/test/derived/rankprofileinheritance/child.sd +++ b/config-model/src/test/derived/rankprofileinheritance/child.sd @@ -39,4 +39,15 @@ schema child { } + rank-profile profile5 inherits profile1 { + match-features { + attribute(field3) + } + } + + rank-profile profile6 inherits profile1 { + summary-features { } + match-features { } + } + } diff --git a/config-model/src/test/derived/rankprofileinheritance/rank-profiles.cfg b/config-model/src/test/derived/rankprofileinheritance/rank-profiles.cfg index a3bc6791412..ccf52da3b5e 100644 --- a/config-model/src/test/derived/rankprofileinheritance/rank-profiles.cfg +++ b/config-model/src/test/derived/rankprofileinheritance/rank-profiles.cfg @@ -52,3 +52,23 @@ rankprofile[].fef.property[].name "vespa.feature.rename" rankprofile[].fef.property[].value "rankingExpression(function4)" rankprofile[].fef.property[].name "vespa.feature.rename" rankprofile[].fef.property[].value "function4" +rankprofile[].name "profile5" +rankprofile[].fef.property[].name "rankingExpression(function1).rankingScript" +rankprofile[].fef.property[].value "attribute(field1) + 5" +rankprofile[].fef.property[].name "rankingExpression(function1b).rankingScript" +rankprofile[].fef.property[].value "attribute(field1) + 42" +rankprofile[].fef.property[].name "vespa.summary.feature" +rankprofile[].fef.property[].value "attribute(field1)" +rankprofile[].fef.property[].name "vespa.summary.feature" +rankprofile[].fef.property[].value "rankingExpression(function1)" +rankprofile[].fef.property[].name "vespa.match.feature" +rankprofile[].fef.property[].value "attribute(field3)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "rankingExpression(function1)" +rankprofile[].fef.property[].name "vespa.feature.rename" +rankprofile[].fef.property[].value "function1" +rankprofile[].name "profile6" +rankprofile[].fef.property[].name "rankingExpression(function1).rankingScript" +rankprofile[].fef.property[].value "attribute(field1) + 5" +rankprofile[].fef.property[].name "rankingExpression(function1b).rankingScript" +rankprofile[].fef.property[].value "attribute(field1) + 42" -- cgit v1.2.3