LUCENE-3446: Fix NPE in BooleanFilter when DocIdSet/DocIdSetIterator is null. Converted code to FixedBitSet and simplified.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1174380 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Uwe Schindler 2011-09-22 20:49:00 +00:00
parent 3133d65617
commit 573ecd5d7b
3 changed files with 165 additions and 32 deletions

View File

@ -92,6 +92,9 @@ Bug Fixes
* LUCENE-3019: Fix unexpected color tags for FastVectorHighlighter. (Koji Sekiguchi)
* LUCENE-3446: Fix NPE in BooleanFilter when DocIdSet/DocIdSetIterator is null.
Converted code to FixedBitSet and simplified. (Uwe Schindler, Shuji Umino)
API Changes
* LUCENE-3436: Add SuggestMode to the spellchecker, so you can specify the strategy

View File

@ -27,8 +27,7 @@ import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.DocIdSet;
import org.apache.lucene.search.DocIdSetIterator;
import org.apache.lucene.search.Filter;
import org.apache.lucene.util.OpenBitSet;
import org.apache.lucene.util.OpenBitSetDISI;
import org.apache.lucene.util.FixedBitSet;
/**
* A container Filter that allows Boolean composition of Filters.
@ -39,7 +38,6 @@ import org.apache.lucene.util.OpenBitSetDISI;
* 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<Filter> shouldFilters = null;
@ -52,53 +50,43 @@ public class BooleanFilter extends Filter {
*/
@Override
public DocIdSet getDocIdSet(AtomicReaderContext context) throws IOException {
OpenBitSetDISI res = null;
FixedBitSet res = null;
final IndexReader reader = context.reader;
if (shouldFilters != null) {
for (int i = 0; i < shouldFilters.size(); i++) {
final DocIdSetIterator disi = getDISI(shouldFilters, i, context);
if (disi == 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 FixedBitSet(reader.maxDoc());
}
res.or(disi);
}
}
if (notFilters != null) {
for (int i = 0; i < notFilters.size(); i++) {
if (res == null) {
res = new OpenBitSetDISI(getDISI(notFilters, i, context), reader.maxDoc());
res.flip(0, reader.maxDoc()); // NOTE: may set bits on deleted docs
} 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));
}
res = new FixedBitSet(reader.maxDoc());
res.set(0, reader.maxDoc()); // NOTE: may set bits on deleted docs
}
final DocIdSetIterator disi = getDISI(notFilters, i, context);
if (disi != null) {
res.andNot(disi);
}
}
}
if (mustFilters != null) {
for (int i = 0; i < mustFilters.size(); i++) {
final DocIdSetIterator disi = getDISI(mustFilters, i, context);
if (disi == null) {
return DocIdSet.EMPTY_DOCIDSET; // no documents can match
}
if (res == null) {
res = new OpenBitSetDISI(getDISI(mustFilters, i, context), reader.maxDoc());
res = new FixedBitSet(reader.maxDoc());
res.or(disi);
} 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));
}
res.and(disi);
}
}
}
@ -131,7 +119,8 @@ public class BooleanFilter extends Filter {
private DocIdSetIterator getDISI(List<Filter> filters, int index, AtomicReaderContext context)
throws IOException {
return filters.get(index).getDocIdSet(context).iterator();
final DocIdSet set = filters.get(index).getDocIdSet(context);
return (set == null) ? null : set.iterator();
}
@Override

View File

@ -30,6 +30,10 @@ import org.apache.lucene.search.BooleanClause;
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.search.TermQuery;
import org.apache.lucene.search.QueryWrapperFilter;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.LuceneTestCase;
@ -82,9 +86,42 @@ public class BooleanFilterTest extends LuceneTestCase {
return tf;
}
private Filter getWrappedTermQuery(String field, String text) {
return new QueryWrapperFilter(new TermQuery(new Term(field, text)));
}
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) {
@ -98,6 +135,11 @@ public class BooleanFilterTest extends LuceneTestCase {
BooleanFilter booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.SHOULD));
tstFilterCard("Should retrieves only 1 doc", 1, booleanFilter);
// same with a real DISI (no OpenBitSetIterator)
booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getWrappedTermQuery("price", "030"), BooleanClause.Occur.SHOULD));
tstFilterCard("Should retrieves only 1 doc", 1, booleanFilter);
}
public void testShoulds() throws Throwable {
@ -116,6 +158,16 @@ public class BooleanFilterTest extends LuceneTestCase {
booleanFilter.add(new FilterClause(getTermsFilter("inStock", "Maybe"), BooleanClause.Occur.MUST_NOT));
tstFilterCard("Shoulds Ored but AndNots", 3, booleanFilter);
// same with a real DISI (no OpenBitSetIterator)
booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getRangeFilter("price", "010", "020"), BooleanClause.Occur.SHOULD));
booleanFilter.add(new FilterClause(getRangeFilter("price", "020", "030"), BooleanClause.Occur.SHOULD));
booleanFilter.add(new FilterClause(getWrappedTermQuery("inStock", "N"), BooleanClause.Occur.MUST_NOT));
tstFilterCard("Shoulds Ored but AndNot", 4, booleanFilter);
booleanFilter.add(new FilterClause(getWrappedTermQuery("inStock", "Maybe"), BooleanClause.Occur.MUST_NOT));
tstFilterCard("Shoulds Ored but AndNots", 3, booleanFilter);
}
public void testShouldsAndMust() throws Throwable {
@ -124,6 +176,13 @@ public class BooleanFilterTest extends LuceneTestCase {
booleanFilter.add(new FilterClause(getRangeFilter("price", "020", "030"), BooleanClause.Occur.SHOULD));
booleanFilter.add(new FilterClause(getTermsFilter("accessRights", "admin"), BooleanClause.Occur.MUST));
tstFilterCard("Shoulds Ored but MUST", 3, booleanFilter);
// same with a real DISI (no OpenBitSetIterator)
booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getRangeFilter("price", "010", "020"), BooleanClause.Occur.SHOULD));
booleanFilter.add(new FilterClause(getRangeFilter("price", "020", "030"), BooleanClause.Occur.SHOULD));
booleanFilter.add(new FilterClause(getWrappedTermQuery("accessRights", "admin"), BooleanClause.Occur.MUST));
tstFilterCard("Shoulds Ored but MUST", 3, booleanFilter);
}
public void testShouldsAndMusts() throws Throwable {
@ -142,18 +201,36 @@ public class BooleanFilterTest extends LuceneTestCase {
booleanFilter.add(new FilterClause(getRangeFilter("date", "20050101", "20051231"), BooleanClause.Occur.MUST));
booleanFilter.add(new FilterClause(getTermsFilter("inStock", "N"), BooleanClause.Occur.MUST_NOT));
tstFilterCard("Shoulds Ored but MUSTs ANDED and MustNot", 0, booleanFilter);
// same with a real DISI (no OpenBitSetIterator)
booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getRangeFilter("price", "030", "040"), BooleanClause.Occur.SHOULD));
booleanFilter.add(new FilterClause(getWrappedTermQuery("accessRights", "admin"), BooleanClause.Occur.MUST));
booleanFilter.add(new FilterClause(getRangeFilter("date", "20050101", "20051231"), BooleanClause.Occur.MUST));
booleanFilter.add(new FilterClause(getWrappedTermQuery("inStock", "N"), BooleanClause.Occur.MUST_NOT));
tstFilterCard("Shoulds Ored but MUSTs ANDED and MustNot", 0, booleanFilter);
}
public void testJustMust() throws Throwable {
BooleanFilter booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getTermsFilter("accessRights", "admin"), BooleanClause.Occur.MUST));
tstFilterCard("MUST", 3, booleanFilter);
// same with a real DISI (no OpenBitSetIterator)
booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getWrappedTermQuery("accessRights", "admin"), BooleanClause.Occur.MUST));
tstFilterCard("MUST", 3, booleanFilter);
}
public void testJustMustNot() throws Throwable {
BooleanFilter booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getTermsFilter("inStock", "N"), BooleanClause.Occur.MUST_NOT));
tstFilterCard("MUST_NOT", 4, booleanFilter);
// same with a real DISI (no OpenBitSetIterator)
booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getWrappedTermQuery("inStock", "N"), BooleanClause.Occur.MUST_NOT));
tstFilterCard("MUST_NOT", 4, booleanFilter);
}
public void testMustAndMustNot() throws Throwable {
@ -161,5 +238,69 @@ public class BooleanFilterTest extends LuceneTestCase {
booleanFilter.add(new FilterClause(getTermsFilter("inStock", "N"), BooleanClause.Occur.MUST));
booleanFilter.add(new FilterClause(getTermsFilter("price", "030"), BooleanClause.Occur.MUST_NOT));
tstFilterCard("MUST_NOT wins over MUST for same docs", 0, booleanFilter);
// same with a real DISI (no OpenBitSetIterator)
booleanFilter = new BooleanFilter();
booleanFilter.add(new FilterClause(getWrappedTermQuery("inStock", "N"), BooleanClause.Occur.MUST));
booleanFilter.add(new FilterClause(getWrappedTermQuery("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);
}
}