Mailing-List: contact java-dev-help@lucene.apache.org; run by ezmlm
Precedence: bulk
Reply-To: java-dev@lucene.apache.org
Received-SPF: neutral (nike.apache.org: local policy)
MIME-Version: 1.0
In-Reply-To: <786fde50903300050x576cb814g550df2128b45fb60@mail.gmail.com>
References: <786fde50903300050x576cb814g550df2128b45fb60@mail.gmail.com>
Date: Mon, 30 Mar 2009 04:07:48 -0400
Message-ID: <9ac0c6aa0903300107t35b31f80u5df09d618692178c@mail.gmail.com>
Subject: Re: Bug in TopFieldCollector?
From: Michael McCandless <lucene@mikemccandless.com>
To: java-dev@lucene.apache.org
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

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():
>
> =A0=A0=A0 ScoreDoc[] scoreDocs =3D new ScoreDoc[queue.size()];
> =A0=A0=A0 if (fillFields) {
> =A0=A0=A0=A0=A0 for (int i =3D queue.size() - 1; i >=3D 0; i--) {
> =A0=A0=A0=A0=A0=A0=A0 scoreDocs[i] =3D queue.fillFields((FieldValueHitQue=
ue.Entry)
> queue.pop());
> =A0=A0=A0=A0=A0 }
> =A0=A0=A0 } else {
> =A0=A0=A0=A0=A0 Entry entry =3D (FieldValueHitQueue.Entry) queue.pop();
> =A0=A0=A0=A0=A0 for (int i =3D queue.size() - 1; i >=3D 0; i--) {
> =A0=A0=A0=A0=A0=A0=A0 scoreDocs[i] =3D new FieldDoc(entry.docID,
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 entry.score);
> =A0=A0=A0=A0=A0 }
> =A0=A0=A0 }
>
> =A0=A0=A0 return new TopFieldDocs(totalHits, scoreDocs, queue.getFields()=
,
> maxScore);
>
>
> Notice that if fillFields is true, then documents are popped from the que=
ue.
> 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 sam=
e
> document and score?
>
> I noticed there's no test case for that ... TopFieldCollector's construct=
or
> is called only from IndexSearcher.search(Weight, Filter, int, Sort, boole=
an
> /* fillFields */) which is called from IndexSearcher.search(Weight, Filte=
r,
> 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:
>
> =A0=A0=A0 FieldSortedHitQueue fshq =3D (FieldSortedHitQueue)hq;
> =A0=A0=A0 ScoreDoc[] scoreDocs =3D new ScoreDoc[fshq.size()];
> =A0=A0=A0 for (int i =3D fshq.size()-1; i >=3D 0; i--)=A0=A0=A0=A0=A0 // =
put docs in array
> =A0=A0=A0=A0=A0 scoreDocs[i] =3D fshq.fillFields ((FieldDoc) fshq.pop());
>
> =A0=A0=A0 return new TopFieldDocs(totalHits, scoreDocs,
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0 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 clas=
s
> anyway. I can also add a test case ... The fix is very simple BTW, just m=
ove
> the line "Entry entry =3D (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