You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/05/27 13:58:52 UTC

[GitHub] [solr] tomglk commented on a change in pull request #151: SOLR-15437: ReRanking/LTR does not work in combination with custom sort and SolrCloud

tomglk commented on a change in pull request #151:
URL: https://github.com/apache/solr/pull/151#discussion_r640649136



##########
File path: solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
##########
@@ -999,20 +1013,34 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) {
 
           shardDoc.sortFieldValues = unmarshalledSortFieldValues;
 
-          queue.insertWithOverflow(shardDoc);
+          if(reRankQueue != null && docCounter++ <= reRankDocsSize) {
+              ShardDoc droppedShardDoc = reRankQueue.insertWithOverflow(shardDoc);
+              // FIXME: Only works if the original request does not sort by score
+            if(droppedShardDoc != null) {
+              queue.insertWithOverflow(droppedShardDoc);
+            }
+          } else {
+              queue.insertWithOverflow(shardDoc);
+          }
         } // end for-each-doc-in-response
       } // end for-each-response
       
       // The queue now has 0 -> queuesize docs, where queuesize <= start + rows
       // So we want to pop the last documents off the queue to get
       // the docs offset -> queuesize
       int resultSize = queue.size() - ss.getOffset();
+      if(reRankQueue != null) {
+        resultSize += reRankQueue.size();
+      }
       resultSize = Math.max(0, resultSize);  // there may not be any docs in range
 
       Map<Object,ShardDoc> resultIds = new HashMap<>();
       for (int i=resultSize-1; i>=0; i--) {
         ShardDoc shardDoc = queue.pop();
-        shardDoc.positionInResponse = i;
+        if(shardDoc == null && reRankQueue != null) {
+          shardDoc = reRankQueue.pop();
+        }
+        shardDoc.positionInResponse = i;  // FIXME check if we can handle a possible NPE more elegantly

Review comment:
       We are quite sure that a NPE can never occur here.
   However, there may be a nicer way to make sure that a meaningful exception is thrown.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org