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
Unified Highlighter to support matched_fields #107640
Unified Highlighter to support matched_fields #107640
Conversation
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
Pinging @elastic/es-search (Team:Search) |
@jimczi What do you think of reusing the already existing parameter of |
+1 to reuse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, should we also throw an error if requireFieldMatch
is false?
...src/yamlRestTest/resources/rest-api-spec/test/search.highlight/60_unified_matched_fields.yml
Outdated
Show resolved
Hide resolved
…hter-multiple-fields
…hter-multiple-fields
…hter-multiple-fields
baa00a1
to
ce905fe
Compare
@jimczi @benwtrent Thanks for your feedback. I've addressed your comments. This is ready for another set of reviews. |
@jimczi Thanks for the feedback so far, I am wondering if you have any additional comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Please add an entry to the change log too |
@mayya-sharipova yes, please manually add a PR changes entry. This way we can keep track of features added when the Lucene snapshot branch is merged into main. |
…hter-multiple-fields
…hter-multiple-fields
@elasticsearchmachine run elasticsearch-ci/part-1 |
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 #5172