Skip all geo point queries in plain highlighter #18495
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Geo queries and plain highlighter do not seem to work well together (https://issues.apache.org/jira/browse/LUCENE-7293) so we need to skip all geo related queries when we highlight. closes elastic#17537
|
LGTM. Highlighter is all hacks so another one won't hurt. Much. |
| protected void extract(Query query, float boost, Map<String, WeightedSpanTerm> terms) throws IOException { | ||
| // skip all geo queries, see https://issues.apache.org/jira/browse/LUCENE-7293 and | ||
| // https://github.com/elastic/elasticsearch/issues/17537 | ||
| if (query instanceof GeoPointInBBoxQuery == false) { |
There was a problem hiding this comment.
Do we also expose GeoPointDistanceQuery or GeoPointInPolygonQuery?
There was a problem hiding this comment.
Sorry, nevermind: both of those queries also subclass GeoPoinInBBoxQuery, so this check is sufficient!
|
I left one comment, else LGTM. Thanks @brwe! |
clintongormley
added
>bug
:Search Relevance/Highlighting
|
LGTM too |
clintongormley
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Geo queries and plain highlighter do not seem to work well
together (https://issues.apache.org/jira/browse/LUCENE-7293)
so we need to skip all geo related queries when we highlight.
hopefully closes #17537
Only plain highlighter seems to be affected.
I am worried that we might need to take care of GeoPointInBBoxQueryImpl too although I do not see how a rewritten query can be used for highlighting, see TODO below.
@nik9000 since this was your idea, maybe you want to take a look?