Skip to content

Deprecates indexing and querying a context completion field without context #30712

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

Merged
merged 5 commits into from
May 31, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 3 additions & 3 deletions docs/reference/search/suggesters/context-suggest.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ POST place/_search?pretty
// CONSOLE
// TEST[continued]

NOTE: When no categories are provided at query-time, all indexed documents are considered.
Querying with no categories on a category enabled completion field should be avoided, as it
will degrade search performance.
Note: deprecated[7.0.0, When no categories are provided at query-time, all indexed documents are considered.
Querying with no categories on a category enabled completion field is deprecated and will be removed in the next major release
as it degrades search performance considerably.]

Suggestions with certain categories can be boosted higher than others.
The following filters suggestions by categories and additionally boosts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,16 +336,36 @@ setup:
- length: { suggest.result.0.options: 1 }
- match: { suggest.result.0.options.0.text: "foo" }

---
"Querying without contexts is deprecated":
- skip:
version: " - 6.99.99"
reason: this feature was deprecated in 7.0
features: "warnings"

- do:
index:
index: test
type: test
id: 1
body:
suggest_context:
input: "foo"
contexts:
color: "red"

- do:
indices.refresh: {}

- do:
search:
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:
body:
suggest:
result:
text: "foo"
completion:
skip_duplicates: true
field: suggest_context

- length: { suggest.result: 1 }
- length: { suggest.result.0.options: 1 }
- match: { suggest.result.0.options.0.text: "foo" }
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,14 @@ setup:

---
"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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
Expand Down Expand Up @@ -57,6 +59,10 @@
* indexing.
*/
public class CompletionSuggestionBuilder extends SuggestionBuilder<CompletionSuggestionBuilder> {

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

private static final XContentType CONTEXT_BYTES_XCONTENT_TYPE = XContentType.JSON;
static final String SUGGESTION_NAME = "completion";
static final ParseField CONTEXTS_FIELD = new ParseField("contexts", "context");
Expand Down Expand Up @@ -298,16 +304,19 @@ public SuggestionContext build(QueryShardContext context) throws IOException {
if (mappedFieldType == null || mappedFieldType instanceof CompletionFieldMapper.CompletionFieldType == false) {
throw new IllegalArgumentException("Field [" + suggestionContext.getField() + "] is not a completion suggest field");
}
if (mappedFieldType instanceof CompletionFieldMapper.CompletionFieldType) {
CompletionFieldMapper.CompletionFieldType type = (CompletionFieldMapper.CompletionFieldType) mappedFieldType;
suggestionContext.setFieldType(type);
if (type.hasContextMappings() && contextBytes != null) {
CompletionFieldMapper.CompletionFieldType type = (CompletionFieldMapper.CompletionFieldType) mappedFieldType;
suggestionContext.setFieldType(type);
if (type.hasContextMappings()) {
if (contextBytes == null) {
Copy link
Member

Choose a reason for hiding this comment

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

This checks for a missing contexts element in the query, but I think it will miss cases like "contexts" : {} or "contexts" : { "someField : [] } which should also emit a deprecation warning. Not sure if this means there should be another check after the queryContexts map is parsed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks ! This also made me realize that we should deprecate the index side if no contexts are provided. This is needed because these suggestions are not retrievable anymore if we remove the ability to search with no context. I pushed f5edec4 to handle all cases of empty context in the query side and to also deprecate indexing without contexts.

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.");
} else {
Map<String, List<ContextMapping.InternalQueryContext>> queryContexts = parseContextBytes(contextBytes,
context.getXContentRegistry(), type.getContextMappings());
context.getXContentRegistry(), type.getContextMappings());
suggestionContext.setQueryContexts(queryContexts);
} else if (contextBytes != null) {
throw new IllegalArgumentException("suggester [" + type.name() + "] doesn't expect any context");
}
} else if (contextBytes != null) {
throw new IllegalArgumentException("suggester [" + type.name() + "] doesn't expect any context");
}
assert suggestionContext.getFieldType() != null : "no completion field type set";
return suggestionContext;
Expand Down