From 584fa261cc51cfca607e2068dac8f743e63496cb Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 9 Jul 2018 16:01:01 +0200 Subject: [PATCH] Remove the ability to index or query context suggestions without context (#31007) This is a follow up of #30712 that removes the ability to index or query and context enabled completion field without context. Relates #30712 --- .../migration/migrate_7_0/search.asciidoc | 7 + .../rest-api-spec/test/suggest/30_context.yml | 26 ++-- .../test/suggest/40_typed_keys.yml | 17 +-- .../completion/context/ContextMappings.java | 13 +- .../ContextCompletionSuggestSearchIT.java | 120 ++---------------- 5 files changed, 33 insertions(+), 150 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc index 123ad201cbbaf..11f4650912723 100644 --- a/docs/reference/migration/migrate_7_0/search.asciidoc +++ b/docs/reference/migration/migrate_7_0/search.asciidoc @@ -85,6 +85,13 @@ for a particular index with the index setting `index.max_regex_length`. Search requests with extra content after the main object will no longer be accepted by the `_search` endpoint. A parsing exception will be thrown instead. +==== Context Completion Suggester + +The ability to query and index context enabled suggestions without context, +deprecated in 6.x, has been removed. Context enabled suggestion queries +without contexts have to visit every suggestion, which degrades the search performance +considerably. + ==== Semantics changed for `max_concurrent_shard_requests` `max_concurrent_shard_requests` used to limit the total number of concurrent shard diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml index dfb849fff5700..1bf70e7dc428f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml @@ -337,11 +337,10 @@ setup: - match: { suggest.result.0.options.0.text: "foo" } --- -"Indexing and Querying without contexts is deprecated": +"Indexing and Querying without contexts is forbidden": - skip: version: " - 6.99.99" - reason: this feature was deprecated in 7.0 - features: "warnings" + reason: this feature was removed in 7.0 - do: index: @@ -359,8 +358,7 @@ setup: color: "blue" - do: - warnings: - - "The ability to index a suggestion with no context on a context enabled completion field is deprecated and will be removed in the next major release." + catch: /Contexts are mandatory in context enabled completion field \[suggest_context\]/ index: index: test type: test @@ -373,9 +371,9 @@ setup: indices.refresh: {} - do: - warnings: - - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." + catch: /Missing mandatory contexts in context query/ search: + allow_partial_search_results: false body: suggest: result: @@ -383,12 +381,10 @@ setup: completion: field: suggest_context - - length: { suggest.result: 1 } - - do: - warnings: - - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." + catch: /Missing mandatory contexts in context query/ search: + allow_partial_search_results: false body: suggest: result: @@ -397,12 +393,10 @@ setup: field: suggest_context contexts: {} - - length: { suggest.result: 1 } - - do: - warnings: - - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." + catch: /Missing mandatory contexts in context query/ search: + allow_partial_search_results: false body: suggest: result: @@ -411,5 +405,3 @@ setup: field: suggest_multi_contexts contexts: location: [] - - - length: { suggest.result: 1 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml index 6e799c2bfc500..44cd6d589c26a 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml @@ -19,8 +19,6 @@ setup: "type" : "category" - do: - warnings: - - "The ability to index a suggestion with no context on a context enabled completion field is deprecated and will be removed in the next major release." bulk: refresh: true index: test @@ -29,20 +27,12 @@ setup: - '{"index": {}}' - '{"title": "Elasticsearch in Action", "suggestions": {"input": "ELK in Action", "contexts": {"format": "ebook"}}}' - '{"index": {}}' - - '{"title": "Elasticsearch - The Definitive Guide", "suggestions": {"input": ["Elasticsearch in Action"]}}' + - '{"title": "Elasticsearch - The Definitive Guide", "suggestions": {"input": ["Elasticsearch in Action"], "contexts": {"format": "ebook"}}}' --- "Test typed keys parameter for suggesters": - - skip: -# version: " - 6.99.99" -# reason: queying a context suggester with no context was deprecated in 7.0 - version: "all" - reason: "Awaiting a fix: https://github.com/elastic/elasticsearch/issues/31698" - features: "warnings" - do: - warnings: - - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." search: typed_keys: true body: @@ -53,10 +43,6 @@ setup: term_suggester: term: field: title - completion_suggester: - prefix: "Elastic" - completion: - field: suggestions context_suggester: prefix: "Elastic" completion: @@ -68,6 +54,5 @@ setup: field: title - is_true: suggest.term#term_suggester - - is_true: suggest.completion#completion_suggester - is_true: suggest.completion#context_suggester - is_true: suggest.phrase#phrase_suggester diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java index 961d7fd9f59a7..3c0f0e80cebdb 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java @@ -25,8 +25,6 @@ import org.apache.lucene.util.CharsRefBuilder; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; -import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.mapper.CompletionFieldMapper; @@ -54,9 +52,6 @@ */ public class ContextMappings implements ToXContent { - private static final DeprecationLogger DEPRECATION_LOGGER = - new DeprecationLogger(Loggers.getLogger(ContextMappings.class)); - private final List> contextMappings; private final Map> contextNameMap; @@ -124,7 +119,7 @@ private class TypedContextField extends ContextSuggestField { private final ParseContext.Document document; TypedContextField(String name, String value, int weight, Map> contexts, - ParseContext.Document document) { + ParseContext.Document document) { super(name, value, weight); this.contexts = contexts; this.document = document; @@ -150,8 +145,7 @@ protected Iterable contexts() { } } if (typedContexts.isEmpty()) { - DEPRECATION_LOGGER.deprecated("The ability to index a suggestion with no context on a context enabled completion field" + - " is deprecated and will be removed in the next major release."); + throw new IllegalArgumentException("Contexts are mandatory in context enabled completion field [" + name + "]"); } return typedContexts; } @@ -186,8 +180,7 @@ public ContextQuery toContextQuery(CompletionQuery query, Map> map = new LinkedHashMap<>(Collections.singletonMap("cat", contextMapping)); - final CompletionMappingBuilder mapping = new CompletionMappingBuilder().context(map); - createIndexAndMapping(mapping); - int numDocs = 10; - List contexts = Arrays.asList("type1", "type2", "type3", "type4"); - List indexRequestBuilders = new ArrayList<>(); - for (int i = 0; i < numDocs; i++) { - XContentBuilder source = jsonBuilder() - .startObject() - .startObject(FIELD) - .field("input", "suggestion" + i) - .field("weight", i + 1) - .endObject() - .field("cat", contexts) - .endObject(); - indexRequestBuilders.add(client().prepareIndex(INDEX, TYPE, "" + i) - .setSource(source)); - } - indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); - - assertSuggestions("foo", prefix, "suggestion9", "suggestion8", "suggestion7", "suggestion6", "suggestion5"); - } - public void testMultiContextFiltering() throws Exception { LinkedHashMap> map = new LinkedHashMap<>(); map.put("cat", ContextBuilder.category("cat").field("cat").build()); @@ -295,14 +273,6 @@ public void testMultiContextFiltering() throws Exception { typeFilterSuggest.contexts(Collections.singletonMap("type", Arrays.asList(CategoryQueryContext.builder().setCategory("type2").build(), CategoryQueryContext.builder().setCategory("type1").build()))); assertSuggestions("foo", typeFilterSuggest, "suggestion9", "suggestion6", "suggestion5", "suggestion2", "suggestion1"); - - CompletionSuggestionBuilder multiContextFilterSuggest = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); - // query context order should never matter - Map> contextMap = new HashMap<>(); - contextMap.put("type", Collections.singletonList(CategoryQueryContext.builder().setCategory("type2").build())); - contextMap.put("cat", Collections.singletonList(CategoryQueryContext.builder().setCategory("cat2").build())); - multiContextFilterSuggest.contexts(contextMap); - assertSuggestions("foo", multiContextFilterSuggest, "suggestion6", "suggestion2"); } @AwaitsFix(bugUrl = "multiple context boosting is broken, as a suggestion, contexts pair is treated as (num(context) entries)") @@ -361,36 +331,6 @@ public void testMultiContextBoosting() throws Exception { assertSuggestions("foo", multiContextBoostSuggest, "suggestion9", "suggestion6", "suggestion5", "suggestion2", "suggestion1"); } - public void testMissingContextValue() throws Exception { - LinkedHashMap> map = new LinkedHashMap<>(); - map.put("cat", ContextBuilder.category("cat").field("cat").build()); - map.put("type", ContextBuilder.category("type").field("type").build()); - final CompletionMappingBuilder mapping = new CompletionMappingBuilder().context(map); - createIndexAndMapping(mapping); - int numDocs = 10; - List indexRequestBuilders = new ArrayList<>(); - for (int i = 0; i < numDocs; i++) { - XContentBuilder source = jsonBuilder() - .startObject() - .startObject(FIELD) - .field("input", "suggestion" + i) - .field("weight", i + 1) - .endObject(); - if (randomBoolean()) { - source.field("cat", "cat" + i % 2); - } - if (randomBoolean()) { - source.field("type", "type" + i % 4); - } - source.endObject(); - indexRequestBuilders.add(client().prepareIndex(INDEX, TYPE, "" + i) - .setSource(source)); - } - indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); - assertSuggestions("foo", prefix, "suggestion9", "suggestion8", "suggestion7", "suggestion6", "suggestion5"); - } - public void testSeveralContexts() throws Exception { LinkedHashMap> map = new LinkedHashMap<>(); final int numContexts = randomIntBetween(2, 5); @@ -417,35 +357,12 @@ public void testSeveralContexts() throws Exception { } indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); + CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg") + .contexts(Collections.singletonMap("type0", + Collections.singletonList(CategoryQueryContext.builder().setCategory("type").setPrefix(true).build()))); assertSuggestions("foo", prefix, "suggestion0", "suggestion1", "suggestion2", "suggestion3", "suggestion4"); } - public void testSimpleGeoPrefix() throws Exception { - LinkedHashMap> map = new LinkedHashMap<>(); - map.put("geo", ContextBuilder.geo("geo").build()); - final CompletionMappingBuilder mapping = new CompletionMappingBuilder().context(map); - createIndexAndMapping(mapping); - int numDocs = 10; - List indexRequestBuilders = new ArrayList<>(); - for (int i = 0; i < numDocs; i++) { - XContentBuilder source = jsonBuilder() - .startObject() - .startObject(FIELD) - .field("input", "suggestion" + i) - .field("weight", i + 1) - .startObject("contexts") - .field("geo", GeoHashUtils.stringEncode(1.2, 1.3)) - .endObject() - .endObject().endObject(); - indexRequestBuilders.add(client().prepareIndex(INDEX, TYPE, "" + i) - .setSource(source)); - } - indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); - assertSuggestions("foo", prefix, "suggestion9", "suggestion8", "suggestion7", "suggestion6", "suggestion5"); - } - public void testGeoFiltering() throws Exception { LinkedHashMap> map = new LinkedHashMap<>(); map.put("geo", ContextBuilder.geo("geo").build()); @@ -468,8 +385,6 @@ public void testGeoFiltering() throws Exception { .setSource(source)); } indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); - assertSuggestions("foo", prefix, "suggestion9", "suggestion8", "suggestion7", "suggestion6", "suggestion5"); CompletionSuggestionBuilder geoFilteringPrefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg") .contexts(Collections.singletonMap("geo", Collections.singletonList( @@ -500,8 +415,6 @@ public void testGeoBoosting() throws Exception { .setSource(source)); } indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); - assertSuggestions("foo", prefix, "suggestion9", "suggestion8", "suggestion7", "suggestion6", "suggestion5"); GeoQueryContext context1 = GeoQueryContext.builder().setGeoPoint(geoPoints[0]).setBoost(11).build(); GeoQueryContext context2 = GeoQueryContext.builder().setGeoPoint(geoPoints[1]).build(); @@ -572,8 +485,6 @@ public void testGeoNeighbours() throws Exception { .setSource(source)); } indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); - assertSuggestions("foo", prefix, "suggestion9", "suggestion8", "suggestion7", "suggestion6", "suggestion5"); CompletionSuggestionBuilder geoNeighbourPrefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg") .contexts(Collections.singletonMap("geo", Collections.singletonList(GeoQueryContext.builder().setGeoPoint(GeoPoint.fromGeohash(geohash)).build()))); @@ -668,14 +579,9 @@ public void testSkipDuplicatesWithContexts() throws Exception { expected[i] = "suggestion" + (numUnique-1-i); } indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder completionSuggestionBuilder = - SuggestBuilders.completionSuggestion(FIELD).prefix("sugg").skipDuplicates(true).size(numUnique); - - assertSuggestions("suggestions", completionSuggestionBuilder, expected); - Map> contextMap = new HashMap<>(); contextMap.put("cat", Arrays.asList(CategoryQueryContext.builder().setCategory("cat0").build())); - completionSuggestionBuilder = + CompletionSuggestionBuilder completionSuggestionBuilder = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg").contexts(contextMap).skipDuplicates(true).size(numUnique); String[] expectedModulo = Arrays.stream(expected)