From a3ab7eb95dc2f1772f4d48a87e9f3156d4e33985 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 7 Jan 2020 09:31:22 +0000 Subject: [PATCH] Correctly handle MSM for nested disjunctions (#50669) With the rewrite of the percolator's QueryAnalyzer to use lucene's QueryVisitor API, term queries that are direct children of a boolean query are handled separately from other children. This works fine for conjunctions, but for disjunctions we need to treat the extracted terms from these direct descendents along with extractions from more deeply nested children to ensure that minimum-should-match requirements are met correctly. This commit changes the logic in QueryAnalyzer#getResult() to bundle child term results with all other results before handling them. Fixes #50305 --- .../percolator/QueryAnalyzer.java | 3 +- .../percolator/QueryAnalyzerTests.java | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java index 208b6b089f3..4616a2cb4d3 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java @@ -130,8 +130,7 @@ final class QueryAnalyzer { Result getResult() { List partialResults = new ArrayList<>(); if (terms.size() > 0) { - partialResults.add(conjunction ? handleConjunction(terms, version) : - handleDisjunction(terms, minimumShouldMatch, version)); + partialResults.addAll(terms); } if (children.isEmpty() == false) { List childResults = children.stream().map(ResultBuilder::getResult).collect(Collectors.toList()); diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java index 57f9cc31417..f3d354bd0e3 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java @@ -1507,6 +1507,43 @@ public class QueryAnalyzerTests extends ESTestCase { assertTermsEqual(result.extractions, new Term("field", "a")); } + public void testRangeAndTermWithNestedMSM() { + + Query q1 = new BooleanQuery.Builder() + .add(new TermQuery(new Term("f", "v3")), Occur.SHOULD) + .add(new BooleanQuery.Builder() + .add(new TermQuery(new Term("f", "n1")), Occur.SHOULD) + .build(), Occur.SHOULD) + .add(new TermQuery(new Term("f", "v4")), Occur.SHOULD) + .setMinimumNumberShouldMatch(2) + .build(); + + Result r1 = analyze(q1, Version.CURRENT); + assertEquals(2, r1.minimumShouldMatch); + assertThat(r1.extractions, hasSize(3)); + assertFalse(r1.matchAllDocs); + assertTrue(r1.verified); + + Query q = new BooleanQuery.Builder() + .add(IntPoint.newRangeQuery("i", 0, 10), Occur.FILTER) + .add(new TermQuery(new Term("f", "v1")), Occur.MUST) + .add(new TermQuery(new Term("f", "v2")), Occur.MUST) + .add(IntPoint.newRangeQuery("i", 2, 20), Occur.FILTER) + .add(new TermQuery(new Term("f", "v3")), Occur.SHOULD) + .add(new BooleanQuery.Builder() + .add(new TermQuery(new Term("f", "n1")), Occur.SHOULD) + .build(), Occur.SHOULD) + .add(new TermQuery(new Term("f", "v4")), Occur.SHOULD) + .setMinimumNumberShouldMatch(2) + .build(); + + Result r = analyze(q, Version.CURRENT); + assertThat(r.minimumShouldMatch, equalTo(5)); + assertThat(r.extractions, hasSize(7)); + assertFalse(r.matchAllDocs); + assertFalse(r.verified); + } + public void testCombinedRangeAndTermWithMinimumShouldMatch() { Query disj = new BooleanQuery.Builder()