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

UnifiedHighlighter highlight on multiple fields #13268

Merged

Conversation

mayya-sharipova
Copy link
Contributor

@mayya-sharipova mayya-sharipova commented Apr 4, 2024

Add ability to UnifiedHighlighter to combine matches from multiple fields
to highlight a single field.

FastVectorHighlighter for a long time has an option to highlight a single field
based on matches from several fields. But UnifiedHighlighter was missing this option.
This adds this ability.

@mayya-sharipova mayya-sharipova force-pushed the unified-highlighter-multi-fields branch 2 times, most recently from 0f00284 to 147b57e Compare April 4, 2024 19:06
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.

This is a great addition @mayya-sharipova .
I left a comment regarding the exposure, we should also document the limit of the feature (input should be identical for all masked fields).

Add ability to UnifiedHighlighter to combine matches from multiple fields
to highlight a single field.

FastVectorHighlighter for a long time has an option to highlight a single field
based on matches from several fields. But UnifiedHighlighter was missing this option.
This adds this ability.
@mayya-sharipova mayya-sharipova force-pushed the unified-highlighter-multi-fields branch from 147b57e to 30a63b0 Compare April 8, 2024 19:38
@mayya-sharipova
Copy link
Contributor Author

@jimczi Thanks for your initial review. I've tried to address all your comments, so this is ready for another review.

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.

LGTM

@mayya-sharipova mayya-sharipova merged commit 0345fca into apache:main Apr 12, 2024
3 checks passed
@mayya-sharipova mayya-sharipova deleted the unified-highlighter-multi-fields branch April 12, 2024 10:36
mayya-sharipova added a commit to mayya-sharipova/lucene that referenced this pull request Apr 12, 2024
Add ability to UnifiedHighlighter to combine matches from multiple fields
to highlight a single field.

FastVectorHighlighter for a long time has an option to highlight a single field
based on matches from several fields. But UnifiedHighlighter was missing this option.

This adds this ability with a new function: `UnifiedHighlighter::withMaskedFieldsFunc` 
that sets up a function that given a field retuns a set of masked fields whose matches 
are combined  to highlight the given field.
mayya-sharipova added a commit to mayya-sharipova/lucene that referenced this pull request Apr 12, 2024
Add ability to UnifiedHighlighter to combine matches from multiple fields
to highlight a single field.

FastVectorHighlighter for a long time has an option to highlight a single field
based on matches from several fields. But UnifiedHighlighter was missing this option.

This adds this ability with a new function: `UnifiedHighlighter::withMaskedFieldsFunc`
that sets up a function that given a field retuns a set of masked fields whose matches
are combined  to highlight the given field.
mayya-sharipova added a commit that referenced this pull request Apr 12, 2024
Add ability to UnifiedHighlighter to combine matches from multiple fields
to highlight a single field.

FastVectorHighlighter for a long time has an option to highlight a single field
based on matches from several fields. But UnifiedHighlighter was missing this option.

This adds this ability with a new function: `UnifiedHighlighter::withMaskedFieldsFunc`
that sets up a function that given a field retuns a set of masked fields whose matches
are combined  to highlight the given field.

Backport for PR #13268
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this pull request Apr 18, 2024
Add support to the Uunified highlighter to combine matches on multiple fields
to highlight a single field: "matched_fields".

Based on Lucene PR: apache/lucene#13268

Lucene PR is based on the concept of masked fields where masked fields
are different from the original highlighted field. This PR in
Elasticsearch uses the already existing highlighter parameter
"matched_fields"

Closes elastic#5172
mayya-sharipova added a commit to elastic/elasticsearch that referenced this pull request May 9, 2024
Add support to the Unified highlighter to combine matches on multiple fields
to highlight a single field: "matched_fields".

Based on Lucene PR: apache/lucene#13268

Lucene PR is based on the concept of masked fields where masked fields
are different from the original highlighted field. This PR in
Elasticsearch uses the already existing highlighter parameter
"matched_fields".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants