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.

Sign up for GitHub

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

Jump to bottom

Highlighting should leverage the new field retrieval API #59931

Closed
jimczi opened this issue Jul 20, 2020 · 3 comments · Fixed by #63572
Closed

Highlighting should leverage the new field retrieval API #59931

jimczi opened this issue Jul 20, 2020 · 3 comments · Fixed by #63572
Labels
>enhancement :Search Relevance/Highlighting How a query matched a document Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch

Comments

@jimczi
Copy link
Contributor

jimczi commented Jul 20, 2020

In #55363 we're adding a core functionality to retrieve field content from the mappings. Today highlighting is limited to fields that are contained in the _source or stored explicitly. Switching to the new API would allow to highlight alias, copy_to and multi-fields seamlessly without requiring the targeted field to be included in the _source. As such, this issue can be seen as a follow up of #55363, for internal usage we should use this generic API since it provides a nice abstraction for field retrieval in general. Note that this issue is different than #5172 which is an high hanging fruit and would require changes in the highlighter.
@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jul 20, 2020
@jimczi
Copy link
Contributor Author

jimczi commented Jul 22, 2020

Highlighting already work for multi-fields and aliases so the switch would only be helpful to retrieve the content of copied fields (the source of the copy_to). Sorry for the confusion. However, that alone would be a nice addition since we don't want users to rely on stored fields just for highlighting.

@jtibshirani
Copy link
Contributor

I really like that this change would reduce the need for stored fields -- users sometimes set store: true on a text field just so highlighting will capture the copy_to content.

@weltenwort weltenwort mentioned this issue Aug 13, 2020
Closed
@weltenwort weltenwort mentioned this issue Sep 2, 2020
Closed
@jtibshirani jtibshirani mentioned this issue Oct 14, 2020
Merged
romseygeek added a commit that referenced this issue Nov 24, 2020
@romseygeek
Use ValueFetcher when loading text snippets to highlight (#63572)
d088171 
HighlighterUtils.loadFieldValues() loads values directly from the source, and
then callers have to deal with filtering out values that would have been removed
by an ignore_above filter on keyword fields. Instead, we can use the
ValueFetcher for the relevant field, which handles all this logic for us.

Closes #59931.
romseygeek added a commit to romseygeek/elasticsearch that referenced this issue Nov 24, 2020
@romseygeek
Use ValueFetcher when loading text snippets to highlight (elastic#63572)
f5e92be 
HighlighterUtils.loadFieldValues() loads values directly from the source, and
then callers have to deal with filtering out values that would have been removed
by an ignore_above filter on keyword fields. Instead, we can use the
ValueFetcher for the relevant field, which handles all this logic for us.

Closes elastic#59931.
@romseygeek romseygeek mentioned this issue Nov 24, 2020
Merged
@jtibshirani jtibshirani mentioned this issue Nov 24, 2020
Merged
romseygeek added a commit that referenced this issue Nov 25, 2020
@romseygeek
Use ValueFetcher when loading text snippets to highlight (#63572) (#6…
92e4b15 
…5441)

HighlighterUtils.loadFieldValues() loads values directly from the source, and
then callers have to deal with filtering out values that would have been removed
by an ignore_above filter on keyword fields. Instead, we can use the
ValueFetcher for the relevant field, which handles all this logic for us.

Closes #59931.
@javanna javanna added Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch and removed Team:Search Meta label for search team labels Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
>enhancement :Search Relevance/Highlighting How a query matched a document Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use ValueFetcher when loading text snippets to highlight romseygeek/elasticsearch
4 participants
@javanna @jtibshirani @elasticmachine @jimczi