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

Don't index ranges including NOW in percolator #52748

Merged
merged 3 commits into from
Feb 25, 2020

Conversation

romseygeek
Copy link
Contributor

Currently, date ranges queries using NOW-based date math are rewritten to
MatchAllDocs queries when being preprocessed for the percolator. However,
since we added the verification step, this can result in incorrect matches when
percolator queries are run without scores. This commit changes things to instead
wrap date queries that use NOW with a new DateRangeIncludingNowQuery.
This is a simple wrapper query that returns its delegate at rewrite time, but it can
be detected by the percolator QueryAnalyzer and be dealt with accordingly.

This also allows us to remove a method on QueryRewriteContext, and push all
logic relating to NOW-based ranges into the DateFieldMapper.

Fixes #52617

@romseygeek romseygeek added >bug :Search Relevance/Percolator Reverse search: find queries that match a document v8.0.0 v7.7.0 labels Feb 25, 2020
@romseygeek romseygeek self-assigned this Feb 25, 2020
@elasticmachine
Copy link
Collaborator

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

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.

Date ranges based on current time are tricky to handle and
this isn't the first time bugs are reported about it. The approach
in this pr is easier to reason about it.

@romseygeek romseygeek merged commit 2a95ecb into elastic:master Feb 25, 2020
romseygeek added a commit that referenced this pull request Feb 25, 2020
Currently, date ranges queries using NOW-based date math are rewritten to
MatchAllDocs queries when being preprocessed for the percolator. However,
since we added the verification step, this can result in incorrect matches when
percolator queries are run without scores. This commit changes things to instead
wrap date queries that use NOW with a new DateRangeIncludingNowQuery.
This is a simple wrapper query that returns its delegate at rewrite time, but it can
be detected by the percolator QueryAnalyzer and be dealt with accordingly.

This also allows us to remove a method on QueryRewriteContext, and push all
logic relating to NOW-based ranges into the DateFieldMapper.

Fixes #52617
romseygeek added a commit that referenced this pull request Feb 26, 2020
Commit #52748 fixed a bug where percolate queries wrapped in a constant score
could report incorrect matches. This commit adds a test to check that it also fixes
the case where a percolate query is sorted by something other than score.

Closes #52618
romseygeek added a commit that referenced this pull request Feb 26, 2020
Commit #52748 fixed a bug where percolate queries wrapped in a constant score
could report incorrect matches. This commit adds a test to check that it also fixes
the case where a percolate query is sorted by something other than score.

Closes #52618
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search Relevance/Percolator Reverse search: find queries that match a document v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Constant score percolate query incorrectly matching date range query
4 participants