-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Fix inaccurate total hit count in _search/template api #53155
Conversation
.../lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java
Show resolved
Hide resolved
...-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java
Outdated
Show resolved
Hide resolved
Pinging @elastic/es-search (:Search/Search) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gaobinlong , the fix looks good to me. I left some comments regarding the moving of tests to a more appropriate place (yaml rest tests).
.../lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java
Show resolved
Hide resolved
...-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java
Outdated
Show resolved
Hide resolved
...-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java
Outdated
Show resolved
Hide resolved
modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java
Outdated
Show resolved
Hide resolved
...-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java
Outdated
Show resolved
Hide resolved
@jimczi, thanks for your review, can you take a look at the new commit I have pushed? |
@elasticmachine ok to test |
@elasticmachine update branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks @gaobinlong .
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
Relates to #52801.
The main changes are:
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.Add some test code in MultiSearchTemplateIT and SearchTemplateIT.