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