Skip to content

Commit

Permalink
Remove the ability to index or query context suggestions without cont…
Browse files Browse the repository at this point in the history
…ext (#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
  • Loading branch information
jimczi committed Jul 9, 2018
1 parent 5f5157a commit 584fa26
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 150 deletions.
7 changes: 7 additions & 0 deletions docs/reference/migration/migrate_7_0/search.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -373,22 +371,20 @@ 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:
text: "foo"
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:
Expand All @@ -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:
Expand All @@ -411,5 +405,3 @@ setup:
field: suggest_multi_contexts
contexts:
location: []

- length: { suggest.result: 1 }
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -53,10 +43,6 @@ setup:
term_suggester:
term:
field: title
completion_suggester:
prefix: "Elastic"
completion:
field: suggestions
context_suggester:
prefix: "Elastic"
completion:
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -54,9 +52,6 @@
*/
public class ContextMappings implements ToXContent {

private static final DeprecationLogger DEPRECATION_LOGGER =
new DeprecationLogger(Loggers.getLogger(ContextMappings.class));

private final List<ContextMapping<?>> contextMappings;
private final Map<String, ContextMapping<?>> contextNameMap;

Expand Down Expand Up @@ -124,7 +119,7 @@ private class TypedContextField extends ContextSuggestField {
private final ParseContext.Document document;

TypedContextField(String name, String value, int weight, Map<String, Set<CharSequence>> contexts,
ParseContext.Document document) {
ParseContext.Document document) {
super(name, value, weight);
this.contexts = contexts;
this.document = document;
Expand All @@ -150,8 +145,7 @@ protected Iterable<CharSequence> 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;
}
Expand Down Expand Up @@ -186,8 +180,7 @@ public ContextQuery toContextQuery(CompletionQuery query, Map<String, List<Conte
}
}
if (hasContext == false) {
DEPRECATION_LOGGER.deprecated("The ability to query with no context on a context enabled completion field is deprecated " +
"and will be removed in the next major release.");
throw new IllegalArgumentException("Missing mandatory contexts in context query");
}
return typedContextQuery;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@
package org.elasticsearch.search.suggest;

import com.carrotsearch.randomizedtesting.generators.RandomStrings;

import org.apache.lucene.util.LuceneTestCase.SuppressCodecs;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.geo.GeoHashUtils;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.Fuzziness;
Expand Down Expand Up @@ -95,7 +93,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");
}

Expand Down Expand Up @@ -126,7 +126,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");
}

Expand Down Expand Up @@ -157,7 +159,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");
}

Expand Down Expand Up @@ -236,32 +240,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<String, ContextMapping<?>> map = new LinkedHashMap<>(Collections.singletonMap("cat", contextMapping));
final CompletionMappingBuilder mapping = new CompletionMappingBuilder().context(map);
createIndexAndMapping(mapping);
int numDocs = 10;
List<String> contexts = Arrays.asList("type1", "type2", "type3", "type4");
List<IndexRequestBuilder> 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<String, ContextMapping<?>> map = new LinkedHashMap<>();
map.put("cat", ContextBuilder.category("cat").field("cat").build());
Expand Down Expand Up @@ -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<String, List<? extends ToXContent>> 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)")
Expand Down Expand Up @@ -361,36 +331,6 @@ public void testMultiContextBoosting() throws Exception {
assertSuggestions("foo", multiContextBoostSuggest, "suggestion9", "suggestion6", "suggestion5", "suggestion2", "suggestion1");
}

public void testMissingContextValue() throws Exception {
LinkedHashMap<String, ContextMapping<?>> 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<IndexRequestBuilder> 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<String, ContextMapping<?>> map = new LinkedHashMap<>();
final int numContexts = randomIntBetween(2, 5);
Expand All @@ -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<String, ContextMapping<?>> map = new LinkedHashMap<>();
map.put("geo", ContextBuilder.geo("geo").build());
final CompletionMappingBuilder mapping = new CompletionMappingBuilder().context(map);
createIndexAndMapping(mapping);
int numDocs = 10;
List<IndexRequestBuilder> 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<String, ContextMapping<?>> map = new LinkedHashMap<>();
map.put("geo", ContextBuilder.geo("geo").build());
Expand All @@ -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(
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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())));
Expand Down Expand Up @@ -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<String, List<? extends ToXContent>> 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)
Expand Down

0 comments on commit 584fa26

Please sign in to comment.