From b3fe47f779805ee472d8f110af54f16e8a0caa76 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Fri, 25 Feb 2022 10:04:38 +0100 Subject: Add strict type checking option to query profiles --- .../com/yahoo/searchdefinition/RankProfile.java | 24 ++++++++++++++++ .../searchdefinition/parser/ParsedRankProfile.java | 5 +++- .../processing/RankingExpressionTypeResolver.java | 18 +++++++----- config-model/src/main/javacc/IntermediateParser.jj | 15 ++++++++-- config-model/src/main/javacc/SDParser.jj | 15 ++++++++-- .../RankingExpressionTypeResolverTestCase.java | 33 +++++++++++++++++++++- 6 files changed, 97 insertions(+), 13 deletions(-) (limited to 'config-model/src') 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 allFieldsList; + private Boolean strict; + /** Global onnx models not tied to a search definition */ private final OnnxModels onnxModels; private final RankingConstants rankingConstants; @@ -202,6 +204,28 @@ public class RankProfile implements Cloneable { return schema != null ? schema.allImportedFields() : Stream.empty(); } + /** + * 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 16b7b507121..c5f4ea812e2 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 @@ -11,8 +11,9 @@ import com.yahoo.searchlib.rankingexpression.evaluation.Value; * 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 { private final String name; @@ -50,4 +51,6 @@ class ParsedRankProfile { void addRankProperty(String key, String value) {} void addConstant(String name, Value value) {} void addConstantTensor(String name, TensorValue value) {} + void setStrict(boolean strict) {} + } 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 716dde26a95..6a7627043f9 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; @@ -1853,7 +1852,8 @@ void rankProfileItem(ParsedRankProfile profile) : { } | rankDegradation() | constants(profile) | matchFeatures(profile) - | summaryFeatures(profile) ) + | summaryFeatures(profile) + | strict(profile) ) } /** @@ -2087,6 +2087,15 @@ void summaryFeatures(ParsedRankProfile profile) : } } +void strict(ParsedRankProfile profile) : +{} +{ + ( + ( { profile.setStrict(true); } ) | + ( { profile.setStrict(false); } ) + ) +} + /** * This rule consumes a match-features block of a rank profile. * @@ -2534,6 +2543,7 @@ String identifier() : { } | | | + | | | | @@ -2581,6 +2591,7 @@ String identifier() : { } | | | + | | | | 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) : +{} +{ + ( + ( { profile.setStrict(true); } ) | + ( { profile.setStrict(false); } ) + ) +} + /** * This rule consumes a match-features block of a rank profile. * @@ -2672,6 +2681,7 @@ String identifier() : { } | | | + | | | | @@ -2719,6 +2729,7 @@ String identifier() : { } | | | + | | | | 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 @@ -407,6 +407,37 @@ public class RankingExpressionTypeResolverTestCase { assertNull(message); } + @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(); @@ -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); } -- cgit v1.2.3