Better logic ignoring filters when parsing

When parsing a filter, we effectively have a case where we need to ignore a filter. For example, when an "and" filter has no elements. Ignoring a filter is different compared to matching on all or matching on none, because an and filter with no elements should simply be ignored in the context of a bool filter for example, regardless if its used within a must or a must_not filter.

We should be consistent in our codebase when handling these use case. See also #3809

closes #3838

src/main/java/org/elasticsearch/index/query/AndFilterParser.java

@@ -108,6 +108,7 @@ public class AndFilterParser implements FilterParser {
         }
 
         if (filters.isEmpty()) {
+            // no filters provided, this should be ignored upstream
             return null;
         }
 

src/main/java/org/elasticsearch/index/query/BoolFilterParser.java

@@ -131,6 +131,7 @@ public class BoolFilterParser implements FilterParser {
         }
 
         if (boolFilter.clauses().isEmpty()) {
+            // no filters provided, it should be ignored upstream
             return null;
         }
 

src/main/java/org/elasticsearch/index/query/ExistsFilterParser.java

@@ -23,6 +23,7 @@ import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.Filter;
 import org.apache.lucene.search.TermRangeFilter;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.lucene.search.XBooleanFilter;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.cache.filter.support.CacheKeyFilter;
@@ -84,7 +85,8 @@ public class ExistsFilterParser implements FilterParser {
 
         Set<String> fields = parseContext.simpleMatchToIndexNames(fieldPattern);
         if (fields.isEmpty()) {
-            return null;
+            // no fields exists, so we should not match anything
+            return Queries.MATCH_NO_FILTER;
         }
         MapperService.SmartNameFieldMappers nonNullFieldMappers = null;
 

src/main/java/org/elasticsearch/index/query/FilterParser.java

@@ -37,6 +37,11 @@ public interface FilterParser {
     /**
      * Parses the into a filter from the current parser location. Will be at "START_OBJECT" location,
      * and should end when the token is at the matching "END_OBJECT".
+     * <p/>
+     * The parser should return null value when it should be ignored, regardless under which context
+     * it is. For example, an and filter with "and []" (no clauses), should be ignored regardless if
+     * it exists within a must clause or a must_not bool clause (that is why returning MATCH_ALL will
+     * not be good, since it will not match anything when returned within a must_not clause).
      */
     @Nullable
     Filter parse(QueryParseContext parseContext) throws IOException, QueryParsingException;

src/main/java/org/elasticsearch/index/query/FilteredQueryParser.java

@@ -121,9 +121,9 @@ public class FilteredQueryParser implements QueryParser {
                 // we allow for null filter, so it makes compositions on the client side to be simpler
                 return query;
             } else {
-                // the filter was provided, but returned null, meaning we should discard it, this means no
-                // matches for this query...
-                return Queries.NO_MATCH_QUERY;
+                // even if the filter is not found, and its null, we should simply ignore it, and go
+                // by the query
+                return query;
             }
         }
         if (filter == Queries.MATCH_ALL_FILTER) {

src/main/java/org/elasticsearch/index/query/IndexQueryParserService.java

@@ -258,6 +258,9 @@ public class IndexQueryParserService extends AbstractIndexComponent {
         }
     }
 
+    /**
+     * Parses an inner filter, returning null if the filter should be ignored.
+     */
     @Nullable
     public ParsedFilter parseInnerFilter(XContentParser parser) throws IOException {
         QueryParseContext context = cache.get();

src/main/java/org/elasticsearch/index/query/MissingFilterParser.java

@@ -24,6 +24,7 @@ import org.apache.lucene.search.Filter;
 import org.apache.lucene.search.TermRangeFilter;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.lucene.search.NotFilter;
+import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.lucene.search.XBooleanFilter;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.cache.filter.support.CacheKeyFilter;
@@ -95,6 +96,10 @@ public class MissingFilterParser implements FilterParser {
 
         Set<String> fields = parseContext.simpleMatchToIndexNames(fieldPattern);
         if (fields.isEmpty()) {
+            if (existence) {
+                // if we ask for existence of fields, and we found none, then we should match on all
+                return Queries.MATCH_ALL_FILTER;
+            }
             return null;
         }
 

src/main/java/org/elasticsearch/index/query/ParsedFilter.java

@@ -2,14 +2,10 @@ package org.elasticsearch.index.query;
 
 import com.google.common.collect.ImmutableMap;
 import org.apache.lucene.search.Filter;
-import org.elasticsearch.common.lucene.search.Queries;
 
 public class ParsedFilter {
 
-    public static final ParsedFilter EMPTY = new ParsedFilter(Queries.MATCH_NO_FILTER, ImmutableMap.<String, Filter>of());
-
     private final Filter filter;
-
     private final ImmutableMap<String, Filter> namedFilters;
 
     public ParsedFilter(Filter filter, ImmutableMap<String, Filter> namedFilters) {

src/main/java/org/elasticsearch/indices/cache/filter/terms/IndicesTermsFilterCache.java

@@ -31,6 +31,7 @@ import org.elasticsearch.client.Client;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeUnit;
 import org.elasticsearch.common.unit.ByteSizeValue;
@@ -47,7 +48,7 @@ import java.util.concurrent.TimeUnit;
  */
 public class IndicesTermsFilterCache extends AbstractComponent {
 
-    private static TermsFilterValue NO_TERMS = new TermsFilterValue(0, null);
+    private static TermsFilterValue NO_TERMS = new TermsFilterValue(0, Queries.MATCH_NO_FILTER);
 
     private final Client client;
 

src/main/java/org/elasticsearch/search/facet/filter/FilterFacetParser.java

@@ -21,6 +21,7 @@ package org.elasticsearch.search.facet.filter;
 
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.ParsedFilter;
@@ -59,9 +60,6 @@ public class FilterFacetParser extends AbstractComponent implements FacetParser
     @Override
     public FacetExecutor parse(String facetName, XContentParser parser, SearchContext context) throws IOException {
         ParsedFilter parsedFilter = context.queryParserService().parseInnerFilter(parser);
-        if (parsedFilter == null) {
-            parsedFilter = ParsedFilter.EMPTY;
-        }
-        return new FilterFacetExecutor(parsedFilter.filter());
+        return new FilterFacetExecutor(parsedFilter == null ? Queries.MATCH_ALL_FILTER : parsedFilter.filter());
     }
 }

src/main/java/org/elasticsearch/search/query/FilterBinaryParseElement.java

@@ -21,6 +21,7 @@ package org.elasticsearch.search.query;
 
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.index.query.ParsedFilter;
 import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.internal.SearchContext;
 
@@ -34,7 +35,10 @@ public class FilterBinaryParseElement implements SearchParseElement {
         byte[] filterSource = parser.binaryValue();
         XContentParser fSourceParser = XContentFactory.xContent(filterSource).createParser(filterSource);
         try {
-            context.parsedFilter(context.queryParserService().parseInnerFilter(fSourceParser));
+            ParsedFilter filter = context.queryParserService().parseInnerFilter(fSourceParser);
+            if (filter != null) {
+                context.parsedFilter(filter);
+            }
         } finally {
             fSourceParser.close();
         }

src/main/java/org/elasticsearch/search/query/FilterParseElement.java

@@ -32,9 +32,8 @@ public class FilterParseElement implements SearchParseElement {
     @Override
     public void parse(XContentParser parser, SearchContext context) throws Exception {
         ParsedFilter filter = context.queryParserService().parseInnerFilter(parser);
-        if (filter == null) {
-            filter = ParsedFilter.EMPTY;
-        }
+        if (filter != null) {
             context.parsedFilter(filter);
         }
+    }
 }

src/test/java/org/elasticsearch/search/facet/SimpleFacetsTests.java

@@ -103,7 +103,7 @@ public class SimpleFacetsTests extends AbstractIntegrationTest {
         assertThat(searchResponse.getHits().hits().length, equalTo(0));
         FilterFacet facet = searchResponse.getFacets().facet("facet1");
         assertThat(facet.getName(), equalTo("facet1"));
-        assertThat(facet.getCount(), equalTo(0l));
+        assertThat(facet.getCount(), equalTo(1l));
     }
 
     @Test