Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the ability to index or query context suggestions without context #31007

Merged
merged 6 commits into from
Jul 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to have DEPRECATION_LOGGER variable in this module?

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