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.

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

Make hits.total an object in the search response #35849

Merged
merged 58 commits into from
Dec 5, 2018

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Nov 23, 2018

This commit changes the format of the hits.total in the search response to be an object with
a value and a relation. The value indicates the number of hits that match the query and the
relation indicates whether the number is accurate (in which case the relation is equals to eq)
or a lower bound of the total (in which case it is equals to gte).
This change also adds a parameter called rest_total_hits_as_int that can be used in the
search APIs to opt out from this change (retrieve the total hits as a number in the rest response).
Note that currently all search responses are accurate (track_total_hits: true) or they don't contain
hits.total (track_total_hits: true). We'll add a way to get a lower bound of the total hits in a
follow up (to allow numbers to be passed to track_total_hits).

Relates #33028

@jimczi jimczi added >feature >breaking :Search/Search Search-related issues that do not fall into other categories >breaking-java v7.0.0 labels Nov 23, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@jimczi
Copy link
Contributor Author

jimczi commented Nov 23, 2018

Note that this pr depends on #35848 to pass the BWC rest tests.

@jimczi jimczi requested a review from jpountz November 23, 2018 10:57
@jimczi
Copy link
Contributor Author

jimczi commented Nov 23, 2018

test this please

This commit changes the format of the `hits.total` in the search response to be an object with
a `value` and a `relation`. The `value` indicates the number of hits that match the query and the
`relation` indicates whether the number is accurate (in which case the relation is equals to `eq`)
or a lower bound of the total (in which case it is equals to `gte`).
This change also adds a parameter called `rest_total_hits_as_int` that can be used in the
search APIs to opt out from this change (retrieve the total hits as a number in the rest response).
Note that currently all search responses are accurate (`track_total_hits: true`) or they don't contain
`hits.total` (`track_total_hits: true`). We'll add a way to get a lower bound of the total hits in a
follow up (to allow numbers to be passed to `track_total_hits`).

Relates elastic#33028
@jimczi
Copy link
Contributor Author

jimczi commented Dec 5, 2018

@elasticmachine run the gradle build tests 2

@jimczi
Copy link
Contributor Author

jimczi commented Dec 5, 2018

run sample packaging tests

@jimczi
Copy link
Contributor Author

jimczi commented Dec 5, 2018

run default distro tests

@jimczi
Copy link
Contributor Author

jimczi commented Dec 5, 2018

run the gradle build tests 2

1 similar comment
@jimczi
Copy link
Contributor Author

jimczi commented Dec 5, 2018

run the gradle build tests 2

@jimczi jimczi merged commit 18866c4 into elastic:master Dec 5, 2018
@jimczi jimczi deleted the total_hits_as_an_object branch December 5, 2018 18:49
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Dec 6, 2018
* master: (133 commits)
  SNAPSHOT: Increase Timeout to Stabilize Test (elastic#36294)
  Fix get certificates HLRC API (elastic#36198)
  Avoid shutting down the only master (elastic#36272)
  Fix typo in comment
  Fix total hits serialization of the search response (elastic#36290)
  Fix FullClusterRestartIT#testRollupIDSchemeAfterRestart
  Mute FullClusterRestartIT#testRollupIDSchemeAfterRestart as we await a fix.
  [Docs] Add Profile API limitations (elastic#36252)
  Make sure test don't use Math.random for reproducability (elastic#36241)
  Fix compilation
  ingest: support default pipeline through an alias (elastic#36231)
  Zen2: Rename tombstones to exclusions (elastic#36226)
  [Zen2] Hide not recovered state (elastic#36224)
  Test: mute testDataNodeRestartWithBusyMasterDuringSnapshot
  Test: mute testSnapshotAndRestoreWithNested
  Revert "Test: mute failing mtermvector rest test"
  Test: mute failing mtermvector rest test
  add version 6.5.3 (elastic#36268)
  Make hits.total an object in the search response (elastic#35849)
  [DOCS] Fixes broken link in execute watch
  ...
robbavey added a commit to robbavey/logstash-output-elasticsearch that referenced this pull request Dec 12, 2018
elastic/elasticsearch#35849 changed the search response
format, which broke a bunch of tests.
elasticsearch-bot pushed a commit to logstash-plugins/logstash-output-elasticsearch that referenced this pull request Dec 13, 2018
elastic/elasticsearch#35849 changed the search response
format, which broke a bunch of tests.

Fixes #817
@Musfiqur01
Copy link

Will not this break the Rest api in 6.x while the upgrade is happening? We are using the java rest api 6.7 and I did not find any way of setting track_total_hits=true and rest_total_hits_as_int=true.

@jimczi
Copy link
Contributor Author

jimczi commented Aug 20, 2020

@Musfiqur01, see #46076, the Java client sets this option automatically from version 6.8.3.

@Musfiqur01
Copy link

Musfiqur01 commented Aug 20, 2020 via email

@Musfiqur01
Copy link

From 46076, it appears that the change is only sending rest_total_hit_as_int = true. Is it also setting track_total_hits=true?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking >breaking-java >feature :Search/Search Search-related issues that do not fall into other categories v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants