-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add sub query explanations in DisjunctionMaxQuery#explain on no-match #13362
Add sub query explanations in DisjunctionMaxQuery#explain on no-match #13362
Conversation
… if it's a match or not.
This could make explanations harder to read for large queries, e.g. queries produced through rewriting. I wonder about doing something in-between such as only including the non-matching subs if the query doesn't match, what do you think? |
…query didn't match
Fair point! Adjusted the PR accordingly and added an additional test for the in-between behavior. |
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. Can you add a CHANGES entry under 9.11 / Improvements?
Thanks for the reviews! Added it to the improvement section ✅ @jpountz |
Closes #13357
Description
#13357 states that it's useful to have the explanations of the sub queries of
DisjuncationMaxQuery
present, even if the document didn't match. Considering that other queries likeCoveringQuery
also include explanations, if the document didn't match I've adjusted the logic according to the issue's proposal.Also added tests for
explain
(match and no match case). I've also adjustedCheckHits#verifyExplanation
to only check sub explanations, if the overall explanation returned a hit and scores need to be checked.