diff --git a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java index b38552663d1..f8b0deaf9be 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java @@ -70,7 +70,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp rethrowUnlessLenient(e); } } - return simplify(bq.build()); + return super.simplify(bq.build()); } /** @@ -93,7 +93,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp rethrowUnlessLenient(e); } } - return simplify(bq.build()); + return super.simplify(bq.build()); } @Override @@ -111,7 +111,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp rethrowUnlessLenient(e); } } - return simplify(bq.build()); + return super.simplify(bq.build()); } /** @@ -140,19 +140,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp return rethrowUnlessLenient(e); } } - return simplify(bq.build()); - } - - /** - * Override of lucenes SimpleQueryParser that doesn't simplify for the 1-clause case. - */ - @Override - protected Query simplify(BooleanQuery bq) { - if (bq.clauses().isEmpty()) { - return null; - } else { - return bq; - } + return super.simplify(bq.build()); } /** @@ -307,7 +295,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp // For further reasoning see // https://issues.apache.org/jira/browse/LUCENE-4021 return (Objects.equals(locale.toLanguageTag(), other.locale.toLanguageTag()) - && Objects.equals(lowercaseExpandedTerms, other.lowercaseExpandedTerms) + && Objects.equals(lowercaseExpandedTerms, other.lowercaseExpandedTerms) && Objects.equals(lenient, other.lenient) && Objects.equals(analyzeWildcard, other.analyzeWildcard)); } diff --git a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java index 2b221ed9ab0..a93c60ec147 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java @@ -20,8 +20,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.analysis.Analyzer; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.Strings; @@ -287,20 +285,8 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder 1 - && ((booleanQuery.clauses().iterator().next().getQuery() instanceof BooleanQuery) == false)) { - // special case for one term query and more than one field: (f1:t1 f2:t1 f3:t1) - // we need to wrap this in additional BooleanQuery so minimum_should_match is applied correctly - BooleanQuery.Builder builder = new BooleanQuery.Builder(); - builder.add(new BooleanClause(booleanQuery, Occur.SHOULD)); - booleanQuery = builder.build(); - } - if (minimumShouldMatch != null) { - booleanQuery = Queries.applyMinimumShouldMatch(booleanQuery, minimumShouldMatch); - } - query = booleanQuery; + if (minimumShouldMatch != null && query instanceof BooleanQuery) { + query = Queries.applyMinimumShouldMatch((BooleanQuery) query, minimumShouldMatch); } return query; } diff --git a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java index c553e94e29f..58cf1f3e522 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -523,7 +523,7 @@ public abstract class AbstractQueryTestCase> testQuery.writeTo(output); try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) { QueryBuilder prototype = queryParser(testQuery.getName()).getBuilderPrototype(); - QueryBuilder deserializedQuery = prototype.readFrom(in); + QueryBuilder deserializedQuery = prototype.readFrom(in); assertEquals(deserializedQuery, testQuery); assertEquals(deserializedQuery.hashCode(), testQuery.hashCode()); assertNotSame(deserializedQuery, testQuery); diff --git a/core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java index ac11c67e516..deb0f5a99fa 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java @@ -38,29 +38,19 @@ import java.util.HashSet; import java.util.Iterator; import java.util.Locale; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; -import java.util.TreeMap; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase { - private String[] queryTerms; - @Override protected SimpleQueryStringBuilder doCreateTestQueryBuilder() { - int numberOfTerms = randomIntBetween(1, 5); - queryTerms = new String[numberOfTerms]; - StringBuilder queryString = new StringBuilder(); - for (int i = 0; i < numberOfTerms; i++) { - queryTerms[i] = randomAsciiOfLengthBetween(1, 10); - queryString.append(queryTerms[i] + " "); - } - SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(queryString.toString().trim()); + SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(randomAsciiOfLengthBetween(1, 10)); if (randomBoolean()) { result.analyzeWildcard(randomBoolean()); } @@ -84,13 +74,9 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase flagSet = new HashSet<>(); - if (numberOfTerms > 1) { - flagSet.add(SimpleQueryStringFlag.WHITESPACE); - } int size = randomIntBetween(0, SimpleQueryStringFlag.values().length); for (int i = 0; i < size; i++) { - SimpleQueryStringFlag randomFlag = randomFrom(SimpleQueryStringFlag.values()); - flagSet.add(randomFlag); + flagSet.add(randomFrom(SimpleQueryStringFlag.values())); } if (flagSet.size() > 0) { result.flags(flagSet.toArray(new SimpleQueryStringFlag[flagSet.size()])); @@ -101,12 +87,13 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase fields = new HashMap<>(); for (int i = 0; i < fieldCount; i++) { if (randomBoolean()) { - fields.put("f" + i + "_" + randomAsciiOfLengthBetween(1, 10), AbstractQueryBuilder.DEFAULT_BOOST); + fields.put(randomAsciiOfLengthBetween(1, 10), AbstractQueryBuilder.DEFAULT_BOOST); } else { - fields.put(randomBoolean() ? STRING_FIELD_NAME : "f" + i + "_" + randomAsciiOfLengthBetween(1, 10), 2.0f / randomIntBetween(1, 20)); + fields.put(randomBoolean() ? STRING_FIELD_NAME : randomAsciiOfLengthBetween(1, 10), 2.0f / randomIntBetween(1, 20)); } } result.fields(fields); + return result; } @@ -271,8 +258,8 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase 0 || shardContext.indexQueryParserService().getIndexCreatedVersion().before(Version.V_1_4_0_Beta1)) { Query luceneQuery = queryBuilder.toQuery(shardContext); - assertThat(luceneQuery, instanceOf(BooleanQuery.class)); - TermQuery termQuery = (TermQuery) ((BooleanQuery) luceneQuery).clauses().get(0).getQuery(); + assertThat(luceneQuery, instanceOf(TermQuery.class)); + TermQuery termQuery = (TermQuery) luceneQuery; assertThat(termQuery.getTerm(), equalTo(new Term(MetaData.ALL, query))); } } @@ -290,7 +277,7 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase 1) { assertTrue("Query should have been BooleanQuery but was " + query.getClass().getName(), query instanceof BooleanQuery); BooleanQuery boolQuery = (BooleanQuery) query; @@ -303,42 +290,32 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase expectedFields = new TreeMap(queryBuilder.fields()); - if (expectedFields.size() == 0) { - expectedFields.put(MetaData.ALL, AbstractQueryBuilder.DEFAULT_BOOST); - } - for (int i = 0; i < queryTerms.length; i++) { - BooleanClause booleanClause = boolQuery.clauses().get(i); - Iterator> fieldsIter = expectedFields.entrySet().iterator(); - - if (queryTerms.length == 1 && expectedFields.size() == 1) { - assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) booleanClause.getQuery(); - Entry entry = fieldsIter.next(); - assertThat(termQuery.getTerm().field(), equalTo(entry.getKey())); - assertThat(termQuery.getBoost(), equalTo(entry.getValue())); - assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryTerms[i].toLowerCase(Locale.ROOT))); - } else { - assertThat(booleanClause.getQuery(), instanceOf(BooleanQuery.class)); - for (BooleanClause clause : ((BooleanQuery) booleanClause.getQuery()).clauses()) { - TermQuery termQuery = (TermQuery) clause.getQuery(); - Entry entry = fieldsIter.next(); - assertThat(termQuery.getTerm().field(), equalTo(entry.getKey())); - assertThat(termQuery.getBoost(), equalTo(entry.getValue())); - assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryTerms[i].toLowerCase(Locale.ROOT))); - } - } + assertThat(boolQuery.clauses().size(), equalTo(queryBuilder.fields().size())); + Iterator fields = queryBuilder.fields().keySet().iterator(); + for (BooleanClause booleanClause : boolQuery) { + assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class)); + TermQuery termQuery = (TermQuery) booleanClause.getQuery(); + assertThat(termQuery.getTerm().field(), equalTo(fields.next())); + assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryBuilder.value().toLowerCase(Locale.ROOT))); } if (queryBuilder.minimumShouldMatch() != null) { - int optionalClauses = queryTerms.length; - if (queryBuilder.defaultOperator().equals(Operator.AND) && queryTerms.length > 1) { - optionalClauses = 0; - } - int expectedMinimumShouldMatch = Queries.calculateMinShouldMatch(optionalClauses, queryBuilder.minimumShouldMatch()); - assertEquals(expectedMinimumShouldMatch, boolQuery.getMinimumNumberShouldMatch()); + assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0)); } + } else if (queryBuilder.fields().size() <= 1) { + assertTrue("Query should have been TermQuery but was " + query.getClass().getName(), query instanceof TermQuery); + + TermQuery termQuery = (TermQuery) query; + String field; + if (queryBuilder.fields().size() == 0) { + field = MetaData.ALL; + } else { + field = queryBuilder.fields().keySet().iterator().next(); + } + assertThat(termQuery.getTerm().field(), equalTo(field)); + assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryBuilder.value().toLowerCase(Locale.ROOT))); + } else { + fail("Encountered lucene query type we do not have a validation implementation for in our " + SimpleQueryStringBuilderTests.class.getSimpleName()); } } @@ -364,18 +341,15 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase 1) { - expectedMinimumShouldMatch = 0; - } - - assertEquals(expectedMinimumShouldMatch, query.getMinimumNumberShouldMatch()); - for (BooleanClause clause : query.clauses()) { - if (numberOfFields == 1 && numberOfTerms == 1) { - assertTrue(clause.getQuery() instanceof TermQuery); - } else { - assertEquals(numberOfFields, ((BooleanQuery) clause.getQuery()).clauses().size()); - } - } - } } diff --git a/core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java b/core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java index 404b221e389..358122f54ec 100644 --- a/core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java +++ b/core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java @@ -109,6 +109,7 @@ public class SimpleQueryStringIT extends ESIntegTestCase { client().prepareIndex("test", "type1", "3").setSource("body", "foo bar"), client().prepareIndex("test", "type1", "4").setSource("body", "foo baz bar")); + logger.info("--> query 1"); SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")).get(); assertHitCount(searchResponse, 2l); @@ -119,13 +120,7 @@ public class SimpleQueryStringIT extends ESIntegTestCase { assertHitCount(searchResponse, 2l); assertSearchHits(searchResponse, "3", "4"); - logger.info("--> query 3"); // test case from #13884 - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo") - .field("body").field("body2").field("body3").minimumShouldMatch("-50%")).get(); - assertHitCount(searchResponse, 3l); - assertSearchHits(searchResponse, "1", "3", "4"); - - logger.info("--> query 4"); + logger.info("--> query 3"); searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body").field("body2").minimumShouldMatch("70%")).get(); assertHitCount(searchResponse, 2l); assertSearchHits(searchResponse, "3", "4"); @@ -136,17 +131,17 @@ public class SimpleQueryStringIT extends ESIntegTestCase { client().prepareIndex("test", "type1", "7").setSource("body2", "foo bar", "other", "foo"), client().prepareIndex("test", "type1", "8").setSource("body2", "foo baz bar", "other", "foo")); - logger.info("--> query 5"); + logger.info("--> query 4"); searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").field("body").field("body2").minimumShouldMatch("2")).get(); assertHitCount(searchResponse, 4l); assertSearchHits(searchResponse, "3", "4", "7", "8"); - logger.info("--> query 6"); + logger.info("--> query 5"); searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")).get(); assertHitCount(searchResponse, 5l); assertSearchHits(searchResponse, "3", "4", "6", "7", "8"); - logger.info("--> query 7"); + logger.info("--> query 6"); searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body2").field("other").minimumShouldMatch("70%")).get(); assertHitCount(searchResponse, 3l); assertSearchHits(searchResponse, "6", "7", "8");