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

Highlighters skip ignored keyword values #53408

Merged

Conversation

mayya-sharipova
Copy link
Contributor

@mayya-sharipova mayya-sharipova commented Mar 11, 2020

Keyword field values with length more than ignore_above are not
indexed. But highlighters still were retrieving these values
from _source and were trying to highlight them. This sometimes lead to
errors if a field length exceeded max_analyzed_offset. But also this
is a wrong behaviour to attempt to highlight something that was
ignored during indexing.

This PR checks if a keyword value was ignored for indexing because of its
length, and if yes, skips highlighting it.

Closes #43800

@mayya-sharipova mayya-sharipova added the :Search Relevance/Highlighting How a query matched a document label Mar 11, 2020
@elasticmachine
Copy link
Collaborator

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

@mayya-sharipova
Copy link
Contributor Author

TODO: for backport to 7.7, update breaking changes docs

Keyword field values with length more than ignore_above are not
indexed. But highlighters still were retrieving these values
from _source and were trying to highlight them. This sometimes lead to
errors if a field length exceeded  max_analyzed_offset. But also this
is a wrong behaviour to attempt to highlight something that was not
ignored during indexing.

This PR checks if a keyword value was ignored because of its length,
and if yes, skips highlighting it.

Closes elastic#43800
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

+1 to handle ignore_above, it bugs me a bit that we're re-analizing the field knowing that it's a keyword but that's out of scope for this pr. LGTM.

@mayya-sharipova mayya-sharipova merged commit 01eee1a into elastic:master Mar 16, 2020
@mayya-sharipova mayya-sharipova deleted the highlighter_ignore_above branch March 16, 2020 10:49
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this pull request Mar 16, 2020
Keyword field values with length more than ignore_above are not
indexed. But highlighters still were retrieving these values
from _source and were trying to highlight them. This sometimes lead to
errors if a field length exceeded  max_analyzed_offset. But also this
is an overall wrong behaviour to attempt to highlight something that was
ignored during indexing.

This PR checks if a keyword value was ignored because of its length,
and if yes, skips highlighting it.

Closes elastic#43800
mayya-sharipova added a commit that referenced this pull request Mar 16, 2020
Keyword field values with length more than ignore_above are not
indexed. But highlighters still were retrieving these values
from _source and were trying to highlight them. This sometimes lead to
errors if a field length exceeded  max_analyzed_offset. But also this
is an overall wrong behaviour to attempt to highlight something that was
ignored during indexing.

This PR checks if a keyword value was ignored because of its length,
and if yes, skips highlighting it.

Backport: #53408
Closes #43800
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this pull request Jul 14, 2020
This was supposed to be done after the backport but was missed.

Related to elastic#53408
mayya-sharipova added a commit that referenced this pull request Jul 14, 2020
This was supposed to be done after the backport but was missed.

Related to #53408
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Highlighters should reject ignored keyword field early
4 participants