Fixes NPE in bool filter, when an empty should filter clause returns …

…a DocIdSet, but null as iterator. Closes #4685
elastic · Jan 10, 2014 · b7ad04a · b7ad04a
1 parent 2df42e4
commit b7ad04a
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 7 deletions.
diff --git a/src/main/java/org/elasticsearch/common/lucene/search/XBooleanFilter.java b/src/main/java/org/elasticsearch/common/lucene/search/XBooleanFilter.java
@@ -282,7 +282,7 @@ public DocIdSet getDocIdSet(AtomicReaderContext context, Bits acceptDocs) throws
             for (int setDoc = it.nextDoc(); setDoc != DocIdSetIterator.NO_MORE_DOCS; setDoc = it.nextDoc()) {
                 for (ResultClause fastOrClause : fastOrClauses) {
                     DocIdSetIterator clauseIterator = fastOrClause.iterator();
-                    if (it == null) {
+                    if (clauseIterator == null) {
                         continue;
                     }
                     if (iteratorMatch(clauseIterator, setDoc)) {

diff --git a/src/test/java/org/elasticsearch/common/lucene/search/XBooleanFilterTests.java b/src/test/java/org/elasticsearch/common/lucene/search/XBooleanFilterTests.java
@@ -28,6 +28,7 @@
 import org.apache.lucene.queries.TermFilter;
 import org.apache.lucene.search.*;
 import org.apache.lucene.store.Directory;
+import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.FixedBitSet;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.test.ElasticsearchLuceneTestCase;
@@ -452,21 +453,36 @@ public void testRandom() throws IOException {
                     case 6:
                     case 5:
                         hasMust = true;
-                        clauses[i] = newFilterClause(field, value, MUST, random().nextBoolean());
-                        topLevel.add(new BooleanClause(new TermQuery(new Term(String.valueOf(field), String.valueOf(value))), MUST));
+                        if (rarely()) {
+                            clauses[i] = new FilterClause(new EmptyFilter(), MUST);
+                            topLevel.add(new BooleanClause(new MatchNoDocsQuery(), MUST));
+                        } else {
+                            clauses[i] = newFilterClause(field, value, MUST, random().nextBoolean());
+                            topLevel.add(new BooleanClause(new TermQuery(new Term(String.valueOf(field), String.valueOf(value))), MUST));
+                        }
                         break;
                     case 4:
                     case 3:
                     case 2:
                     case 1:
                         hasShould = true;
-                        clauses[i] = newFilterClause(field, value, SHOULD, random().nextBoolean());
-                        orQuery.add(new BooleanClause(new TermQuery(new Term(String.valueOf(field), String.valueOf(value))), SHOULD));
+                        if (rarely()) {
+                            clauses[i] = new FilterClause(new EmptyFilter(), SHOULD);
+                            orQuery.add(new BooleanClause(new MatchNoDocsQuery(), SHOULD));
+                        } else {
+                            clauses[i] = newFilterClause(field, value, SHOULD, random().nextBoolean());
+                            orQuery.add(new BooleanClause(new TermQuery(new Term(String.valueOf(field), String.valueOf(value))), SHOULD));
+                        }
                         break;
                     case 0:
                         hasMustNot = true;
-                        clauses[i] = newFilterClause(field, value, MUST_NOT, random().nextBoolean());
-                        topLevel.add(new BooleanClause(new TermQuery(new Term(String.valueOf(field), String.valueOf(value))), MUST_NOT));
+                        if (rarely()) {
+                            clauses[i] = new FilterClause(new EmptyFilter(), MUST_NOT);
+                            topLevel.add(new BooleanClause(new MatchNoDocsQuery(), MUST_NOT));
+                        } else {
+                            clauses[i] = newFilterClause(field, value, MUST_NOT, random().nextBoolean());
+                            topLevel.add(new BooleanClause(new TermQuery(new Term(String.valueOf(field), String.valueOf(value))), MUST_NOT));
+                        }
                         break;
 
                 }
@@ -531,5 +547,21 @@ public String toString() {
         }
     }
 
+    public final class EmptyFilter extends Filter {
+
+        @Override
+        public DocIdSet getDocIdSet(AtomicReaderContext context, Bits acceptDocs) throws IOException {
+            return random().nextBoolean() ? new Empty() : null;
+        }
+
+        private class Empty extends DocIdSet {
+
+            @Override
+            public DocIdSetIterator iterator() throws IOException {
+                return null;
+            }
+        }
+    }
+
 }