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.

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

Skip explain phase when only suggestions are requested #41739

Merged
merged 1 commit into from
May 6, 2019
Merged

Skip explain phase when only suggestions are requested #41739

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
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/search/fetch/subphase/ExplainFetchSubPhase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public final class ExplainFetchSubPhase implements FetchSubPhase {

@Override
public void hitExecute(SearchContext context, HitContext hitContext) {
if (context.explain() == false) {
if (context.explain() == false || context.hasOnlySuggest()) {
return;
}
try {
Expand Down
1 change: 0 additions & 1 deletion server/src/main/java/org/elasticsearch/search/query/QueryPhase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ public void preProcess(SearchContext context) {
public void execute(SearchContext searchContext) throws QueryPhaseExecutionException {
if (searchContext.hasOnlySuggest()) {
suggestPhase.execute(searchContext);
// TODO: fix this once we can fetch docs for suggestions
searchContext.queryResult().topDocs(new TopDocsAndMaxScore(
new TopDocs(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Lucene.EMPTY_SCORE_DOCS), Float.NaN),
new DocValueFormat[0]);
Expand Down
120 changes: 61 additions & 59 deletions server/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 @@ -379,17 +379,13 @@ public void testThatWeightsAreWorking() throws Exception {
public void testThatWeightMustBeAnInteger() throws Exception {
createIndexAndMapping(completionMappingBuilder);

try {
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("sth").endArray()
.field("weight", 2.5)
.endObject().endObject()
).get();
fail("Indexing with a float weight was successful, but should not be");
} catch (MapperParsingException e) {
assertThat(e.toString(), containsString("2.5"));
}
MapperParsingException e = expectThrows(MapperParsingException.class,
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("sth").endArray()
.field("weight", 2.5)
.endObject().endObject()).get());
assertThat(e.toString(), containsString("2.5"));
}

public void testThatWeightCanBeAString() throws Exception {
Expand Down Expand Up @@ -422,34 +418,28 @@ public void testThatWeightCanBeAString() throws Exception {
public void testThatWeightMustNotBeANonNumberString() throws Exception {
createIndexAndMapping(completionMappingBuilder);

try {
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("sth").endArray()
.field("weight", "thisIsNotValid")
.endObject().endObject()
).get();
fail("Indexing with a non-number representing string as weight was successful, but should not be");
} catch (MapperParsingException e) {
assertThat(e.toString(), containsString("thisIsNotValid"));
}
MapperParsingException e = expectThrows(MapperParsingException.class,
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("sth").endArray()
.field("weight", "thisIsNotValid")
.endObject().endObject()
).get());
assertThat(e.toString(), containsString("thisIsNotValid"));
}

public void testThatWeightAsStringMustBeInt() throws Exception {
createIndexAndMapping(completionMappingBuilder);

String weight = String.valueOf(Long.MAX_VALUE - 4);
try {
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("testing").endArray()
.field("weight", weight)
.endObject().endObject()
).get();
fail("Indexing with weight string representing value > Int.MAX_VALUE was successful, but should not be");
} catch (MapperParsingException e) {
assertThat(e.toString(), containsString(weight));
}

MapperParsingException e = expectThrows(MapperParsingException.class,
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("testing").endArray()
.field("weight", weight)
.endObject().endObject()).get());
assertThat(e.toString(), containsString(weight));
}

public void testThatInputCanBeAStringInsteadOfAnArray() throws Exception {
Expand Down Expand Up @@ -821,13 +811,11 @@ public void testThatSortingOnCompletionFieldReturnsUsefulException() throws Exce
).get();

refresh();
try {
client().prepareSearch(INDEX).setTypes(TYPE).addSort(new FieldSortBuilder(FIELD)).get();
fail("Expected an exception due to trying to sort on completion field, but did not happen");
} catch (SearchPhaseExecutionException e) {
assertThat(e.status().getStatus(), is(400));
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
}

SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class,
() -> client().prepareSearch(INDEX).setTypes(TYPE).addSort(new FieldSortBuilder(FIELD)).get());
assertThat(e.status().getStatus(), is(400));
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
}

public void testThatSuggestStopFilterWorks() throws Exception {
Expand Down Expand Up @@ -1118,17 +1106,12 @@ public void testReservedChars() throws IOException {
.endObject()).get());
// can cause stack overflow without the default max_input_length
String string = "foo" + (char) 0x00 + "bar";
try {
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value(string).endArray()
.field("output", "foobar")
.endObject().endObject()
).get();
fail("Expected MapperParsingException");
} catch (MapperParsingException e) {
assertThat(e.getMessage(), containsString("failed to parse"));
}
MapperParsingException e = expectThrows(MapperParsingException.class,
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder().startObject().startObject(FIELD)
.startArray("input").value(string).endArray()
.field("output", "foobar")
.endObject().endObject()).get());
assertThat(e.getMessage(), containsString("failed to parse"));
}

// see #5930
Expand All @@ -1147,14 +1130,10 @@ public void testIssue5930() throws IOException {
.endObject()
).setRefreshPolicy(IMMEDIATE).get();

try {
client().prepareSearch(INDEX).addAggregation(AggregationBuilders.terms("suggest_agg").field(FIELD)
.collectMode(randomFrom(SubAggCollectionMode.values()))).get();
// Exception must be thrown
assertFalse(true);
} catch (SearchPhaseExecutionException e) {
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
}
SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class,
() -> client().prepareSearch(INDEX).addAggregation(AggregationBuilders.terms("suggest_agg").field(FIELD)
.collectMode(randomFrom(SubAggCollectionMode.values()))).get());
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
}

public void testMultiDocSuggestions() throws Exception {
Expand Down Expand Up @@ -1205,6 +1184,29 @@ public void testSuggestWithFieldAlias() throws Exception {
assertSuggestions("suggestion", suggestionBuilder, "apple");
}

public void testSuggestOnlyExplain() 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 prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg");
SearchResponse searchResponse = client().prepareSearch(INDEX).setExplain(true)
.suggest(new SuggestBuilder().addSuggestion("foo", prefix)).get();
assertSuggestions(searchResponse, "foo", "suggestion10", "suggestion9", "suggestion8", "suggestion7", "suggestion6");
}

public static boolean isReservedChar(char c) {
switch (c) {
case '\u001F':
Expand Down