jpountz commented on a change in pull request #734: LUCENE-8857: Introduce 
Custom Tiebreakers in TopDocs#merge
URL: https://github.com/apache/lucene-solr/pull/734#discussion_r298529361
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/search/TopDocs.java
 ##########
 @@ -39,66 +50,52 @@ public TopDocs(TotalHits totalHits, ScoreDoc[] scoreDocs) {
     // Which shard (index into shardHits[]):
     final int shardIndex;
 
-    // True if we should use the incoming ScoreDoc.shardIndex for sort order
-    final boolean useScoreDocIndex;
-
     // Which hit within the shard:
     int hitIndex;
 
-    ShardRef(int shardIndex, boolean useScoreDocIndex) {
+    ShardRef(int shardIndex) {
       this.shardIndex = shardIndex;
-      this.useScoreDocIndex = useScoreDocIndex;
     }
 
     @Override
     public String toString() {
       return "ShardRef(shardIndex=" + shardIndex + " hitIndex=" + hitIndex + 
")";
     }
-
-    int getShardIndex(ScoreDoc scoreDoc) {
-      if (useScoreDocIndex) {
-        if (scoreDoc.shardIndex == -1) {
-          throw new IllegalArgumentException("setShardIndex is false but 
TopDocs[" + shardIndex + "].scoreDocs[" + hitIndex + "] is not set");
-        }
-        return scoreDoc.shardIndex;
-      } else {
-        // NOTE: we don't assert that shardIndex is -1 here, because caller 
could in fact have set it but asked us to ignore it now
-        return shardIndex;
-      }
-    }
   }
 
   /**
-   * if we need to tie-break since score / sort value are the same we first 
compare shard index (lower shard wins)
-   * and then iff shard index is the same we use the hit index.
+   * Use the tie breaker if provided. If tie breaker returns 0 signifying 
equal values, we use hit indices
+   * to tie break intra shard ties
    */
-  static boolean tieBreakLessThan(ShardRef first, ScoreDoc firstDoc, ShardRef 
second, ScoreDoc secondDoc) {
-    final int firstShardIndex = first.getShardIndex(firstDoc);
-    final int secondShardIndex = second.getShardIndex(secondDoc);
-    // Tie break: earlier shard wins
-    if (firstShardIndex< secondShardIndex) {
-      return true;
-    } else if (firstShardIndex > secondShardIndex) {
-      return false;
-    } else {
+  static boolean tieBreakLessThan(ShardRef first, ScoreDoc firstDoc, ShardRef 
second, ScoreDoc secondDoc,
+                                  Comparator<ScoreDoc> tieBreaker) {
+    assert tieBreaker != null;
+    int value = tieBreaker.compare(firstDoc, secondDoc);
+
+    if (value == 0) {
+      // Equal Values
       // Tie break in same shard: resolve however the
       // shard had resolved it:
       assert first.hitIndex != second.hitIndex;
       return first.hitIndex < second.hitIndex;
     }
+
+    return value < 0 ? true : false;
 
 Review comment:
   should be just `return value < 0;`

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to