lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <luc...@mikemccandless.com>
Subject Re: Bug in TopFieldCollector?
Date Mon, 30 Mar 2009 08:07:48 GMT
Looks like quite a bug, Shai!  Thanks.  It came in with LUCENE-1483.
I would say add test case & fix it under 1575.

Mike

On Mon, Mar 30, 2009 at 3:50 AM, Shai Erera <serera@gmail.com> wrote:
> 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
>

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


Mime
View raw message