From 1bd31e9b5359af99622f0a8cc7f88296236546c0 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Fri, 10 Nov 2017 11:53:17 +0100 Subject: [PATCH] percolator: fixed issue where in indices created before 6.1 if minimum should match has been specified on a disjunction, the query would be marked as verified candidate match. This is wrong as it can only marked as verified candidate match on indices created on or after 6.1, due to the use of the CoveringQuery. --- .../percolator/QueryAnalyzer.java | 8 ++-- .../percolator/QueryAnalyzerTests.java | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+), 4 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 bed2d86a425..940f9ebab5a 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/QueryAnalyzer.java @@ -493,10 +493,12 @@ final class QueryAnalyzer { Version version) { // Keep track of the msm for each clause: int[] msmPerClause = new int[disjunctions.size()]; - String[] rangeFieldNames = new String[disjunctions.size()]; - boolean verified = otherClauses == false; + if (version.before(Version.V_6_1_0)) { + verified &= requiredShouldClauses <= 1; + } + Set terms = new HashSet<>(); for (int i = 0; i < disjunctions.size(); i++) { Query disjunct = disjunctions.get(i); @@ -513,9 +515,7 @@ final class QueryAnalyzer { int msm = 0; if (version.onOrAfter(Version.V_6_1_0)) { - Set seenRangeFields = new HashSet<>(); - // Figure out what the combined msm is for this disjunction: // (sum the lowest required clauses, otherwise we're too strict and queries may not match) Arrays.sort(msmPerClause); 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 27d00f9c30b..f2f5a4e5861 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java @@ -224,6 +224,50 @@ public class QueryAnalyzerTests extends ESTestCase { assertThat(terms.get(2).bytes(), equalTo(termQuery3.getTerm().bytes())); } + public void testExtractQueryMetadata_booleanQuery_msm() { + BooleanQuery.Builder builder = new BooleanQuery.Builder(); + builder.setMinimumNumberShouldMatch(2); + TermQuery termQuery1 = new TermQuery(new Term("_field", "_term1")); + builder.add(termQuery1, BooleanClause.Occur.SHOULD); + TermQuery termQuery2 = new TermQuery(new Term("_field", "_term2")); + builder.add(termQuery2, BooleanClause.Occur.SHOULD); + TermQuery termQuery3 = new TermQuery(new Term("_field", "_term3")); + builder.add(termQuery3, BooleanClause.Occur.SHOULD); + + BooleanQuery booleanQuery = builder.build(); + Result result = analyze(booleanQuery, Version.CURRENT); + assertThat(result.verified, is(true)); + assertThat(result.minimumShouldMatch, equalTo(2)); + List extractions = new ArrayList<>(result.extractions); + extractions.sort(Comparator.comparing(extraction -> extraction.term)); + assertThat(extractions.size(), equalTo(3)); + assertThat(extractions.get(0).term, equalTo(new Term("_field", "_term1"))); + assertThat(extractions.get(1).term, equalTo(new Term("_field", "_term2"))); + assertThat(extractions.get(2).term, equalTo(new Term("_field", "_term3"))); + } + + public void testExtractQueryMetadata_booleanQuery_msm_pre6dot1() { + BooleanQuery.Builder builder = new BooleanQuery.Builder(); + builder.setMinimumNumberShouldMatch(2); + TermQuery termQuery1 = new TermQuery(new Term("_field", "_term1")); + builder.add(termQuery1, BooleanClause.Occur.SHOULD); + TermQuery termQuery2 = new TermQuery(new Term("_field", "_term2")); + builder.add(termQuery2, BooleanClause.Occur.SHOULD); + TermQuery termQuery3 = new TermQuery(new Term("_field", "_term3")); + builder.add(termQuery3, BooleanClause.Occur.SHOULD); + + BooleanQuery booleanQuery = builder.build(); + Result result = analyze(booleanQuery, Version.V_6_0_0); + assertThat(result.verified, is(false)); + assertThat(result.minimumShouldMatch, equalTo(1)); + List extractions = new ArrayList<>(result.extractions); + extractions.sort(Comparator.comparing(extraction -> extraction.term)); + assertThat(extractions.size(), equalTo(3)); + assertThat(extractions.get(0).term, equalTo(new Term("_field", "_term1"))); + assertThat(extractions.get(1).term, equalTo(new Term("_field", "_term2"))); + assertThat(extractions.get(2).term, equalTo(new Term("_field", "_term3"))); + } + public void testExtractQueryMetadata_booleanQuery_onlyShould() { BooleanQuery.Builder builder = new BooleanQuery.Builder(); TermQuery termQuery1 = new TermQuery(new Term("_field", "_term1"));