lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shai Erera <ser...@gmail.com>
Subject Bug in TopFieldCollector?
Date Mon, 30 Mar 2009 07:50:30 GMT
Hi

As I prepared the patch for 1575, I noticed a strange implementation in
TopFieldCollector's topDocs():

    ScoreDoc[] scoreDocs = new ScoreDoc[queue.size()];
    if (fillFields) {
      for (int i = queue.size() - 1; i >= 0; i--) {
        scoreDocs[i] = queue.fillFields((FieldValueHitQueue.Entry)
queue.pop());
      }
    } else {
      Entry entry = (FieldValueHitQueue.Entry) queue.pop();
      for (int i = queue.size() - 1; i >= 0; i--) {
        scoreDocs[i] = new FieldDoc(entry.docID,
                                    entry.score);
      }
    }

    return new TopFieldDocs(totalHits, scoreDocs, queue.getFields(),
maxScore);


Notice that if fillFields is true, then documents are popped from the queue.
However if it's false, then the first document is popped out of the queue
and used to populate the entire ScoreDoc[]? I believe that's wrong, right?
Otherwise, the returned TopFieldDocs.scoreDocs array will include the same
document and score?

I noticed there's no test case for that ... TopFieldCollector's constructor
is called only from IndexSearcher.search(Weight, Filter, int, Sort, boolean
/* fillFields */) which is called from IndexSearcher.search(Weight, Filter,
int, sort) with fillFields always set to true. So perhaps that's why this
hasn't showed up?

BTW, the now deprecated TopFieldDocCollector's topDocs() implementation
looks like this:

    FieldSortedHitQueue fshq = (FieldSortedHitQueue)hq;
    ScoreDoc[] scoreDocs = new ScoreDoc[fshq.size()];
    for (int i = fshq.size()-1; i >= 0; i--)      // put docs in array
      scoreDocs[i] = fshq.fillFields ((FieldDoc) fshq.pop());

    return new TopFieldDocs(totalHits, scoreDocs,
                            fshq.getFields(), fshq.getMaxScore());

It assumes fillFields is always true and always pops elements out of the
queue.

If this is a bug, I can fix it as part of 1575, as I'm touching that class
anyway. I can also add a test case ... The fix is very simple BTW, just move
the line "Entry entry = (FieldValueHitQueue.Entry) queue.pop();" inside the
for loop.

Shai

Mime
View raw message