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

Enforce no-op setWeight on collectors that filter docs out #94886

Merged
merged 6 commits into from
Mar 31, 2023

Conversation

javanna
Copy link
Member

@javanna javanna commented Mar 30, 2023

MinimumScoreCollector and FilteredCollector filter documents out as part of their collection. The have an inner collector to delegate to, but they should never propagate the Weight to them otherwise the total hit count may not reflect the filtering. This commit clarifies this through an empty final setWeight method on both collectors and additional javadocs.

MinimumScoreCollector and FilteredCollector filter documents out as part
of their collection. The have an inner collector to delegate to, but
they should never propagate the Weight to them otherwise the total hit
count may not reflect the filtering. This commit clarifies this through
an empty final setWeight method on both collectors and additional
javadocs.
@javanna javanna added :Search/Search Search-related issues that do not fall into other categories >refactoring v8.8.0 labels Mar 30, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Mar 30, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@javanna
Copy link
Member Author

javanna commented Mar 30, 2023

run elasticsearch-ci/bwc

Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM.

@javanna
Copy link
Member Author

javanna commented Mar 30, 2023

These collectors did not have any existing unit tests, so I added simple ones where we can now also test that weight does not get propagated. This way we catch it if it ever happens. There is already a more end-to-end test for this in QueryPhaseTests but these are more specific to the individual collectors.

@javanna
Copy link
Member Author

javanna commented Mar 30, 2023

run elasticsearch-ci/bwc

@javanna javanna merged commit 4cf9b0c into elastic:main Mar 31, 2023
@javanna javanna deleted the docs/filtered_collector_set_weight branch March 31, 2023 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>refactoring :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants