> >> I do agree we should be returning null not EMPTY_DOCIDSET since
> >> Filter.getDocIDSet's jdoc clearly states a null return means no docs
> >> are accepted.
> >
> > I disagree. NULL is a stupid indicator, if something is empty it
> > should return something empty. I also don't like somebody returning
> > NULL instead of
> > Collections.emptySet() or like that. We should document Filter and
> > also Weight.scorer to always return non-null and also supply a
> > Scorer.EMPTY_SCORER. If you want to optimize something, you can always
> > change you loops to first do next() and then exit early.
>
> I don't think it's so clear cut. I think it depends on how the API is
used, and
> how advanced an API it is.
>
> For example, Fields.terms("foobar") returns null if foobar is a
non-existent
> field. I think that's appropriate, because to return a "fake instance"
loses
> information. EG caller can no longer tell if a given field exists or not.
OK, I agree with that. If the field does not exist, it makes no sense to
return anything.
> In some cases the null return can make a difference in performance, eg if
BQ
> is OR'ing two terms, but one of them yields a null scorer (matches no
docs)
> then the scorer can [almost -- coord] rewrite to a TermScorer with just
the
> other term vs BooleanScorer. We don't do this today, but we should/could.
In that case, BQ could simply do a check on scorer==EMPTY_SCORER to achieve
the same. As Scorer subclasses DocIdSetIterator, and is an iterator, it
should return something empty (see below).
> So my feeling is we have to take it case by case, and I think these two
cases
> (Weight.scorer and Filter.getDocIdSet) should keep their current contract
> (null may be returned if no docs will match).
But we should not force them to return either null or empty. So the docs say
"*may* return null". They dont need to. They can return an empty scorer if
they like.
BUT:
I am just upset about such code:
final DocIdSet dis = filter.getDocIdSet(reader);
if (dis == null)
return null;
final DocIdSetIterator disi = dis.iterator();
if (disi == null)
return null;
return new ConstantScorer(similarity, disi, this);
(this is what I have seen during my work for ConstantScoreQuery)
> Regardless, I think each API should clearly state the contract
unambiguously.
> Ie "this method never returns null", or, "this method will return null if
XYZ",
> or, "this method may return null if XYZ".
>
> >> And I think I'm on the opposite side of the fence on null returns, at
> > least for
> >> advanced APIs -- I'd prefer not to hide information (by returning an
> >> empty
> >> instance) in this case. But other cases I think should never return
> >> null
> > -- eg
> >> once you have a non-null DocIdSet, then its .iterator() should never
> > return
> >> null.
> >
> > I agree. DocIdSet.iterator() should return EMPTY_DOCIDSETITERATOR.
>
> Right, for an iterator from any class I think it should never return null.
Scorer is an iterator :-)
Uwe
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
