Skip to content

Commit

Permalink
Fix inaccurate total hit count in _search template api (#53155)
Browse files Browse the repository at this point in the history
When 'rest_track_total_hits_as_int' is set to true, the total hits count in the response should be accurate. So we should set trackTotalHits to true if need when parsing the inline script of a search template request.

Closes #52801
  • Loading branch information
gaobinlong committed Mar 13, 2020
1 parent a808010 commit fb158df
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
searchTemplateRequest = SearchTemplateRequest.fromXContent(parser);
}
searchTemplateRequest.setRequest(searchRequest);
RestSearchAction.checkRestTotalHits(request, searchRequest);

return channel -> client.execute(SearchTemplateAction.INSTANCE, searchTemplateRequest, new RestStatusToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.rest.action.search.RestSearchAction;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.script.TemplateScript;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.transport.TransportService;

Expand Down Expand Up @@ -110,8 +112,27 @@ static SearchRequest convert(SearchTemplateRequest searchTemplateRequest, Search
builder.parseXContent(parser, false);
builder.explain(searchTemplateRequest.isExplain());
builder.profile(searchTemplateRequest.isProfile());
checkRestTotalHitsAsInt(searchRequest, builder);
searchRequest.source(builder);
}
return searchRequest;
}

private static void checkRestTotalHitsAsInt(SearchRequest searchRequest, SearchSourceBuilder searchSourceBuilder) {
if (searchRequest.source() == null) {
searchRequest.source(new SearchSourceBuilder());
}
Integer trackTotalHitsUpTo = searchRequest.source().trackTotalHitsUpTo();
// trackTotalHitsUpTo is set to Integer.MAX_VALUE when `rest_total_hits_as_int` is true
if (trackTotalHitsUpTo != null) {
if (searchSourceBuilder.trackTotalHitsUpTo() == null) {
// trackTotalHitsUpTo should be set here, ensure that we can get an accurate total hits count
searchSourceBuilder.trackTotalHitsUpTo(trackTotalHitsUpTo);
} else if (searchSourceBuilder.trackTotalHitsUpTo() != SearchContext.TRACK_TOTAL_HITS_ACCURATE
&& searchSourceBuilder.trackTotalHitsUpTo() != SearchContext.TRACK_TOTAL_HITS_DISABLED) {
throw new IllegalArgumentException("[" + RestSearchAction.TOTAL_HITS_AS_INT_PARAM + "] cannot be used " +
"if the tracking of total hits is not accurate, got " + searchSourceBuilder.trackTotalHitsUpTo());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@

- match: { hits.total: 2 }

---
"Test with invalid track_total_hits":

- do:
catch: bad_request
search_template:
rest_total_hits_as_int: true
body: { "source" : { "query": { "match_{{template}}": {} }, "track_total_hits": "{{trackTotalHits}}" }, "params" : { "template" : "all", "trackTotalHits" : 1 } }

- match: { status: 400 }
- match: { error.type: illegal_argument_exception }
- match: { error.reason: "[rest_total_hits_as_int] cannot be used if the tracking of total hits is not accurate, got 1" }

---
"Missing template search request":

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,21 @@ setup:
- source: '{"query": {"{{query_type}}": {} }' # Unknown query type
params:
query_type: "unknown"
# Search 4 has an unsupported track_total_hits
- index: index_*
- source: '{"query": {"match_all": {} }, "track_total_hits": "{{trackTotalHits}}" }'
params:
trackTotalHits: 1

- match: { responses.0.hits.total: 2 }
- match: { responses.1.error.root_cause.0.type: json_e_o_f_exception }
- match: { responses.1.error.root_cause.0.reason: "/Unexpected.end.of.input/" }
- match: { responses.2.hits.total: 1 }
- match: { responses.3.error.root_cause.0.type: parsing_exception }
- match: { responses.3.error.root_cause.0.reason: "/unknown.query.\\[unknown\\]/" }
- match: { responses.4.error.root_cause.0.type: illegal_argument_exception }
- match: { responses.4.error.root_cause.0.reason: "[rest_total_hits_as_int] cannot be used if the tracking of total hits is not accurate, got 1" }


---
"Multi-search template with invalid request":
Expand Down

0 comments on commit fb158df

Please sign in to comment.