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

SQL: Removed the always on total hit tracking #70319

Merged
merged 1 commit into from Mar 15, 2021

Conversation

palesz
Copy link
Contributor

@palesz palesz commented Mar 11, 2021

If the query uses COUNT(*) the total hits are automatically tracked:

Otherwise the track_total_hits is only useful for scrolling.

If the query is translated to:

  • a scroll query (all the non-agg only queries), ES will automatically provide accurate total hits
    without us having to flip the track_total_hits
  • agg-only query: implicit group by (result is single record) or composite aggregation.
    track_total_hits does not affect these queries, it can only affect document search hits

Closes #52787

If the query uses `COUNT(*)` the total hits are automatically tracked:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java#L639

Otherwise the `track_total_hits` is only useful for scrolling.

If the query is translated to (https://github.com/elastic/elasticsearch/blob/ee5cc5442a8e01d7d8fa426748d0b086959bf99d/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java#L130):
- a scroll query (all the non-agg only queries), ES will automatically provide accurate total hits
without us having to flip the `track_total_hits`
- agg-only query: implicit group by (result is single record) or composite aggregation.
`track_total_hits` does not affect these queries, it can only affect document search hits

Closes elastic#52787
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@bpintea bpintea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
(was scheduled for slashing since a while back).

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@palesz palesz merged commit 4c7c378 into elastic:master Mar 15, 2021
palesz pushed a commit to palesz/elasticsearch that referenced this pull request Mar 15, 2021
If the query uses `COUNT(*)` the total hits are automatically tracked:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java#L639

Otherwise, the `track_total_hits` is only useful for scrolling.

If the query is translated to:
- a scroll query (all the non-agg only queries), ES will automatically provide accurate total hits
without us having to flip the `track_total_hits`
- agg-only query, the `track_total_hits` does not affect these queries (implicit group by (result 
is single record) or composite aggregation), it can only affect document search hits

See translation at: https://github.com/elastic/elasticsearch/blob/ee5cc5442a8e01d7d8fa426748d0b086959bf99d/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java#L130

Closes elastic#52787
dakrone pushed a commit to dakrone/elasticsearch that referenced this pull request Mar 15, 2021
If the query uses `COUNT(*)` the total hits are automatically tracked:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java#L639

Otherwise, the `track_total_hits` is only useful for scrolling.

If the query is translated to:
- a scroll query (all the non-agg only queries), ES will automatically provide accurate total hits
without us having to flip the `track_total_hits`
- agg-only query, the `track_total_hits` does not affect these queries (implicit group by (result 
is single record) or composite aggregation), it can only affect document search hits

See translation at: https://github.com/elastic/elasticsearch/blob/ee5cc5442a8e01d7d8fa426748d0b086959bf99d/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java#L130

Closes elastic#52787
@palesz palesz deleted the total-hit-tracking branch March 16, 2021 20:05
palesz pushed a commit that referenced this pull request Mar 16, 2021
If the query uses `COUNT(*)` the total hits are automatically tracked:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java#L639

Otherwise, the `track_total_hits` is only useful for scrolling.

If the query is translated to:
- a scroll query (all the non-agg only queries), ES will automatically provide accurate total hits
without us having to flip the `track_total_hits`
- agg-only query, the `track_total_hits` does not affect these queries (implicit group by (result 
is single record) or composite aggregation), it can only affect document search hits

See translation at: https://github.com/elastic/elasticsearch/blob/ee5cc5442a8e01d7d8fa426748d0b086959bf99d/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java#L130

Closes #52787
easyice pushed a commit to easyice/elasticsearch that referenced this pull request Mar 25, 2021
If the query uses `COUNT(*)` the total hits are automatically tracked:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java#L639

Otherwise, the `track_total_hits` is only useful for scrolling.

If the query is translated to:
- a scroll query (all the non-agg only queries), ES will automatically provide accurate total hits
without us having to flip the `track_total_hits`
- agg-only query, the `track_total_hits` does not affect these queries (implicit group by (result 
is single record) or composite aggregation), it can only affect document search hits

See translation at: https://github.com/elastic/elasticsearch/blob/ee5cc5442a8e01d7d8fa426748d0b086959bf99d/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java#L130

Closes elastic#52787
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SQL: track total hits seems to always be enabled
6 participants