Skip to content

Completion suggestion should also consider text if prefix/regex is missing #23451

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.

Sign up for GitHub

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 1 commit into from
Mar 16, 2017
Merged

Completion suggestion should also consider text if prefix/regex is missing #23451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 19 additions & 10 deletions ...rc/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.elasticsearch.search.suggest.completion;

import org.apache.lucene.search.suggest.document.CompletionQuery;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.index.mapper.CompletionFieldMapper;
import org.elasticsearch.index.query.QueryShardContext;
Expand Down Expand Up @@ -77,15 +78,7 @@ CompletionQuery toQuery() {
CompletionFieldMapper.CompletionFieldType fieldType = getFieldType();
final CompletionQuery query;
if (getPrefix() != null) {
if (fuzzyOptions != null) {
query = fieldType.fuzzyQuery(getPrefix().utf8ToString(),
Fuzziness.fromEdits(fuzzyOptions.getEditDistance()),
fuzzyOptions.getFuzzyPrefixLength(), fuzzyOptions.getFuzzyMinLength(),
fuzzyOptions.getMaxDeterminizedStates(), fuzzyOptions.isTranspositions(),
fuzzyOptions.isUnicodeAware());
} else {
query = fieldType.prefixQuery(getPrefix());
}
query = createCompletionQuery(getPrefix(), fieldType);
} else if (getRegex() != null) {
if (fuzzyOptions != null) {
throw new IllegalArgumentException("can not use 'fuzzy' options with 'regex");
Expand All @@ -95,8 +88,10 @@ CompletionQuery toQuery() {
}
query = fieldType.regexpQuery(getRegex(), regexOptions.getFlagsValue(),
regexOptions.getMaxDeterminizedStates());
} else if (getText() != null) {
query = createCompletionQuery(getText(), fieldType);
} else {
throw new IllegalArgumentException("'prefix' or 'regex' must be defined");
throw new IllegalArgumentException("'prefix/text' or 'regex' must be defined");
}
if (fieldType.hasContextMappings()) {
ContextMappings contextMappings = fieldType.getContextMappings();
Expand All @@ -105,4 +100,18 @@ CompletionQuery toQuery() {
return query;
}

private CompletionQuery createCompletionQuery(BytesRef prefix, CompletionFieldMapper.CompletionFieldType fieldType) {
final CompletionQuery query;
if (fuzzyOptions != null) {
query = fieldType.fuzzyQuery(prefix.utf8ToString(),
Fuzziness.fromEdits(fuzzyOptions.getEditDistance()),
fuzzyOptions.getFuzzyPrefixLength(), fuzzyOptions.getFuzzyMinLength(),
fuzzyOptions.getMaxDeterminizedStates(), fuzzyOptions.isTranspositions(),
fuzzyOptions.isUnicodeAware());
} else {
query = fieldType.prefixQuery(prefix);
}
return query;
}

}
38 changes: 33 additions & 5 deletions core/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,10 @@
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAllSuccessful;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHit;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasScore;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
Expand Down Expand Up @@ -116,6 +114,36 @@ public void testPrefix() throws Exception {
assertSuggestions("foo", prefix, "suggestion10", "suggestion9", "suggestion8", "suggestion7", "suggestion6");
}

/**
* test that suggestion works if prefix is either provided via {@link CompletionSuggestionBuilder#text(String)} or
* {@link SuggestBuilder#setGlobalText(String)}
*/
public void testTextAndGlobalText() throws Exception {
final CompletionMappingBuilder mapping = new CompletionMappingBuilder();
createIndexAndMapping(mapping);
int numDocs = 10;
List<IndexRequestBuilder> indexRequestBuilders = new ArrayList<>();
for (int i = 1; i <= numDocs; i++) {
indexRequestBuilders.add(client().prepareIndex(INDEX, TYPE, "" + i).setSource(jsonBuilder().startObject().startObject(FIELD)
.field("input", "suggestion" + i).field("weight", i).endObject().endObject()));
}
indexRandom(true, indexRequestBuilders);
CompletionSuggestionBuilder noText = SuggestBuilders.completionSuggestion(FIELD);
SearchResponse searchResponse = client().prepareSearch(INDEX)
.suggest(new SuggestBuilder().addSuggestion("foo", noText).setGlobalText("sugg")).execute().actionGet();
assertSuggestions(searchResponse, "foo", "suggestion10", "suggestion9", "suggestion8", "suggestion7", "suggestion6");

CompletionSuggestionBuilder withText = SuggestBuilders.completionSuggestion(FIELD).text("sugg");
searchResponse = client().prepareSearch(INDEX)
.suggest(new SuggestBuilder().addSuggestion("foo", withText)).execute().actionGet();
assertSuggestions(searchResponse, "foo", "suggestion10", "suggestion9", "suggestion8", "suggestion7", "suggestion6");

// test that suggestion text takes precedence over global text
searchResponse = client().prepareSearch(INDEX)
.suggest(new SuggestBuilder().addSuggestion("foo", withText).setGlobalText("bogus")).execute().actionGet();
assertSuggestions(searchResponse, "foo", "suggestion10", "suggestion9", "suggestion8", "suggestion7", "suggestion6");
}

public void testRegex() throws Exception {
final CompletionMappingBuilder mapping = new CompletionMappingBuilder();
createIndexAndMapping(mapping);
Expand Down Expand Up @@ -217,7 +245,7 @@ public void testSuggestDocument() throws Exception {
for (CompletionSuggestion.Entry.Option option : options) {
assertThat(option.getText().toString(), equalTo("suggestion" + id));
assertSearchHit(option.getHit(), hasId("" + id));
assertSearchHit(option.getHit(), hasScore(((float) id)));
assertSearchHit(option.getHit(), hasScore((id)));
assertNotNull(option.getHit().getSourceAsMap());
id--;
}
Expand Down Expand Up @@ -252,7 +280,7 @@ public void testSuggestDocumentNoSource() throws Exception {
for (CompletionSuggestion.Entry.Option option : options) {
assertThat(option.getText().toString(), equalTo("suggestion" + id));
assertSearchHit(option.getHit(), hasId("" + id));
assertSearchHit(option.getHit(), hasScore(((float) id)));
assertSearchHit(option.getHit(), hasScore((id)));
assertNull(option.getHit().getSourceAsMap());
id--;
}
Expand Down Expand Up @@ -289,7 +317,7 @@ public void testSuggestDocumentSourceFiltering() throws Exception {
for (CompletionSuggestion.Entry.Option option : options) {
assertThat(option.getText().toString(), equalTo("suggestion" + id));
assertSearchHit(option.getHit(), hasId("" + id));
assertSearchHit(option.getHit(), hasScore(((float) id)));
assertSearchHit(option.getHit(), hasScore((id)));
assertNotNull(option.getHit().getSourceAsMap());
Set<String> sourceFields = option.getHit().getSourceAsMap().keySet();
assertThat(sourceFields, contains("a"));
Expand Down