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

Skip segment for MatchNoDocsQuery filters #98295

Merged
merged 6 commits into from Aug 10, 2023

Conversation

kkrik-es
Copy link
Contributor

@kkrik-es kkrik-es commented Aug 8, 2023

When a query of a filter gets rewritten to MatchNoDocsQuery, segments will not produce any results. We can therefore skip processing such segments.

This applies to FilterByFilterAggregator and FiltersAggregator, as well as to TermsAggregator when it uses StringTermsAggregatorFromFilters internally; the latter is an adapter aggregator wrapping FilterByFilterAggregator.

Fixes #94637

When a query of a filter gets rewritten to MatchNoDocsQuery, segments
will not produce any results. We can therefore skip processing such
segments.

This applies to FilterByFilterAggregator and FiltersAggregator, as well
as to TermsAggregator when it uses StringTermsAggregatorFromFilters
internally; the latter is an adapter aggregator to
FilterByFilterAggregator.

Fixes elastic#94637
@elasticsearchmachine
Copy link
Collaborator

Hi @kkrik-es, I've created a changelog YAML for you.

@kkrik-es kkrik-es added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Aug 8, 2023
@kkrik-es
Copy link
Contributor Author

kkrik-es commented Aug 8, 2023

@elasticsearchmachine run elasticsearch-ci/part-2

@kkrik-es kkrik-es marked this pull request as ready for review August 8, 2023 18:03
@kkrik-es kkrik-es requested a review from nik9000 August 8, 2023 18:03
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

Looks good, left a question.

@kkrik-es
Copy link
Contributor Author

kkrik-es commented Aug 9, 2023

@elasticsearchmachine run elasticsearch-ci/part-3

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

lgtm

@kkrik-es
Copy link
Contributor Author

@elasticsearchmachine run elasticsearch-ci/part-3

@kkrik-es kkrik-es added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Aug 10, 2023
@kkrik-es
Copy link
Contributor Author

The optimization doesn't apply when other bucket is added to the filter spec. Good thing that a test caught this, added a yaml test too.

@elasticsearchmachine elasticsearchmachine merged commit 693ab3d into elastic:main Aug 10, 2023
12 checks passed
@kkrik-es kkrik-es deleted the fix/94637 branch August 10, 2023 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aggs and constant_keyword
3 participants