-
Notifications
You must be signed in to change notification settings - Fork 24.6k
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
Fix highlighting for script_score query #46507
Fix highlighting for script_score query #46507
Conversation
Pinging @elastic/es-search |
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.
This looks good to me. I think the fix could also be backported to 7.4.1
, as it could be considered a bug that highlighting doesn't work, and the change is quite safe.
"Script Score With Highlight": | ||
- skip: | ||
version: " - 7.4.99" | ||
reason: "highlight for script_score was introduce in 7.5" |
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.
Small typo: introduce -> introduced. Also, should this be 7.99.99
for now since it won't be available on 7.5 until it's backported?
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.
@jtibshirani Thanks for the review. I have changed the version to 7.4.1.
should this be 7.99.99 for now since it won't be available on 7.5 until it's backported?
Looks like there are no bwc tests for modules:lang-painless, as the build passed. But I am going to backport the change immediately after the merge to master.
--- | ||
"Script Score With Highlight": | ||
- skip: | ||
version: " - 7.4.09" |
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.
I think this should just be - 7.4.0
?
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.
@jtibshirani thanks, corrected in the last commit
closes #46471