From a1c1a8b230287f2387c9e7fe64921f1eb26fb136 Mon Sep 17 00:00:00 2001
From: javanna <cavannaluca@gmail.com>
Date: Fri, 16 Oct 2015 11:06:35 +0200
Subject: [PATCH] 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
---
 .../index/query/BoolQueryBuilder.java         | 29 +++++----
 .../index/query/BoolQueryBuilderTests.java    | 59 +++++++++++++++----
 2 files changed, 62 insertions(+), 26 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java
index 25821e1dd1d..43b23778d56 100644
--- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java
+++ b/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));
diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java
index 7e50f6db5b1..68e739c11df 100644
--- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java
+++ b/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));
+    }
 }