-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Rescore universal query #4252
Rescore universal query #4252
Conversation
753caea
to
2e699c8
Compare
@coszio I have pushed a change to keep intermediate results in case of top level RRF rescoring. I applied the merging properly at the lower level by tracking the depth of the recursion. WDYT? |
// in case of top level RFF we need to propagate intermediate results | ||
// so the number of results is not limited by the outer limit at the shard level | ||
// the first source returned all its inner results | ||
assert_eq!(sources_scores[0].len(), inner_limit); |
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.
Good check 👍
} | ||
|
||
self.merge_prefetches(sources, &prefetch.merge).await | ||
if depth == 0 && prefetch.merge.rescore == Some(ScoringQuery::Fusion(Fusion::Rrf)) { |
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.
TBH I thought it was going to be handled at do_planned_query
, but this way there is less repetition. I like this.
* Rescore universal query * fix tests * top level RRF fusion should not merge results at shard level
This PR adds the implementation for re-scoring with RRF or a query at the shard level.
I took extra care make this testable by setting up test fixtures for shard tests.