On Wed, Jan 5, 2011 at 1:35 PM, Uwe Schindler <u...@thetaphi.de> wrote:
> Scorer is an iterator :-) Aha! You are right... I keep forgetting this about Scorer :) >> 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). I think that's a good idea? Ie the contract would then be "if Weight.scorer can determine up front that no docs can match, you should return EMPTY_SCORER since caller may optimize for that case". Ie, impls shouldn't return their own custom empty impl; they should return that specific instance. Hmm are we ever gonna run into classloader hell, where we have more than one instance of this EMPTY_SCORER somehow...? null is safer in that regard since null is always null... >> 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 don’t need to. They can return an empty scorer if > they like. Right -- I think "may return null" is the right contract here. > 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) Alas the DIS.iterator now states null is a valid return... sigh. I think it should not... but dangerous to change that now. But, even if we switch to the EMPTY_X sentinel, you'd still need these ifs? IE, we want CSQ to detect an "empty" filter and forward this on as an "empty scorer". Mike --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
