diff options
9 files changed, 108 insertions, 16 deletions
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 068e85b7fcb..702eba8c8b5 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java @@ -131,6 +131,8 @@ public class RankProfile implements Cloneable { private List<ImmutableSDField> allFieldsList; + private Boolean strict; + /** Global onnx models not tied to a search definition */ private final OnnxModels onnxModels; private final RankingConstants rankingConstants; @@ -203,6 +205,28 @@ public class RankProfile implements Cloneable { } /** + * Returns whether type checking should fail if this profile accesses query features that are + * not defined in query profile types. + * + * Default is false. + */ + public boolean isStrict() { + Boolean declaredStrict = declaredStrict(); + if (declaredStrict != null) return declaredStrict; + return false; + } + + /** Returns the strict value declared in this or any parent profile. */ + public Boolean declaredStrict() { + if (strict != null) return strict; + return uniquelyInherited(p -> p.declaredStrict(), "strict").orElse(null); + } + + public void setStrict(Boolean strict) { + this.strict = strict; + } + + /** * Adds a profile to those inherited by this. * The profile must belong to this schema (directly or by inheritance). */ diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java index d0e9c231968..6fce026948d 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java @@ -17,8 +17,9 @@ import java.util.Optional; * This class holds the extracted information after parsing a * rank-profile block in a schema (.sd) file, using simple data * structures as far as possible. Do not put advanced logic here! + * * @author arnej27959 - **/ + */ class ParsedRankProfile extends ParsedBlock { private boolean ignoreDefaultRankFeatures = false; @@ -37,6 +38,7 @@ class ParsedRankProfile extends ParsedBlock { private String inheritedSummaryFeatures = null; private String inheritedMatchFeatures = null; private String secondPhaseExpression = null; + private Boolean strict = null; private final List<String> inherited = new ArrayList<>(); private final Map<String, Boolean> fieldsRankFilter = new HashMap<>(); private final Map<String, Integer> fieldsRankWeight = new HashMap<>(); @@ -72,6 +74,7 @@ class ParsedRankProfile extends ParsedBlock { Map<String, Value> getConstants() { return Map.copyOf(constants); } Optional<String> getInheritedSummaryFeatures() { return Optional.ofNullable(this.inheritedSummaryFeatures); } Optional<String> getSecondPhaseExpression() { return Optional.ofNullable(this.secondPhaseExpression); } + Optional<Boolean> isStrict() { return Optional.ofNullable(this.strict); } void addSummaryFeatures(FeatureList features) { verifyThat(summaryFeatures == null, "already has summary-features"); @@ -184,8 +187,15 @@ class ParsedRankProfile extends ParsedBlock { this.secondPhaseExpression = expression; } + void setStrict(boolean strict) { + verifyThat(this.strict == null, "already has strict"); + this.strict = strict; + } + void setTermwiseLimit(double limit) { verifyThat(termwiseLimit == null, "already has termwise-limit"); this.termwiseLimit = limit; } + + } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolver.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolver.java index e6adb8b683b..4154526d950 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolver.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolver.java @@ -86,13 +86,17 @@ public class RankingExpressionTypeResolver extends Processor { profile.getSummaryFeatures().forEach(f -> resolveType(f, "summary feature " + f, context)); ensureValidDouble(profile.getFirstPhaseRanking(), "first-phase expression", context); ensureValidDouble(profile.getSecondPhaseRanking(), "second-phase expression", context); - if ( context.tensorsAreUsed() && - ! context.queryFeaturesNotDeclared().isEmpty() && - ! warnedAbout.containsAll(context.queryFeaturesNotDeclared())) { - deployLogger.logApplicationPackage(Level.WARNING, "The following query features used in '" + profile.name() + - "' are not declared in query profile " + - "types and will be interpreted as scalars, not tensors: " + - context.queryFeaturesNotDeclared()); + if ( ( context.tensorsAreUsed() || profile.isStrict()) + && ! context.queryFeaturesNotDeclared().isEmpty() + && ! warnedAbout.containsAll(context.queryFeaturesNotDeclared())) { + if (profile.isStrict()) + throw new IllegalArgumentException(profile + " is strict but is missing a query profile type " + + "declaration of features " + context.queryFeaturesNotDeclared()); + else + deployLogger.logApplicationPackage(Level.WARNING, "The following query features used in " + profile + + " are not declared in query profile " + + "types and will be interpreted as scalars, not tensors: " + + context.queryFeaturesNotDeclared()); warnedAbout.addAll(context.queryFeaturesNotDeclared()); } } diff --git a/config-model/src/main/javacc/IntermediateParser.jj b/config-model/src/main/javacc/IntermediateParser.jj index cf0ad9eb101..cc03773f333 100644 --- a/config-model/src/main/javacc/IntermediateParser.jj +++ b/config-model/src/main/javacc/IntermediateParser.jj @@ -7,7 +7,6 @@ options { UNICODE_INPUT = true; CACHE_TOKENS = false; - STATIC = false; DEBUG_PARSER = false; ERROR_REPORTING = true; FORCE_LA_CHECK = true; @@ -1845,7 +1844,8 @@ void rankProfileItem(ParsedRankProfile profile) : { } | rankDegradation() | constants(profile) | matchFeatures(profile) - | summaryFeatures(profile) ) + | summaryFeatures(profile) + | strict(profile) ) } /** @@ -2079,6 +2079,15 @@ void summaryFeatures(ParsedRankProfile profile) : } } +void strict(ParsedRankProfile profile) : +{} +{ + <STRICT> <COLON> ( + ( <TRUE> { profile.setStrict(true); } ) | + ( <FALSE> { profile.setStrict(false); } ) + ) +} + /** * This rule consumes a match-features block of a rank profile. * @@ -2526,6 +2535,7 @@ String identifier() : { } | <LITERAL> | <LOCALE> | <LONG> + | <LOOSE> | <LOWERBOUND> | <LOWERCASE> | <MACRO> @@ -2573,6 +2583,7 @@ String identifier() : { } | <STATIC> | <STEMMING> | <STRENGTH> + | <STRICT> | <STRING> | <STRUCT> | <SUBSTRING> diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index 4cbee4a8503..f18107f574a 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -11,7 +11,6 @@ options { UNICODE_INPUT = true; CACHE_TOKENS = false; - STATIC = false; DEBUG_PARSER = false; ERROR_REPORTING = true; FORCE_LA_CHECK = true; @@ -1983,7 +1982,8 @@ void rankProfileItem(RankProfile profile) : { } | rankDegradation(profile) | constants(profile) | matchFeatures(profile) - | summaryFeatures(profile) ) + | summaryFeatures(profile) + | strict(profile) ) } /** @@ -2218,6 +2218,15 @@ void summaryFeatures(RankProfile profile) : } } +void strict(RankProfile profile) : +{} +{ + <STRICT> <COLON> ( + ( <TRUE> { profile.setStrict(true); } ) | + ( <FALSE> { profile.setStrict(false); } ) + ) +} + /** * This rule consumes a match-features block of a rank profile. * @@ -2672,6 +2681,7 @@ String identifier() : { } | <LITERAL> | <LOCALE> | <LONG> + | <LOOSE> | <LOWERBOUND> | <LOWERCASE> | <MACRO> @@ -2719,6 +2729,7 @@ String identifier() : { } | <STATIC> | <STEMMING> | <STRENGTH> + | <STRICT> | <STRING> | <STRUCT> | <SUBSTRING> diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolverTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolverTestCase.java index 7f5e8f8753a..2606b9a25aa 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolverTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolverTestCase.java @@ -408,6 +408,37 @@ public class RankingExpressionTypeResolverTestCase { } @Test + public void undeclaredQueryFeaturesAreNotAcceptedWhenStrict() throws Exception { + try { + InspectableDeployLogger logger = new InspectableDeployLogger(); + ApplicationBuilder builder = new ApplicationBuilder(logger); + builder.addSchema(joinLines( + "search test {", + " document test { ", + " field anyfield type double {" + + " indexing: attribute", + " }", + " }", + " rank-profile my_rank_profile {", + " strict: true" + + " first-phase {", + " expression: query(foo) + f() + sum(attribute(anyfield))", + " }", + " function f() {", + " expression: query(bar) + query(baz)", + " }", + " }", + "}" + )); + builder.build(true); + } + catch (IllegalArgumentException e) { + assertEquals("In schema 'test', rank profile 'my_rank_profile': rank profile 'my_rank_profile' is strict but is missing a query profile type declaration of features [query(bar), query(baz), query(foo)]", + Exceptions.toMessageString(e)); + } + } + + @Test public void undeclaredQueryFeaturesAreAcceptedWithWarningWhenUsingTensors() throws Exception { InspectableDeployLogger logger = new InspectableDeployLogger(); ApplicationBuilder builder = new ApplicationBuilder(logger); @@ -431,7 +462,7 @@ public class RankingExpressionTypeResolverTestCase { builder.build(true); String message = logger.findMessage("The following query features"); assertNotNull(message); - assertEquals("WARNING: The following query features used in 'my_rank_profile' are not declared in query profile types and " + + assertEquals("WARNING: The following query features used in rank profile 'my_rank_profile' are not declared in query profile types and " + "will be interpreted as scalars, not tensors: [query(bar), query(baz), query(foo)]", message); } diff --git a/integration/intellij/build.gradle b/integration/intellij/build.gradle index dd8a8e0a21c..c85429d4d94 100644 --- a/integration/intellij/build.gradle +++ b/integration/intellij/build.gradle @@ -36,7 +36,7 @@ compileJava { } group 'ai.vespa' -version '1.1.2' // Also update pom.xml version if this is changed +version '1.1.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 9982441c449..cb0df927ff9 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.1.2</version> <!-- See copy-zip below, which depends on this being the same as the v. in build.gradle --> + <version>1.1.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 8fc0aede912..6a5b59984e4 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 @@ -163,7 +163,7 @@ private RankProfileBodyOptions ::= MatchPhaseDefinition | NumThreadsDefinition | ignore-default-rank-features | RankPropertiesDefinition | FirstPhaseDefinition | SummaryFeaturesDefinition | MatchFeaturesDefinition | RankFeaturesDefinition | SecondPhaseDefinition | ConstantsDefinition | RankDefinition | RankTypeDefinition | - MinHitsDefinition | NumSearchPartitionDefinition | FieldWeightDefinition + MinHitsDefinition | NumSearchPartitionDefinition | FieldWeightDefinition | StrictDefinition MatchPhaseDefinition ::= match-phase '{' MatchPhaseBody '}' MatchPhaseBody ::= MatchPhaseBodyOptions+ @@ -180,6 +180,7 @@ private TermwiseLimitDefinition ::= termwise-limit ':' ('-')? (FLOAT_REG | INTEG private MinHitsDefinition ::= min-hits-per-thread ':' ('-')? INTEGER_REG private NumSearchPartitionDefinition ::= num-search-partition ':' INTEGER_REG FieldWeightDefinition ::= weight DottedIdentifiers ':' INTEGER_REG +StrictDefinition ::= strict ':' (true | false) FirstPhaseDefinition ::= first-phase '{' FirstPhaseBody '}' { mixin="ai.vespa.intellij.schema.psi.impl.SdFirstPhaseDefinitionMixin" } FirstPhaseBody ::= FirstPhaseBodyOptions* // Does not support zero-or-one occurrences private FirstPhaseBodyOptions ::= (keep-rank-count ':' INTEGER_REG) | (rank-score-drop-limit ':' ('-')? (FLOAT_REG | INTEGER_REG)) | ExpressionDefinition |