From 32f3a452a34850d0527f8c6e2f1a4c9ebd32703f Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 31 May 2018 17:50:33 +0200 Subject: [PATCH 1/3] Remove the ability to index or query context suggestions without context 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 | 23 +++++-------------- .../test/suggest/40_typed_keys.yml | 15 +----------- .../completion/context/ContextMappings.java | 8 +++---- 4 files changed, 17 insertions(+), 36 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc index 7505b6f14d1b4..68e8c07f4eb77 100644 --- a/docs/reference/migration/migrate_7_0/search.asciidoc +++ b/docs/reference/migration/migrate_7_0/search.asciidoc @@ -81,3 +81,10 @@ 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. \ No newline at end of file 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..d1ec184a8226a 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,8 +371,7 @@ 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: body: suggest: @@ -383,11 +380,8 @@ 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: body: suggest: @@ -397,11 +391,8 @@ 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: body: suggest: @@ -411,5 +402,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 604be9ec99e00..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,18 +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 - 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: @@ -51,10 +43,6 @@ setup: term_suggester: term: field: title - completion_suggester: - prefix: "Elastic" - completion: - field: suggestions context_suggester: prefix: "Elastic" completion: @@ -66,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 4d6b53296f157..6235eb9eecf6d 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 @@ -124,7 +124,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 +150,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 +185,7 @@ public ContextQuery toContextQuery(CompletionQuery query, Map Date: Thu, 31 May 2018 19:05:47 +0200 Subject: [PATCH 2/3] fix integration tests --- .../ContextCompletionSuggestSearchIT.java | 118 ++---------------- 1 file changed, 13 insertions(+), 105 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java b/server/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java index 13f7e55277cc4..f47ae1cefe854 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java @@ -94,7 +94,9 @@ public void testContextPrefix() throws Exception { .setSource(source)); } indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg"); + CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg") + .contexts(Collections.singletonMap("cat", + Collections.singletonList(CategoryQueryContext.builder().setCategory("cat").setPrefix(true).build()))); assertSuggestions("foo", prefix, "suggestion9", "suggestion8", "suggestion7", "suggestion6", "suggestion5"); } @@ -125,7 +127,9 @@ public void testContextRegex() throws Exception { .setSource(source)); } indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).regex("sugg.*es"); + CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).regex("sugg.*es") + .contexts(Collections.singletonMap("cat", + Collections.singletonList(CategoryQueryContext.builder().setCategory("cat").setPrefix(true).build()))); assertSuggestions("foo", prefix, "sugg9estion", "sugg8estion", "sugg7estion", "sugg6estion", "sugg5estion"); } @@ -156,7 +160,9 @@ public void testContextFuzzy() throws Exception { .setSource(source)); } indexRandom(true, indexRequestBuilders); - CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg", Fuzziness.ONE); + CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg", Fuzziness.ONE) + .contexts(Collections.singletonMap("cat", + Collections.singletonList(CategoryQueryContext.builder().setCategory("cat").setPrefix(true).build()))); assertSuggestions("foo", prefix, "sugxgestion9", "sugxgestion8", "sugxgestion7", "sugxgestion6", "sugxgestion5"); } @@ -235,32 +241,6 @@ public void testSingleContextBoosting() throws Exception { assertSuggestions("foo", prefix, "suggestion8", "suggestion6", "suggestion4", "suggestion9", "suggestion2"); } - public void testSingleContextMultipleContexts() throws Exception { - CategoryContextMapping contextMapping = ContextBuilder.category("cat").field("cat").build(); - LinkedHashMap 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()); @@ -294,14 +274,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)") @@ -360,36 +332,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); @@ -416,35 +358,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()); @@ -467,8 +386,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( @@ -499,8 +416,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(); @@ -571,8 +486,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()))); @@ -667,14 +580,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) From ad80320ab95a7a4f49dbc696fe34708787759e77 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Fri, 6 Jul 2018 10:28:26 +0200 Subject: [PATCH 3/3] fix rest tests --- .../main/resources/rest-api-spec/test/suggest/30_context.yml | 3 +++ 1 file changed, 3 insertions(+) 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 d1ec184a8226a..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 @@ -373,6 +373,7 @@ setup: - do: catch: /Missing mandatory contexts in context query/ search: + allow_partial_search_results: false body: suggest: result: @@ -383,6 +384,7 @@ setup: - do: catch: /Missing mandatory contexts in context query/ search: + allow_partial_search_results: false body: suggest: result: @@ -394,6 +396,7 @@ setup: - do: catch: /Missing mandatory contexts in context query/ search: + allow_partial_search_results: false body: suggest: result: