dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322141633
##########
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##########
@@ -205,7 +203,7 @@ public void testMultipleQueryWithSort() throws Exception {
}
/**
- * Tests the hybrid query functionality of the system.
+ * Tests the hybrid query functionality of the system with various setting
of pagination.
Review Comment:
thanks for improving this test :-)
##########
solr/core/src/java/org/apache/solr/search/combine/ReciprocalRankFusion.java:
##########
@@ -69,21 +68,24 @@ public int getK() {
@Override
public QueryResult combine(List<QueryResult> rankedLists, SolrParams
solrParams) {
int kVal = solrParams.getInt(CombinerParams.COMBINER_RRF_K, this.k);
- List<DocListAndSet> docListAndSet =
getDocListsAndSetFromQueryResults(rankedLists);
QueryResult combinedResult = new QueryResult();
- combineResults(combinedResult, docListAndSet, false, kVal);
+ combineResults(combinedResult, rankedLists, false, kVal);
return combinedResult;
}
- private static List<DocListAndSet> getDocListsAndSetFromQueryResults(
- List<QueryResult> rankedLists) {
- List<DocListAndSet> docLists = new ArrayList<>(rankedLists.size());
- for (QueryResult rankedList : rankedLists) {
- docLists.add(rankedList.getDocListAndSet());
- }
- return docLists;
- }
-
+ /**
+ * Merges per-shard ranked results using Reciprocal Rank Fusion (RRF).
Review Comment:
Reading this sentence, I think okay this is a typical application of RRF.
And the code does that, so it's technically not wrong. But lets be real with
the reader; in practice this is going to interleave the results because docs
uniquely only appear in one shard. Say that. And say that you change the
score to the RRF score.
As an aside, it'd probably be ideal to secondarily sort based on doc ID so
we have more reproducibility. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
