Fix query analyzer logic for mixed conjunctions of terms and ranges

modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java

@@ -232,7 +232,7 @@ private static Result handleConjunction(List<Result> conjunctionsWithUnknowns) {
         List<Result> conjunctions = conjunctionsWithUnknowns.stream().filter(r -> r.isUnknown() == false).collect(Collectors.toList());
         if (conjunctions.isEmpty()) {
             if (conjunctionsWithUnknowns.isEmpty()) {
-                throw new IllegalArgumentException("Must have at least on conjunction sub result");
+                throw new IllegalArgumentException("Must have at least one conjunction sub result");
             }
             return conjunctionsWithUnknowns.get(0); // all conjunctions are unknown, so just return the first one
         }
@@ -261,23 +261,17 @@ private static Result handleConjunction(List<Result> conjunctionsWithUnknowns) {
             for (QueryExtraction queryExtraction : result.extractions) {
                 if (queryExtraction.range != null) {
                     // In case of range queries each extraction does not simply increment the
-                    // minimum_should_match
-                    // for that percolator query like for a term based extraction, so that can lead
-                    // to more false
-                    // positives for percolator queries with range queries than term based queries.
-                    // The is because the way number fields are extracted from the document to be
-                    // percolated.
-                    // Per field a single range is extracted and if a percolator query has two or
-                    // more range queries
-                    // on the same field, then the minimum should match can be higher than clauses
-                    // in the CoveringQuery.
-                    // Therefore right now the minimum should match is incremented once per number
-                    // field when processing
-                    // the percolator query at index time.
-                    if (seenRangeFields.add(queryExtraction.range.fieldName)) {
-                        resultMsm = 1;
-                    } else {
-                        resultMsm = 0;
+                    // minimum_should_match for that percolator query like for a term based extraction,
+                    // so that can lead to more false positives for percolator queries with range queries
+                    // than term based queries.
+                    // This is because the way number fields are extracted from the document to be
+                    // percolated.  Per field a single range is extracted and if a percolator query has two or
+                    // more range queries on the same field, then the minimum should match can be higher than clauses
+                    // in the CoveringQuery. Therefore right now the minimum should match is only incremented once per
+                    // number field when processing the percolator query at index time.
+                    if (seenRangeFields.add(queryExtraction.range.fieldName) == false) {
+                        resultMsm = Math.max(0, resultMsm - 1);
+                        verified = false;
                     }
                 }
 

modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java

@@ -1208,4 +1208,51 @@ public void testIntervalQueries() {
         assertTermsEqual(result.extractions, new Term("field", "a"));
     }
 
+    public void testCombinedRangeAndTermWithMinimumShouldMatch() {
+
+        Query disj = new BooleanQuery.Builder()
+            .add(IntPoint.newRangeQuery("i", 0, 10), Occur.SHOULD)
+            .add(new TermQuery(new Term("f", "v1")), Occur.SHOULD)
+            .add(new TermQuery(new Term("f", "v1")), Occur.SHOULD)
+            .setMinimumNumberShouldMatch(2)
+            .build();
+
+        Result r = analyze(disj, Version.CURRENT);
+        assertThat(r.minimumShouldMatch, equalTo(1));
+
+        Query q = new BooleanQuery.Builder()
+            .add(IntPoint.newRangeQuery("i", 0, 10), Occur.SHOULD)
+            .add(new TermQuery(new Term("f", "v1")), Occur.SHOULD)
+            .add(new TermQuery(new Term("f", "v1")), Occur.SHOULD)
+            .add(new TermQuery(new Term("f", "v1")), Occur.FILTER)
+            .setMinimumNumberShouldMatch(2)
+            .build();
+
+        Result result = analyze(q, Version.CURRENT);
+        assertThat(result.minimumShouldMatch, equalTo(1));
+        assertThat(result.extractions.size(), equalTo(2));
+        assertFalse(result.verified);
+
+        q = new BooleanQuery.Builder()
+            .add(q, Occur.MUST)
+            .add(q, Occur.MUST)
+            .build();
+
+        result = analyze(q, Version.CURRENT);
+        assertThat(result.minimumShouldMatch, equalTo(1));
+        assertThat(result.extractions.size(), equalTo(2));
+        assertFalse(result.verified);
+
+        Query q2 = new BooleanQuery.Builder()
+            .add(new TermQuery(new Term("f", "v1")), Occur.FILTER)
+            .add(IntPoint.newRangeQuery("i", 15, 20), Occur.SHOULD)
+            .add(new TermQuery(new Term("f", "v2")), Occur.SHOULD)
+            .add(new TermQuery(new Term("f", "v2")), Occur.MUST)
+            .setMinimumNumberShouldMatch(1)
+            .build();
+
+        result = analyze(q2, Version.CURRENT);
+        assertThat(result.minimumShouldMatch, equalTo(2));
+    }
+
 }