Skip to content

Commit a3ab7eb

Browse files
committed
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
1 parent a2ef0e8 commit a3ab7eb

File tree

2 files changed

+38
-2
lines changed

2 files changed

+38
-2
lines changed

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,7 @@ public String toString() {
130130
Result getResult() {
131131
List<Result> partialResults = new ArrayList<>();
132132
if (terms.size() > 0) {
133-
partialResults.add(conjunction ? handleConjunction(terms, version) :
134-
handleDisjunction(terms, minimumShouldMatch, version));
133+
partialResults.addAll(terms);
135134
}
136135
if (children.isEmpty() == false) {
137136
List<Result> childResults = children.stream().map(ResultBuilder::getResult).collect(Collectors.toList());

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

+37
Original file line numberDiff line numberDiff line change
@@ -1507,6 +1507,43 @@ public void testIntervalQueries() {
15071507
assertTermsEqual(result.extractions, new Term("field", "a"));
15081508
}
15091509

1510+
public void testRangeAndTermWithNestedMSM() {
1511+
1512+
Query q1 = new BooleanQuery.Builder()
1513+
.add(new TermQuery(new Term("f", "v3")), Occur.SHOULD)
1514+
.add(new BooleanQuery.Builder()
1515+
.add(new TermQuery(new Term("f", "n1")), Occur.SHOULD)
1516+
.build(), Occur.SHOULD)
1517+
.add(new TermQuery(new Term("f", "v4")), Occur.SHOULD)
1518+
.setMinimumNumberShouldMatch(2)
1519+
.build();
1520+
1521+
Result r1 = analyze(q1, Version.CURRENT);
1522+
assertEquals(2, r1.minimumShouldMatch);
1523+
assertThat(r1.extractions, hasSize(3));
1524+
assertFalse(r1.matchAllDocs);
1525+
assertTrue(r1.verified);
1526+
1527+
Query q = new BooleanQuery.Builder()
1528+
.add(IntPoint.newRangeQuery("i", 0, 10), Occur.FILTER)
1529+
.add(new TermQuery(new Term("f", "v1")), Occur.MUST)
1530+
.add(new TermQuery(new Term("f", "v2")), Occur.MUST)
1531+
.add(IntPoint.newRangeQuery("i", 2, 20), Occur.FILTER)
1532+
.add(new TermQuery(new Term("f", "v3")), Occur.SHOULD)
1533+
.add(new BooleanQuery.Builder()
1534+
.add(new TermQuery(new Term("f", "n1")), Occur.SHOULD)
1535+
.build(), Occur.SHOULD)
1536+
.add(new TermQuery(new Term("f", "v4")), Occur.SHOULD)
1537+
.setMinimumNumberShouldMatch(2)
1538+
.build();
1539+
1540+
Result r = analyze(q, Version.CURRENT);
1541+
assertThat(r.minimumShouldMatch, equalTo(5));
1542+
assertThat(r.extractions, hasSize(7));
1543+
assertFalse(r.matchAllDocs);
1544+
assertFalse(r.verified);
1545+
}
1546+
15101547
public void testCombinedRangeAndTermWithMinimumShouldMatch() {
15111548

15121549
Query disj = new BooleanQuery.Builder()

0 commit comments

Comments
 (0)