Skip to content

Commit 0bbae8a

Browse files
authored
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 8893e58 commit 0bbae8a

File tree

2 files changed

+38
-1
lines changed

2 files changed

+38
-1
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public String toString() {
126126
Result getResult() {
127127
List<Result> partialResults = new ArrayList<>();
128128
if (terms.size() > 0) {
129-
partialResults.add(conjunction ? handleConjunction(terms) : handleDisjunction(terms, minimumShouldMatch));
129+
partialResults.addAll(terms);
130130
}
131131
if (children.isEmpty() == false) {
132132
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
@@ -1209,6 +1209,43 @@ public void testIntervalQueries() {
12091209
assertTermsEqual(result.extractions, new Term("field", "a"));
12101210
}
12111211

1212+
public void testRangeAndTermWithNestedMSM() {
1213+
1214+
Query q1 = new BooleanQuery.Builder()
1215+
.add(new TermQuery(new Term("f", "v3")), Occur.SHOULD)
1216+
.add(new BooleanQuery.Builder()
1217+
.add(new TermQuery(new Term("f", "n1")), Occur.SHOULD)
1218+
.build(), Occur.SHOULD)
1219+
.add(new TermQuery(new Term("f", "v4")), Occur.SHOULD)
1220+
.setMinimumNumberShouldMatch(2)
1221+
.build();
1222+
1223+
Result r1 = analyze(q1, Version.CURRENT);
1224+
assertEquals(2, r1.minimumShouldMatch);
1225+
assertThat(r1.extractions, hasSize(3));
1226+
assertFalse(r1.matchAllDocs);
1227+
assertTrue(r1.verified);
1228+
1229+
Query q = new BooleanQuery.Builder()
1230+
.add(IntPoint.newRangeQuery("i", 0, 10), Occur.FILTER)
1231+
.add(new TermQuery(new Term("f", "v1")), Occur.MUST)
1232+
.add(new TermQuery(new Term("f", "v2")), Occur.MUST)
1233+
.add(IntPoint.newRangeQuery("i", 2, 20), Occur.FILTER)
1234+
.add(new TermQuery(new Term("f", "v3")), Occur.SHOULD)
1235+
.add(new BooleanQuery.Builder()
1236+
.add(new TermQuery(new Term("f", "n1")), Occur.SHOULD)
1237+
.build(), Occur.SHOULD)
1238+
.add(new TermQuery(new Term("f", "v4")), Occur.SHOULD)
1239+
.setMinimumNumberShouldMatch(2)
1240+
.build();
1241+
1242+
Result r = analyze(q, Version.CURRENT);
1243+
assertThat(r.minimumShouldMatch, equalTo(5));
1244+
assertThat(r.extractions, hasSize(7));
1245+
assertFalse(r.matchAllDocs);
1246+
assertFalse(r.verified);
1247+
}
1248+
12121249
public void testCombinedRangeAndTermWithMinimumShouldMatch() {
12131250

12141251
Query disj = new BooleanQuery.Builder()

0 commit comments

Comments
 (0)