The `terms` query should always map to a Lucene `TermsQuery`. (#21786)

Currently, the `terms` query is just syctactic sugar for a `bool` query when
used in a query context. This change proposes to always generate the same query
in query and filter contexts, which is less confusing.

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

@@ -19,12 +19,8 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.index.Term;
 import org.apache.lucene.queries.TermsQuery;
-import org.apache.lucene.search.BooleanClause;
-import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.Query;
-import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefBuilder;
 import org.elasticsearch.action.get.GetRequest;
@@ -406,7 +402,17 @@ public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
         if (values == null || values.isEmpty()) {
             return Queries.newMatchNoDocsQuery("No terms supplied for \"" + getName() + "\" query.");
         }
-        return handleTermsQuery(values, fieldName, context);
+        MappedFieldType fieldType = context.fieldMapper(fieldName);
+
+        if (fieldType != null) {
+            return fieldType.termsQuery(values, context);
+        } else {
+            BytesRef[] filterValues = new BytesRef[values.size()];
+            for (int i = 0; i < filterValues.length; i++) {
+                filterValues[i] = BytesRefs.toBytesRef(values.get(i));
+            }
+            return new TermsQuery(fieldName, filterValues);
+        }
     }
 
     private List<Object> fetch(TermsLookup termsLookup, Client client) {
@@ -421,40 +427,6 @@ public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
         return terms;
     }
 
-    private static Query handleTermsQuery(List<?> terms, String fieldName, QueryShardContext context) {
-        MappedFieldType fieldType = context.fieldMapper(fieldName);
-        String indexFieldName;
-        if (fieldType != null) {
-            indexFieldName = fieldType.name();
-        } else {
-            indexFieldName = fieldName;
-        }
-
-        Query query;
-        if (context.isFilter()) {
-            if (fieldType != null) {
-                query = fieldType.termsQuery(terms, context);
-            } else {
-                BytesRef[] filterValues = new BytesRef[terms.size()];
-                for (int i = 0; i < filterValues.length; i++) {
-                    filterValues[i] = BytesRefs.toBytesRef(terms.get(i));
-                }
-                query = new TermsQuery(indexFieldName, filterValues);
-            }
-        } else {
-            BooleanQuery.Builder bq = new BooleanQuery.Builder();
-            for (Object term : terms) {
-                if (fieldType != null) {
-                    bq.add(fieldType.termQuery(term, context), BooleanClause.Occur.SHOULD);
-                } else {
-                    bq.add(new TermQuery(new Term(indexFieldName, BytesRefs.toBytesRef(term))), BooleanClause.Occur.SHOULD);
-                }
-            }
-            query = bq.build();
-        }
-        return query;
-    }
-
     @Override
     protected int doHashCode() {
         return Objects.hash(fieldName, values, termsLookup);

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

@@ -19,14 +19,11 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.index.Term;
-import org.apache.lucene.search.BooleanClause;
-import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.queries.TermsQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
+import org.apache.lucene.search.PointInSetQuery;
 import org.apache.lucene.search.Query;
-import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.util.BytesRef;
-import org.apache.lucene.util.CollectionUtil;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.get.GetRequest;
 import org.elasticsearch.action.get.GetResponse;
@@ -45,9 +42,12 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.either;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 
@@ -108,8 +108,7 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase<TermsQueryBuil
             MatchNoDocsQuery matchNoDocsQuery = (MatchNoDocsQuery) query;
             assertThat(matchNoDocsQuery.toString(), containsString("No terms supplied for \"terms\" query."));
         } else {
-            assertThat(query, instanceOf(BooleanQuery.class));
-            BooleanQuery booleanQuery = (BooleanQuery) query;
+            assertThat(query, either(instanceOf(TermsQuery.class)).or(instanceOf(PointInSetQuery.class)));
 
             // 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)
@@ -125,24 +124,9 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase<TermsQueryBuil
                 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);
+            TermsQuery expected = new TermsQuery(queryBuilder.fieldName(),
+                    terms.stream().filter(Objects::nonNull).map(Object::toString).map(BytesRef::new).collect(Collectors.toList()));
+            assertEquals(expected, query);
         }
     }
 

docs/reference/migration/migrate_6_0/search.asciidoc

@@ -18,6 +18,9 @@
 
 * The `fuzzy_match` and `match_fuzzy` query (synonyma for the `match` query) have been removed
 
+* The `terms` query now always returns scores equal to `1` and is not subject to
+  `indices.query.bool.max_clause_count` anymore.
+
 ==== Search shards API
 
 The search shards API no longer accepts the `type` url parameter, which didn't

docs/reference/query-dsl/terms-query.asciidoc

@@ -9,11 +9,7 @@ Filters documents that have fields that match any of the provided terms
 GET /_search
 {
     "query": {
-        "constant_score" : {
-            "filter" : {
-                "terms" : { "user" : ["kimchy", "elasticsearch"]}
-            }
-        }
+        "terms" : { "user" : ["kimchy", "elasticsearch"]}
     }
 }
 --------------------------------------------------