zacharymorn commented on a change in pull request #2205:
URL: https://github.com/apache/lucene-solr/pull/2205#discussion_r559275690
##########
File path: lucene/core/src/java/org/apache/lucene/search/WANDScorer.java
##########
@@ -257,13 +288,17 @@ public int advance(int target) throws IOException {
// Advance 'head' as well
advanceHead(target);
- // Pop the new 'lead' from 'head'
- moveToNextCandidate(target);
+ if (scoreMode == ScoreMode.TOP_SCORES) {
+ // Update score bounds if necessary so
+ updateMaxScoresIfNecessary(target);
- if (doc == DocIdSetIterator.NO_MORE_DOCS) {
- return DocIdSetIterator.NO_MORE_DOCS;
+ if (doc == DocIdSetIterator.NO_MORE_DOCS) {
Review comment:
For this one I actually moved the doc advancement to `NO_MORE_DOCS` for
condition `head.size() == 0` into `updateMaxScoresIfNecessary`, as initially I
felt these two are closely related and only applicable for TOP_SCORES mode (I
probably should have updated the method name though)
https://github.com/zacharymorn/lucene-solr/blob/9243ef01de6f4a4154275919a32f5703e970f49d/lucene/core/src/java/org/apache/lucene/search/WANDScorer.java#L463-L468
. And thus the original `moveToNextCandidate` implementation was separated
into `updateMaxScoresIfNecessary` and `setDocAndFreq`.
However, in hindsight I see this is not a very good abstraction, and it's
probably better to keep the original `moveToNextCandidate` and handle the
non-scoring mode inside. Let me give that a try.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
