Query DSL: don't change state of the bool query while converting to lucene query

We used to change the minimumShouldMatch field of the query depending on the context, the final minimim should match should still be applied based on that, but the original minimumShouldMatch of the query shouldn't change. This was revelead by some recent test failure.

Closes #14153

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

@@ -272,26 +272,29 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> {
         if (booleanQuery.clauses().isEmpty()) {
             return new MatchAllDocsQuery();
         }
+        final String minimumShouldMatch;
+        if (context.isFilter() && this.minimumShouldMatch == null) {
+            //will be applied for real only if there are should clauses
+            minimumShouldMatch = "1";
+        } else {
+            minimumShouldMatch = this.minimumShouldMatch;
+        }
         booleanQuery = Queries.applyMinimumShouldMatch(booleanQuery, minimumShouldMatch);
         return adjustPureNegative ? fixNegativeQueryIfNeeded(booleanQuery) : booleanQuery;
     }
 
-    private void addBooleanClauses(QueryShardContext context, BooleanQuery.Builder booleanQueryBuilder, List<QueryBuilder> clauses, Occur occurs) throws IOException {
+    private static void addBooleanClauses(QueryShardContext context, BooleanQuery.Builder booleanQueryBuilder, List<QueryBuilder> clauses, Occur occurs) throws IOException {
         for (QueryBuilder query : clauses) {
             Query luceneQuery = null;
             switch (occurs) {
-            case SHOULD:
-                if (context.isFilter() && minimumShouldMatch == null) {
-                    minimumShouldMatch = "1";
-                }
-                luceneQuery = query.toQuery(context);
-                break;
-            case FILTER:
-            case MUST_NOT:
-                luceneQuery = query.toFilter(context);
-                break;
-            case MUST:
-                luceneQuery = query.toQuery(context);
+                case MUST:
+                case SHOULD:
+                    luceneQuery = query.toQuery(context);
+                    break;
+                case FILTER:
+                case MUST_NOT:
+                    luceneQuery = query.toFilter(context);
+                    break;
             }
             if (luceneQuery != null) {
                 booleanQueryBuilder.add(new BooleanClause(luceneQuery, occurs));

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

@@ -19,25 +19,15 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.search.BooleanClause;
-import org.apache.lucene.search.BooleanQuery;
-import org.apache.lucene.search.ConstantScoreQuery;
-import org.apache.lucene.search.MatchAllDocsQuery;
-import org.apache.lucene.search.Query;
+import org.apache.lucene.search.*;
 import org.hamcrest.Matchers;
 import org.junit.Test;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
-import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
-import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery;
-import static org.elasticsearch.index.query.QueryBuilders.termQuery;
+import static org.elasticsearch.index.query.QueryBuilders.*;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.instanceOf;
 
@@ -210,4 +200,47 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilde
         bq = (BooleanQuery) csq.getQuery();
         assertEquals(1, bq.getMinimumNumberShouldMatch());
     }
+
+    public void testMinShouldMatchFilterWithoutShouldClauses() throws Exception {
+        BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder();
+        boolQueryBuilder.filter(new BoolQueryBuilder().must(new MatchAllQueryBuilder()));
+        Query query = boolQueryBuilder.toQuery(createShardContext());
+        assertThat(query, instanceOf(BooleanQuery.class));
+        BooleanQuery booleanQuery = (BooleanQuery) query;
+        assertThat(booleanQuery.getMinimumNumberShouldMatch(), equalTo(0));
+        assertThat(booleanQuery.clauses().size(), equalTo(1));
+        BooleanClause booleanClause = booleanQuery.clauses().get(0);
+        assertThat(booleanClause.getOccur(), equalTo(BooleanClause.Occur.FILTER));
+        assertThat(booleanClause.getQuery(), instanceOf(BooleanQuery.class));
+        BooleanQuery innerBooleanQuery = (BooleanQuery) booleanClause.getQuery();
+        //we didn't set minimum should match initially, there are no should clauses so it should be 0
+        assertThat(innerBooleanQuery.getMinimumNumberShouldMatch(), equalTo(0));
+        assertThat(innerBooleanQuery.clauses().size(), equalTo(1));
+        BooleanClause innerBooleanClause = innerBooleanQuery.clauses().get(0);
+        assertThat(innerBooleanClause.getOccur(), equalTo(BooleanClause.Occur.MUST));
+        assertThat(innerBooleanClause.getQuery(), instanceOf(MatchAllDocsQuery.class));
+    }
+
+    public void testMinShouldMatchFilterWithShouldClauses() throws Exception {
+        BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder();
+        boolQueryBuilder.filter(new BoolQueryBuilder().must(new MatchAllQueryBuilder()).should(new MatchAllQueryBuilder()));
+        Query query = boolQueryBuilder.toQuery(createShardContext());
+        assertThat(query, instanceOf(BooleanQuery.class));
+        BooleanQuery booleanQuery = (BooleanQuery) query;
+        assertThat(booleanQuery.getMinimumNumberShouldMatch(), equalTo(0));
+        assertThat(booleanQuery.clauses().size(), equalTo(1));
+        BooleanClause booleanClause = booleanQuery.clauses().get(0);
+        assertThat(booleanClause.getOccur(), equalTo(BooleanClause.Occur.FILTER));
+        assertThat(booleanClause.getQuery(), instanceOf(BooleanQuery.class));
+        BooleanQuery innerBooleanQuery = (BooleanQuery) booleanClause.getQuery();
+        //we didn't set minimum should match initially, but there are should clauses so it should be 1
+        assertThat(innerBooleanQuery.getMinimumNumberShouldMatch(), equalTo(1));
+        assertThat(innerBooleanQuery.clauses().size(), equalTo(2));
+        BooleanClause innerBooleanClause1 = innerBooleanQuery.clauses().get(0);
+        assertThat(innerBooleanClause1.getOccur(), equalTo(BooleanClause.Occur.MUST));
+        assertThat(innerBooleanClause1.getQuery(), instanceOf(MatchAllDocsQuery.class));
+        BooleanClause innerBooleanClause2 = innerBooleanQuery.clauses().get(1);
+        assertThat(innerBooleanClause2.getOccur(), equalTo(BooleanClause.Occur.SHOULD));
+        assertThat(innerBooleanClause2.getQuery(), instanceOf(MatchAllDocsQuery.class));
+    }
 }