diff --git a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java index 574835c6b7a..8848a5ce558 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -91,7 +91,7 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); - Optional query = Optional.empty(); + Optional query = null; boolean queryFound = false; String queryName = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; diff --git a/core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java index 3ceca1a07a0..b2d8f8b8d15 100644 --- a/core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -168,7 +169,7 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder } } - if (!queriesFound || queries.isEmpty()) { + if (!queriesFound) { throw new ParsingException(parser.getTokenLocation(), "[dis_max] requires 'queries' field with at least one clause"); } @@ -186,6 +187,10 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder protected Query doToQuery(QueryShardContext context) throws IOException { // return null if there are no queries at all Collection luceneQueries = toQueries(queries, context); + if (luceneQueries.isEmpty()) { + return Queries.newMatchNoDocsQuery("no clauses for dismax query."); + } + return new DisjunctionMaxQuery(luceneQueries, tieBreaker); } diff --git a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index 4c47304330a..8c4be225127 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -611,7 +611,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ /** * Add an aggregation to perform as part of the search. */ - public SearchSourceBuilder aggregation(PipelineAggregatorBuilder aggregation) { + public SearchSourceBuilder aggregation(PipelineAggregatorBuilder aggregation) { if (aggregations == null) { aggregations = AggregatorFactories.builder(); } diff --git a/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java b/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java index 6f59b446dc2..5603446914c 100644 --- a/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java @@ -72,6 +72,9 @@ public class QueryRescorerBuilder extends RescoreBuilder { * @param builder the query builder to build the rescore query from */ public QueryRescorerBuilder(QueryBuilder builder) { + if (builder == null) { + throw new IllegalArgumentException("rescore_query cannot be null"); + } this.queryBuilder = builder; } diff --git a/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java index f18db46346c..ebd6446f80a 100644 --- a/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java @@ -25,7 +25,7 @@ import org.apache.lucene.search.DisjunctionMaxQuery; import org.apache.lucene.search.PrefixQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParseFieldMatcher; -import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.lucene.search.MatchNoDocsQuery; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; @@ -35,7 +35,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -60,17 +59,13 @@ public class DisMaxQueryBuilderTests extends AbstractQueryTestCase queries = AbstractQueryBuilder.toQueries(queryBuilder.innerQueries(), context); - if (queries.isEmpty()) { - assertThat(query, nullValue()); - } else { - assertThat(query, instanceOf(DisjunctionMaxQuery.class)); - DisjunctionMaxQuery disjunctionMaxQuery = (DisjunctionMaxQuery) query; - assertThat(disjunctionMaxQuery.getTieBreakerMultiplier(), equalTo(queryBuilder.tieBreaker())); - assertThat(disjunctionMaxQuery.getDisjuncts().size(), equalTo(queries.size())); - Iterator queryIterator = queries.iterator(); - for (int i = 0; i < disjunctionMaxQuery.getDisjuncts().size(); i++) { - assertThat(disjunctionMaxQuery.getDisjuncts().get(i), equalTo(queryIterator.next())); - } + assertThat(query, instanceOf(DisjunctionMaxQuery.class)); + DisjunctionMaxQuery disjunctionMaxQuery = (DisjunctionMaxQuery) query; + assertThat(disjunctionMaxQuery.getTieBreakerMultiplier(), equalTo(queryBuilder.tieBreaker())); + assertThat(disjunctionMaxQuery.getDisjuncts().size(), equalTo(queries.size())); + Iterator queryIterator = queries.iterator(); + for (int i = 0; i < disjunctionMaxQuery.getDisjuncts().size(); i++) { + assertThat(disjunctionMaxQuery.getDisjuncts().get(i), equalTo(queryIterator.next())); } } @@ -90,7 +85,7 @@ public class DisMaxQueryBuilderTests extends AbstractQueryTestCase parseQuery(queryString, ParseFieldMatcher.EMPTY)); - assertEquals("[dis_max] requires 'queries' field with at least one clause", ex.getMessage()); + QueryBuilder queryBuilder = parseQuery(queryString, ParseFieldMatcher.EMPTY); + QueryShardContext context = createShardContext(); + Query luceneQuery = queryBuilder.toQuery(context); + assertThat(luceneQuery, instanceOf(MatchNoDocsQuery.class)); + assertThat(((MatchNoDocsQuery) luceneQuery).toString(), equalTo("MatchNoDocsQuery[\"no clauses for dismax query.\"]")); } public void testIllegalArguments() { diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java index ddb62d57ffe..a48facc4d66 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java @@ -132,7 +132,7 @@ public class FilterIT extends ESIntegTestCase { String emtpyFilterBody = "{ }"; XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); - AggregationBuilder filterAgg = FilterAggregationBuilder.parse("tag1", parseContext); + AggregationBuilder filterAgg = FilterAggregationBuilder.parse("tag1", parseContext); SearchResponse response = client().prepareSearch("idx").addAggregation(filterAgg).execute().actionGet(); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java index 0be0f76de90..a95df3ff5e6 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java @@ -217,7 +217,7 @@ public class FiltersIT extends ESIntegTestCase { XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); parser.nextToken(); QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); - AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); + AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); SearchResponse response = client().prepareSearch("idx").addAggregation(filtersAgg).execute().actionGet(); @@ -234,7 +234,7 @@ public class FiltersIT extends ESIntegTestCase { XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); parser.nextToken(); QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); - AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); + AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); SearchResponse response = client().prepareSearch("idx").addAggregation(filtersAgg) .execute().actionGet(); diff --git a/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java b/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java index 5cc59046433..f965d3ac5fd 100644 --- a/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java @@ -54,6 +54,7 @@ import org.junit.AfterClass; import org.junit.BeforeClass; import java.io.IOException; + import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -180,6 +181,11 @@ public class QueryRescoreBuilderTests extends ESTestCase { } } + public void testRescoreQueryNull() throws IOException { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new QueryRescorerBuilder((QueryBuilder) null)); + assertEquals("rescore_query cannot be null", e.getMessage()); + } + /** * test parsing exceptions for incorrect rescorer syntax */ diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index cb73ebc0752..b7a0d5a16f6 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -19,26 +19,9 @@ package org.elasticsearch.test; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.either; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.not; - -import java.io.Closeable; -import java.io.IOException; -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.Method; -import java.lang.reflect.Proxy; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ExecutionException; +import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.io.JsonStringEncoder; import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.Query; @@ -132,9 +115,26 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; -import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; -import com.fasterxml.jackson.core.JsonParseException; -import com.fasterxml.jackson.core.io.JsonStringEncoder; +import java.io.Closeable; +import java.io.IOException; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ExecutionException; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.either; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; public abstract class AbstractQueryTestCase> extends ESTestCase { @@ -418,13 +418,13 @@ public abstract class AbstractQueryTestCase> } setSearchContext(randomTypes, context); Query secondLuceneQuery = rewriteQuery(secondQuery, context).toQuery(context); + assertNotNull("toQuery should not return null", secondLuceneQuery); assertLuceneQuery(secondQuery, secondLuceneQuery, context); SearchContext.removeCurrent(); assertEquals("two equivalent query builders lead to different lucene queries", rewrite(secondLuceneQuery), rewrite(firstLuceneQuery)); - // if the initial lucene query is null, changing its boost won't have any effect, we shouldn't test that - if (firstLuceneQuery != null && supportsBoostAndQueryName()) { + if (supportsBoostAndQueryName()) { secondQuery.boost(firstQuery.boost() + 1f + randomFloat()); setSearchContext(randomTypes, context); Query thirdLuceneQuery = rewriteQuery(secondQuery, context).toQuery(context);