Merge pull request #18007 from MaineC/enhancement/switch_boolean_to_match_no_docs_query

Switches from empty boolean query to matchNoDocs

core/src/main/java/org/elasticsearch/common/lucene/search/MultiPhrasePrefixQuery.java

@@ -150,7 +150,7 @@ public class MultiPhrasePrefixQuery extends Query {
             }
         }
         if (terms.isEmpty()) {
-            return Queries.newMatchNoDocsQuery();
+            return Queries.newMatchNoDocsQuery("No terms supplied for " + MultiPhrasePrefixQuery.class.getName());
         }
         query.add(terms.toArray(Term.class), position);
         return query.build();

core/src/main/java/org/elasticsearch/common/lucene/search/Queries.java

@@ -44,8 +44,8 @@ public class Queries {
     }
 
     /** Return a query that matches no document. */
-    public static Query newMatchNoDocsQuery() {
-        return new BooleanQuery.Builder().build();
+    public static Query newMatchNoDocsQuery(String reason) {
+        return new MatchNoDocsQuery(reason);
     }
 
     public static Query newNestedFilter() {

core/src/main/java/org/elasticsearch/index/mapper/internal/IndexFieldMapper.java

@@ -144,7 +144,7 @@ public class IndexFieldMapper extends MetadataFieldMapper {
             if (isSameIndex(value, context.index().getName())) {
                 return Queries.newMatchAllQuery();
             } else {
-                return Queries.newMatchNoDocsQuery();
+                return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.index().getName() + " vs. " + value);
             }
         }
 
@@ -161,7 +161,7 @@ public class IndexFieldMapper extends MetadataFieldMapper {
                 }
             }
             // None of the listed index names are this one
-            return Queries.newMatchNoDocsQuery();
+            return Queries.newMatchNoDocsQuery("Index didn't match. Index queried: " + context.index().getName() + " vs. " + values);
         }
 
         private boolean isSameIndex(Object value, String indexName) {

core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java

@@ -134,7 +134,7 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder<ExistsQueryBuilder>
                 (FieldNamesFieldMapper.FieldNamesFieldType)context.getMapperService().fullName(FieldNamesFieldMapper.NAME);
         if (fieldNamesFieldType == null) {
             // can only happen when no types exist, so no docs exist either
-            return Queries.newMatchNoDocsQuery();
+            return Queries.newMatchNoDocsQuery("Missing types in \"" + NAME + "\" query.");
         }
 
         final Collection<String> fields;

core/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java

@@ -204,7 +204,7 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
     protected Query doToQuery(QueryShardContext context) throws IOException {
         Query query;
         if (this.ids.isEmpty()) {
-             query = Queries.newMatchNoDocsQuery();
+             query = Queries.newMatchNoDocsQuery("Missing ids in \"" + this.getName() + "\" query.");
         } else {
             Collection<String> typesForQuery;
             if (types.length == 0) {

core/src/main/java/org/elasticsearch/index/query/MatchNoneQueryBuilder.java

@@ -93,7 +93,7 @@ public class MatchNoneQueryBuilder extends AbstractQueryBuilder<MatchNoneQueryBu
 
     @Override
     protected Query doToQuery(QueryShardContext context) throws IOException {
-        return Queries.newMatchNoDocsQuery();
+        return Queries.newMatchNoDocsQuery("User requested \"" + this.getName() + "\" query.");
     }
 
     @Override

core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

@@ -373,7 +373,7 @@ public class QueryShardContext extends QueryRewriteContext {
     private static Query toQuery(final QueryBuilder<?> queryBuilder, final QueryShardContext context) throws IOException {
         final Query query = QueryBuilder.rewriteQuery(queryBuilder, context).toQuery(context);
         if (query == null) {
-            return Queries.newMatchNoDocsQuery();
+            return Queries.newMatchNoDocsQuery("No query left after rewrite.");
         }
         return query;
     }

core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java

@@ -317,7 +317,7 @@ public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
             throw new UnsupportedOperationException("query must be rewritten first");
         }
         if (values == null || values.isEmpty()) {
-            return Queries.newMatchNoDocsQuery();
+            return Queries.newMatchNoDocsQuery("No terms supplied for \"" + getName() + "\" query.");
         }
         return handleTermsQuery(values, fieldName, context);
     }

core/src/main/java/org/elasticsearch/index/search/MatchQuery.java

@@ -286,7 +286,11 @@ public class MatchQuery {
     }
 
     protected Query zeroTermsQuery() {
-        return zeroTermsQuery == DEFAULT_ZERO_TERMS_QUERY ? Queries.newMatchNoDocsQuery() : Queries.newMatchAllQuery();
+        if (zeroTermsQuery == DEFAULT_ZERO_TERMS_QUERY) {
+            return Queries.newMatchNoDocsQuery("Matching no documents because no terms present.");
+        }
+
+        return Queries.newMatchAllQuery();
     }
 
     private class MatchQueryBuilder extends QueryBuilder {

core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java

@@ -24,7 +24,6 @@ import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.core.io.JsonStringEncoder;
 
 import org.apache.lucene.search.BoostQuery;
-import org.apache.lucene.search.PrefixQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.search.spans.SpanBoostQuery;

core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java

@@ -21,16 +21,15 @@ package org.elasticsearch.index.query;
 
 
 import org.apache.lucene.queries.TermsQuery;
-import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
-import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
@@ -88,8 +87,7 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase<IdsQueryBuilder>
     @Override
     protected void doAssertLuceneQuery(IdsQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
         if (queryBuilder.ids().size() == 0) {
-            assertThat(query, instanceOf(BooleanQuery.class));
-            assertThat(((BooleanQuery)query).clauses().size(), equalTo(0));
+            assertThat(query, instanceOf(MatchNoDocsQuery.class));
         } else {
             assertThat(query, instanceOf(TermsQuery.class));
         }

core/src/test/java/org/elasticsearch/index/query/MatchNoneQueryBuilderTests.java

@@ -19,12 +19,11 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.Query;
+import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
 
 import java.io.IOException;
 
-import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.instanceOf;
 
 public class MatchNoneQueryBuilderTests extends AbstractQueryTestCase<MatchNoneQueryBuilder> {
@@ -36,9 +35,7 @@ public class MatchNoneQueryBuilderTests extends AbstractQueryTestCase<MatchNoneQ
 
     @Override
     protected void doAssertLuceneQuery(MatchNoneQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
-        assertThat(query, instanceOf(BooleanQuery.class));
-        BooleanQuery booleanQuery = (BooleanQuery) query;
-        assertThat(booleanQuery.clauses().size(), equalTo(0));
+        assertThat(query, instanceOf(MatchNoDocsQuery.class));
     }
 
     public void testFromJson() throws IOException {

core/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java

@@ -23,6 +23,7 @@ import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.PointRangeQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
+import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
 import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery;
 import java.io.IOException;
 import static org.hamcrest.CoreMatchers.either;
@@ -72,7 +73,7 @@ public class MatchPhrasePrefixQueryBuilderTests extends AbstractQueryTestCase<Ma
         assertThat(query, notNullValue());
         assertThat(query,
                 either(instanceOf(BooleanQuery.class)).or(instanceOf(MultiPhrasePrefixQuery.class))
-                .or(instanceOf(TermQuery.class)).or(instanceOf(PointRangeQuery.class)));
+                .or(instanceOf(TermQuery.class)).or(instanceOf(PointRangeQuery.class)).or(instanceOf(MatchNoDocsQuery.class)));
     }
 
     public void testIllegalValues() {

core/src/test/java/org/elasticsearch/index/query/MatchPhraseQueryBuilderTests.java

@@ -24,6 +24,7 @@ import org.apache.lucene.search.PhraseQuery;
 import org.apache.lucene.search.PointRangeQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
+import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
 
 import java.io.IOException;
 
@@ -68,7 +69,7 @@ public class MatchPhraseQueryBuilderTests extends AbstractQueryTestCase<MatchPhr
     protected void doAssertLuceneQuery(MatchPhraseQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
         assertThat(query, notNullValue());
         assertThat(query, either(instanceOf(BooleanQuery.class)).or(instanceOf(PhraseQuery.class))
-                .or(instanceOf(TermQuery.class)).or(instanceOf(PointRangeQuery.class)));
+                .or(instanceOf(TermQuery.class)).or(instanceOf(PointRangeQuery.class)).or(instanceOf(MatchNoDocsQuery.class)));
     }
 
     public void testIllegalValues() {

core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java

@@ -30,7 +30,7 @@ import org.apache.lucene.search.PointRangeQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
 import org.elasticsearch.common.ParseFieldMatcher;
-import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
 import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery;
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.unit.Fuzziness;
@@ -127,7 +127,7 @@ public class MatchQueryBuilderTests extends AbstractQueryTestCase<MatchQueryBuil
         switch (queryBuilder.type()) {
         case BOOLEAN:
             assertThat(query, either(instanceOf(BooleanQuery.class)).or(instanceOf(ExtendedCommonTermsQuery.class))
-                    .or(instanceOf(TermQuery.class)).or(instanceOf(FuzzyQuery.class))
+                    .or(instanceOf(TermQuery.class)).or(instanceOf(FuzzyQuery.class)).or(instanceOf(MatchNoDocsQuery.class))
                     .or(instanceOf(LegacyNumericRangeQuery.class)).or(instanceOf(PointRangeQuery.class)));
             break;
         case PHRASE:

core/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java

@@ -27,12 +27,12 @@ import org.apache.lucene.search.DisjunctionMaxQuery;
 import org.apache.lucene.search.FuzzyQuery;
 import org.apache.lucene.search.LegacyNumericRangeQuery;
 import org.apache.lucene.search.MatchAllDocsQuery;
-import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.PhraseQuery;
 import org.apache.lucene.search.PointRangeQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
 import org.elasticsearch.common.lucene.all.AllTermQuery;
+import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
 import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery;
 import org.elasticsearch.index.search.MatchQuery;
 

core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java

@@ -31,6 +31,7 @@ import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.common.ParseFieldMatcher;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.common.lucene.search.MatchNoDocsQuery;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.get.GetResult;
@@ -93,41 +94,45 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase<TermsQueryBuil
 
     @Override
     protected void doAssertLuceneQuery(TermsQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
-        assertThat(query, instanceOf(BooleanQuery.class));
-        BooleanQuery booleanQuery = (BooleanQuery) query;
-
-        // we only do the check below for string fields (otherwise we'd have to decode the values)
-        if (queryBuilder.fieldName().equals(INT_FIELD_NAME) || queryBuilder.fieldName().equals(DOUBLE_FIELD_NAME)
-                || queryBuilder.fieldName().equals(BOOLEAN_FIELD_NAME) || queryBuilder.fieldName().equals(DATE_FIELD_NAME)) {
-            return;
-        }
-
-        // expected returned terms depending on whether we have a terms query or a terms lookup query
-        List<Object> terms;
-        if (queryBuilder.termsLookup() != null) {
-            terms = randomTerms;
+        if (queryBuilder.termsLookup() == null && (queryBuilder.values() == null || queryBuilder.values().isEmpty())) {
+            assertThat(query, instanceOf(MatchNoDocsQuery.class));
         } else {
-            terms = queryBuilder.values();
-        }
-
-        // compare whether we have the expected list of terms returned
-        final List<Term> booleanTerms = new ArrayList<>();
-        for (BooleanClause booleanClause : booleanQuery) {
-            assertThat(booleanClause.getOccur(), equalTo(BooleanClause.Occur.SHOULD));
-            assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class));
-            Term term = ((TermQuery) booleanClause.getQuery()).getTerm();
-            booleanTerms.add(term);
-        }
-        CollectionUtil.timSort(booleanTerms);
-        List<Term> expectedTerms = new ArrayList<>();
-        for (Object term : terms) {
-            if (term != null) { // terms lookup filters this out
-                expectedTerms.add(new Term(queryBuilder.fieldName(), term.toString()));
+            assertThat(query, instanceOf(BooleanQuery.class));
+            BooleanQuery booleanQuery = (BooleanQuery) query;
+    
+            // we only do the check below for string fields (otherwise we'd have to decode the values)
+            if (queryBuilder.fieldName().equals(INT_FIELD_NAME) || queryBuilder.fieldName().equals(DOUBLE_FIELD_NAME)
+                    || queryBuilder.fieldName().equals(BOOLEAN_FIELD_NAME) || queryBuilder.fieldName().equals(DATE_FIELD_NAME)) {
+                return;
+            }
+    
+            // expected returned terms depending on whether we have a terms query or a terms lookup query
+            List<Object> terms;
+            if (queryBuilder.termsLookup() != null) {
+                terms = randomTerms;
+            } else {
+                terms = queryBuilder.values();
+            }
+    
+            // compare whether we have the expected list of terms returned
+            final List<Term> booleanTerms = new ArrayList<>();
+            for (BooleanClause booleanClause : booleanQuery) {
+                assertThat(booleanClause.getOccur(), equalTo(BooleanClause.Occur.SHOULD));
+                assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class));
+                Term term = ((TermQuery) booleanClause.getQuery()).getTerm();
+                booleanTerms.add(term);
+            }
+            CollectionUtil.timSort(booleanTerms);
+            List<Term> expectedTerms = new ArrayList<>();
+            for (Object term : terms) {
+                if (term != null) { // terms lookup filters this out
+                    expectedTerms.add(new Term(queryBuilder.fieldName(), term.toString()));
+                }
             }
+            CollectionUtil.timSort(expectedTerms);
+            assertEquals(expectedTerms + " vs. " + booleanTerms, expectedTerms.size(), booleanTerms.size());
+            assertEquals(expectedTerms + " vs. " + booleanTerms, expectedTerms, booleanTerms);
         }
-        CollectionUtil.timSort(expectedTerms);
-        assertEquals(expectedTerms + " vs. " + booleanTerms, expectedTerms.size(), booleanTerms.size());
-        assertEquals(expectedTerms + " vs. " + booleanTerms, expectedTerms, booleanTerms);
     }
 
     public void testEmtpyFieldName() {