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

Move PartialMatch field to Explanation struct and update related logic #2103

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

CascadingRadium
Copy link
Member

@CascadingRadium CascadingRadium commented Nov 21, 2024

  • Previously, the PartialMatch field was returned for every hit, but this caused confusion in complex queries involving disjunctions and match queries. As a result, we moved PartialMatch to the score explanation, where each subquery's explanation will include its own PartialMatch. This field is set only if the query uses the DisjunctionSearcher or scorer.

@abhinavdangeti
Copy link
Member

So just so we're clear here - partial_match will be set if an explicit disjunction were used and all its components did not match yes?
And also separately, if a match query resolves to a disjunction - and if some of the components did not match, we'll now be setting partial_match - correct?

@CascadingRadium
Copy link
Member Author

CascadingRadium commented Nov 21, 2024

  • A match query will be a disjunction query over all the terms, and if the document has all
    the terms then partial_match will be set to false else it will be set to true.
  • For all other queries partial_match will never show up in the search result as its omitempty and will be simply excluded from the search result.

@abhinavdangeti
Copy link
Member

Ok, then I disagree with your proposal here - we should be returning a partial_match when a disjunction's component doesn't match as well.

@CascadingRadium
Copy link
Member Author

CascadingRadium commented Nov 21, 2024

like when the top-most query is a disjunction query ?

@abhinavdangeti
Copy link
Member

like when the top-most query is a disjunction query ?

Ya.

Copy link
Member

@abhinavdangeti abhinavdangeti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per last conversation.

@CascadingRadium
Copy link
Member Author

CascadingRadium commented Jan 16, 2025

Actually, this approach has the limitation with disjunction + match queries
like so

{
  "disjuncts":[
    {
      "match":"termA termB"
    },
    {
      "match":"SONY PS5"
    }
  ]
}

where a single PartialMatch in the searchResult would lead to confusion regarding which component was partially matched (matchQuery1 / matchQuery2 / outerDisjuncts)

i will propose a new solution where the partialMatch is reflected in the score explanation, where each subQuery's expl will have the partialMatch attribute

@CascadingRadium CascadingRadium changed the title Retrieve PartialMatch Only for Match Queries Add PartialMatch field to Explanation struct and update related logic Jan 16, 2025
@abhinavdangeti abhinavdangeti changed the title Add PartialMatch field to Explanation struct and update related logic Move PartialMatch field to Explanation struct and update related logic Feb 3, 2025
@CascadingRadium CascadingRadium merged commit 7c54de5 into master Feb 4, 2025
18 checks passed
@CascadingRadium CascadingRadium deleted the optionalPartial branch February 4, 2025 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants