Index: modules/queries/src/java/org/apache/lucene/queries/BooleanFilter.java =================================================================== --- modules/queries/src/java/org/apache/lucene/queries/BooleanFilter.java (revision 1174017) +++ modules/queries/src/java/org/apache/lucene/queries/BooleanFilter.java (working copy) @@ -30,6 +30,10 @@ import org.apache.lucene.util.OpenBitSet; import org.apache.lucene.util.OpenBitSetDISI; +// TODO: Use FixedBitSet instead of OpenBitSet, +// the optimizations below are no longer used, +// as core filters return FixedBitSet + /** * A container Filter that allows Boolean composition of Filters. * Filters are allocated into one of three logical constructs; @@ -39,7 +43,6 @@ * The resulting Filter is NOT'd with the NOT Filters * The resulting Filter is AND'd with the MUST Filters */ - public class BooleanFilter extends Filter { List shouldFilters = null; @@ -56,16 +59,22 @@ final IndexReader reader = context.reader; if (shouldFilters != null) { for (int i = 0; i < shouldFilters.size(); i++) { + final DocIdSet dis = getDIS(shouldFilters, i, context); + if (dis == null) { + continue; + } if (res == null) { - res = new OpenBitSetDISI(getDISI(shouldFilters, i, context), reader.maxDoc()); - } else { - DocIdSet dis = shouldFilters.get(i).getDocIdSet(context); - if(dis instanceof OpenBitSet) { - // optimized case for OpenBitSets - res.or((OpenBitSet) dis); - } else { - res.inPlaceOr(getDISI(shouldFilters, i, context)); + res = new OpenBitSetDISI(reader.maxDoc()); + } + if(dis instanceof OpenBitSet) { + // optimized case for OpenBitSets + res.or((OpenBitSet) dis); + } else { + final DocIdSetIterator disi = dis.iterator(); + if (disi == null) { + continue; } + res.inPlaceOr(disi); } } } @@ -73,31 +82,48 @@ if (notFilters != null) { for (int i = 0; i < notFilters.size(); i++) { if (res == null) { - res = new OpenBitSetDISI(getDISI(notFilters, i, context), reader.maxDoc()); + res = new OpenBitSetDISI(reader.maxDoc()); res.flip(0, reader.maxDoc()); // NOTE: may set bits on deleted docs + } + final DocIdSet dis = getDIS(notFilters, i, context); + if (dis == null) { + continue; + } + if(dis instanceof OpenBitSet) { + // optimized case for OpenBitSets + res.andNot((OpenBitSet) dis); } else { - DocIdSet dis = notFilters.get(i).getDocIdSet(context); - if(dis instanceof OpenBitSet) { - // optimized case for OpenBitSets - res.andNot((OpenBitSet) dis); - } else { - res.inPlaceNot(getDISI(notFilters, i, context)); + final DocIdSetIterator disi = dis.iterator(); + if (disi == null) { + continue; } + res.inPlaceNot(disi); } } } if (mustFilters != null) { for (int i = 0; i < mustFilters.size(); i++) { + final DocIdSet dis = getDIS(mustFilters, i, context); + if (dis == null) { + return DocIdSet.EMPTY_DOCIDSET; // no documents can match + } if (res == null) { - res = new OpenBitSetDISI(getDISI(mustFilters, i, context), reader.maxDoc()); + final DocIdSetIterator disi = dis.iterator(); + if (disi == null) { + return DocIdSet.EMPTY_DOCIDSET; // no documents can match + } + res = new OpenBitSetDISI(disi, reader.maxDoc()); } else { - DocIdSet dis = mustFilters.get(i).getDocIdSet(context); if(dis instanceof OpenBitSet) { // optimized case for OpenBitSets res.and((OpenBitSet) dis); } else { - res.inPlaceAnd(getDISI(mustFilters, i, context)); + final DocIdSetIterator disi = dis.iterator(); + if (disi == null) { + return DocIdSet.EMPTY_DOCIDSET; // no documents can match + } + res.inPlaceAnd(disi); } } } @@ -129,9 +155,9 @@ } } - private DocIdSetIterator getDISI(List filters, int index, AtomicReaderContext context) + private DocIdSet getDIS(List filters, int index, AtomicReaderContext context) throws IOException { - return filters.get(index).getDocIdSet(context).iterator(); + return filters.get(index).getDocIdSet(context); } @Override Index: modules/queries/src/test/org/apache/lucene/queries/BooleanFilterTest.java =================================================================== --- modules/queries/src/test/org/apache/lucene/queries/BooleanFilterTest.java (revision 1174017) +++ modules/queries/src/test/org/apache/lucene/queries/BooleanFilterTest.java (working copy) @@ -30,6 +30,8 @@ import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.search.Filter; import org.apache.lucene.search.TermRangeFilter; +import org.apache.lucene.search.DocIdSet; +import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.store.Directory; import org.apache.lucene.util.LuceneTestCase; @@ -82,9 +84,38 @@ return tf; } + + private Filter getNullDISFilter() { + return new Filter() { + @Override + public DocIdSet getDocIdSet(AtomicReaderContext context) { + return null; + } + }; + } + private Filter getNullDISIFilter() { + return new Filter() { + @Override + public DocIdSet getDocIdSet(AtomicReaderContext context) { + return new DocIdSet() { + @Override + public DocIdSetIterator iterator() { + return null; + } + + @Override + public boolean isCacheable() { + return true; + } + }; + } + }; + } + private void tstFilterCard(String mes, int expected, Filter filt) throws Throwable { + // BooleanFilter never returns null DIS or null DISI! DocIdSetIterator disi = filt.getDocIdSet(new AtomicReaderContext(reader)).iterator(); int actual = 0; while (disi.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) { @@ -162,4 +193,62 @@ booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.MUST_NOT)); tstFilterCard("MUST_NOT wins over MUST for same docs", 0, booleanFilter); } + + public void testCombinedNullDocIdSets() throws Throwable { + BooleanFilter booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.MUST)); + booleanFilter.add(new FilterClause(getNullDISFilter(), BooleanClause.Occur.MUST)); + tstFilterCard("A MUST filter that returns a null DIS should never return documents", 0, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.MUST)); + booleanFilter.add(new FilterClause(getNullDISIFilter(), BooleanClause.Occur.MUST)); + tstFilterCard("A MUST filter that returns a null DISI should never return documents", 0, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.SHOULD)); + booleanFilter.add(new FilterClause(getNullDISFilter(), BooleanClause.Occur.SHOULD)); + tstFilterCard("A SHOULD filter that returns a null DIS should be invisible", 1, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.SHOULD)); + booleanFilter.add(new FilterClause(getNullDISIFilter(), BooleanClause.Occur.SHOULD)); + tstFilterCard("A SHOULD filter that returns a null DISI should be invisible", 1, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.MUST)); + booleanFilter.add(new FilterClause(getNullDISFilter(), BooleanClause.Occur.MUST_NOT)); + tstFilterCard("A MUST_NOT filter that returns a null DIS should be invisible", 1, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.MUST)); + booleanFilter.add(new FilterClause(getNullDISIFilter(), BooleanClause.Occur.MUST_NOT)); + tstFilterCard("A MUST_NOT filter that returns a null DISI should be invisible", 1, booleanFilter); + } + + public void testJustNullDocIdSets() throws Throwable { + BooleanFilter booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getNullDISFilter(), BooleanClause.Occur.MUST)); + tstFilterCard("A MUST filter that returns a null DIS should never return documents", 0, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getNullDISIFilter(), BooleanClause.Occur.MUST)); + tstFilterCard("A MUST filter that returns a null DISI should never return documents", 0, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getNullDISFilter(), BooleanClause.Occur.SHOULD)); + tstFilterCard("A single SHOULD filter that returns a null DIS should never return documents", 0, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getNullDISIFilter(), BooleanClause.Occur.SHOULD)); + tstFilterCard("A single SHOULD filter that returns a null DISI should never return documents", 0, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getNullDISFilter(), BooleanClause.Occur.MUST_NOT)); + tstFilterCard("A single MUST_NOT filter that returns a null DIS should be invisible", 5, booleanFilter); + + booleanFilter = new BooleanFilter(); + booleanFilter.add(new FilterClause(getNullDISIFilter(), BooleanClause.Occur.MUST_NOT)); + tstFilterCard("A single MUST_NOT filter that returns a null DIS should be invisible", 5, booleanFilter); + } }