lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (JIRA)" <>
Subject [jira] Commented: (LUCENE-1614) Add next() and skipTo() variants to DocIdSetIterator that return the current doc, instead of boolean
Date Wed, 20 May 2009 06:20:45 GMT


Shai Erera commented on LUCENE-1614:

So Mike - I've checked BS and BS2, and I don't see where does the return value can come into
play the way you describe. Perhaps I'm missing it, but here's what I found:

* BS - advance is not supported. In nextDoc the return value is checked only to verify if
the sub scorer is done or not, therefore comparing to >= 0 seems the better option here.

* BS2 - delegates advance and nextDoc to its counting scorer, which could have two variants
that may be affected:
** DisjunctionSumScorer - I don't see where the return value of the scorers is even considered.
** ConjunctionScorer - both nextDoc and advance delegate the call to doNext() which iterates
over the scorers. But it only checks the return value of advance to decide whether to continue
with the iteration or not. The only thing I changed is using advance() >= 0 instead of
skipTo which returned boolean. Is that the one you're talking about? Maybe you mean that in
that scorer (only?) I could drop the 'more' member and stop iterating when the first scorer
hits MAX_VAL, in which case it will not be less than the last doc, even if the last one is
on MAX_VAL also?

If I didn't miss anything, than that is just one scorer, which is not always instantiated.
The rest do compare the return value of nextDoc and advance to determine what to do, exactly
as they did before when the equivalent deprecated methods returned a boolean. It seems like
this change will hurt the performance of most Scorers/DISIs more than improve the performance
of BS2 in some cases where it instantiates a ConjunctionScorer. But like I said, maybe I'm
missing a Scorer, so I'd appreciate if you can refer me to the one you had in mind.

Also, given Marvin's response above, using 0 as sentinel is no different than using -1 in
terms of "suddenly moving backwards".

I fixed the documentation of advance and reinstated the implementation of TermScorer, so I'm
ready to post an updated patch. I'd like us to resolve the return value issue before I do
that though.

> Add next() and skipTo() variants to DocIdSetIterator that return the current doc, instead
of boolean
> ----------------------------------------------------------------------------------------------------
>                 Key: LUCENE-1614
>                 URL:
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Search
>            Reporter: Shai Erera
>             Fix For: 2.9
>         Attachments: LUCENE-1614.patch
> See
for the full discussion. The basic idea is to add variants to those two methods that return
the current doc they are at, to save successive calls to doc(). If there are no more docs,
return -1. A summary of what was discussed so far:
> # Deprecate those two methods.
> # Add nextDoc() and skipToDoc(int) that return doc, with default impl in DISI (calls
next() and skipTo() respectively, and will be changed to abstract in 3.0).
> #* I actually would like to propose an alternative to the names: advance() and advance(int)
- the first advances by one, the second advances to target.
> # Wherever these are used, do something like '(doc = advance()) >= 0' instead of comparing
to -1 for improved performance.
> I will post a patch shortly

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message