From 238efe505b6a511e324073d4ecfd7c6ec675a618 Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Tue, 19 Aug 2014 22:48:15 +0200 Subject: [PATCH] bool query: parser should return match_all in case there are no clauses This also fixes has_parent filters with a nested empty bool filter (see test SimpleChildQuerySearchTests#test6722, the test should actually expect either 0 results when searching for has_parent "test" or one result when search for has_parent "foo") closes #7240 closes #7347 --- .../index/query/BoolQueryParser.java | 3 +- .../query/SimpleIndexQueryParserTests.java | 43 ++++++++++++++ ...-query-with-empty-clauses-for-parsing.json | 17 ++++++ .../query/fquery-with-empty-bool-query.json | 16 ++++++ .../bucket/DedicatedAggregationTests.java | 56 +++++++++++++++++++ .../bucket/agg-filter-with-empty-bool.json | 33 +++++++++++ .../child/SimpleChildQuerySearchTests.java | 10 ++-- .../child/bool-query-with-empty-clauses.json | 19 +++++++ .../test/ElasticsearchSingleNodeTest.java | 2 +- 9 files changed, 193 insertions(+), 6 deletions(-) create mode 100644 src/test/java/org/elasticsearch/index/query/bool-query-with-empty-clauses-for-parsing.json create mode 100644 src/test/java/org/elasticsearch/index/query/fquery-with-empty-bool-query.json create mode 100644 src/test/java/org/elasticsearch/search/aggregations/bucket/DedicatedAggregationTests.java create mode 100644 src/test/java/org/elasticsearch/search/aggregations/bucket/agg-filter-with-empty-bool.json create mode 100644 src/test/java/org/elasticsearch/search/child/bool-query-with-empty-clauses.json diff --git a/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java b/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java index 243b2afe51e..29d4ba2edd5 100644 --- a/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java +++ b/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java @@ -21,6 +21,7 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.lucene.search.Queries; @@ -131,7 +132,7 @@ public class BoolQueryParser implements QueryParser { } if (clauses.isEmpty()) { - return null; + return new MatchAllDocsQuery(); } BooleanQuery booleanQuery = new BooleanQuery(disableCoord); diff --git a/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java b/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java index 596d29b4825..7488719b05c 100644 --- a/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java +++ b/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java @@ -45,15 +45,20 @@ import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.unit.Fuzziness; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.cache.filter.support.CacheKeyFilter; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.core.NumberFieldMapper; import org.elasticsearch.index.search.NumericRangeFieldDataFilter; +import org.elasticsearch.index.search.child.CustomQueryWrappingFilter; +import org.elasticsearch.index.search.child.ParentConstantScoreQuery; import org.elasticsearch.index.search.geo.GeoDistanceFilter; import org.elasticsearch.index.search.geo.GeoPolygonFilter; import org.elasticsearch.index.search.geo.InMemoryGeoBoundingBoxFilter; import org.elasticsearch.index.search.morelikethis.MoreLikeThisFetchService; import org.elasticsearch.index.service.IndexService; +import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.ElasticsearchSingleNodeTest; import org.hamcrest.Matchers; import org.junit.Before; @@ -67,6 +72,7 @@ import java.util.List; import static org.elasticsearch.common.io.Streams.copyToBytesFromClasspath; import static org.elasticsearch.common.io.Streams.copyToStringFromClasspath; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.FilterBuilders.*; import static org.elasticsearch.index.query.QueryBuilders.*; import static org.elasticsearch.index.query.RegexpFlag.*; @@ -2289,5 +2295,42 @@ public class SimpleIndexQueryParserTests extends ElasticsearchSingleNodeTest { assertThat( ((FuzzyQuery) parsedQuery).getTranspositions(), equalTo(false)); } + // https://github.com/elasticsearch/elasticsearch/issues/7240 + @Test + public void testEmptyBooleanQuery() throws Exception { + IndexQueryParserService queryParser = queryParser(); + String query = jsonBuilder().startObject().startObject("bool").endObject().endObject().string(); + Query parsedQuery = queryParser.parse(query).query(); + assertThat(parsedQuery, instanceOf(MatchAllDocsQuery.class)); + } + // https://github.com/elasticsearch/elasticsearch/issues/7240 + @Test + public void testEmptyBooleanQueryInsideFQuery() throws Exception { + IndexQueryParserService queryParser = queryParser(); + String query = copyToStringFromClasspath("/org/elasticsearch/index/query/fquery-with-empty-bool-query.json"); + XContentParser parser = XContentHelper.createParser(new BytesArray(query)); + ParsedFilter parsedQuery = queryParser.parseInnerFilter(parser); + assertThat(parsedQuery.filter(), instanceOf(QueryWrapperFilter.class)); + assertThat(((QueryWrapperFilter) parsedQuery.filter()).getQuery(), instanceOf(XFilteredQuery.class)); + assertThat(((XFilteredQuery) ((QueryWrapperFilter) parsedQuery.filter()).getQuery()).getFilter(), instanceOf(TermFilter.class)); + TermFilter filter = (TermFilter) ((XFilteredQuery) ((QueryWrapperFilter) parsedQuery.filter()).getQuery()).getFilter(); + assertThat(filter.getTerm().toString(), equalTo("text:apache")); + } + + // https://github.com/elasticsearch/elasticsearch/issues/6722 + public void testEmptyBoolSubClausesIsMatchAll() throws ElasticsearchException, IOException { + String query = copyToStringFromClasspath("/org/elasticsearch/index/query/bool-query-with-empty-clauses-for-parsing.json"); + IndexService indexService = createIndex("testidx", client().admin().indices().prepareCreate("testidx") + .addMapping("foo") + .addMapping("test", "_parent", "type=foo")); + SearchContext.setCurrent(createSearchContext(indexService)); + IndexQueryParserService queryParser = indexService.queryParserService(); + Query parsedQuery = queryParser.parse(query).query(); + assertThat(parsedQuery, instanceOf(XConstantScoreQuery.class)); + assertThat(((XConstantScoreQuery) parsedQuery).getFilter(), instanceOf(CustomQueryWrappingFilter.class)); + assertThat(((CustomQueryWrappingFilter) ((XConstantScoreQuery) parsedQuery).getFilter()).getQuery(), instanceOf(ParentConstantScoreQuery.class)); + assertThat(((CustomQueryWrappingFilter) ((XConstantScoreQuery) parsedQuery).getFilter()).getQuery().toString(), equalTo("parent_filter[foo](*:*)")); + SearchContext.removeCurrent(); + } } diff --git a/src/test/java/org/elasticsearch/index/query/bool-query-with-empty-clauses-for-parsing.json b/src/test/java/org/elasticsearch/index/query/bool-query-with-empty-clauses-for-parsing.json new file mode 100644 index 00000000000..8071fa96ab5 --- /dev/null +++ b/src/test/java/org/elasticsearch/index/query/bool-query-with-empty-clauses-for-parsing.json @@ -0,0 +1,17 @@ +{ + "filtered": { + "filter": { + "has_parent": { + "type": "foo", + "query": { + "bool": { + "must": [], + "must_not": [], + "should": [] + } + } + }, + "query": [] + } + } +} \ No newline at end of file diff --git a/src/test/java/org/elasticsearch/index/query/fquery-with-empty-bool-query.json b/src/test/java/org/elasticsearch/index/query/fquery-with-empty-bool-query.json new file mode 100644 index 00000000000..58efd910a76 --- /dev/null +++ b/src/test/java/org/elasticsearch/index/query/fquery-with-empty-bool-query.json @@ -0,0 +1,16 @@ +{ + "fquery": { + "query": { + "filtered": { + "query": { + "bool": {} + }, + "filter": { + "term": { + "text": "apache" + } + } + } + } + } +} \ No newline at end of file diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/DedicatedAggregationTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/DedicatedAggregationTests.java new file mode 100644 index 00000000000..9e3fed6b49b --- /dev/null +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/DedicatedAggregationTests.java @@ -0,0 +1,56 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.search.aggregations.bucket; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.search.aggregations.bucket.filter.Filter; +import org.elasticsearch.search.aggregations.bucket.terms.StringTerms; +import org.elasticsearch.test.ElasticsearchIntegrationTest; +import org.junit.Test; + +import java.io.IOException; + +import static org.elasticsearch.common.io.Streams.copyToStringFromClasspath; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; + + +public class DedicatedAggregationTests extends ElasticsearchIntegrationTest { + + // https://github.com/elasticsearch/elasticsearch/issues/7240 + @Test + public void testEmptyBoolIsMatchAll() throws IOException { + String query = copyToStringFromClasspath("/org/elasticsearch/search/aggregations/bucket/agg-filter-with-empty-bool.json"); + createIndex("testidx"); + index("testidx", "apache", "1", "field", "text"); + index("testidx", "nginx", "2", "field", "text"); + refresh(); + ensureGreen("testidx"); + SearchResponse searchResponse = client().prepareSearch("testidx").setQuery(matchAllQuery()).get(); + assertThat(searchResponse.getHits().getTotalHits(), equalTo(2l)); + searchResponse = client().prepareSearch("testidx").setSource(query).get(); + assertSearchResponse(searchResponse); + assertThat(searchResponse.getAggregations().getAsMap().get("issue7240"), instanceOf(Filter.class)); + Filter filterAgg = (Filter) searchResponse.getAggregations().getAsMap().get("issue7240"); + assertThat(filterAgg.getAggregations().getAsMap().get("terms"), instanceOf(StringTerms.class)); + assertThat(((StringTerms) filterAgg.getAggregations().getAsMap().get("terms")).getBuckets().get(0).getDocCount(), equalTo(1l)); + } +} diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/agg-filter-with-empty-bool.json b/src/test/java/org/elasticsearch/search/aggregations/bucket/agg-filter-with-empty-bool.json new file mode 100644 index 00000000000..f730b43c49e --- /dev/null +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/agg-filter-with-empty-bool.json @@ -0,0 +1,33 @@ +{ + "aggs": { + "issue7240": { + "aggs": { + "terms": { + "terms": { + "field": "field" + } + } + }, + "filter": { + "fquery": { + "query": { + "filtered": { + "query": { + "bool": {} + }, + "filter": { + "fquery": { + "query": { + "query_string": { + "query": "_type:apache" + } + } + } + } + } + } + } + } + } + } +} diff --git a/src/test/java/org/elasticsearch/search/child/SimpleChildQuerySearchTests.java b/src/test/java/org/elasticsearch/search/child/SimpleChildQuerySearchTests.java index 661b7a2fd77..2f143f4a4b1 100644 --- a/src/test/java/org/elasticsearch/search/child/SimpleChildQuerySearchTests.java +++ b/src/test/java/org/elasticsearch/search/child/SimpleChildQuerySearchTests.java @@ -59,6 +59,7 @@ import java.util.concurrent.atomic.AtomicReference; import static com.google.common.collect.Maps.newHashMap; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; +import static org.elasticsearch.common.io.Streams.copyToStringFromClasspath; import static org.elasticsearch.common.settings.ImmutableSettings.builder; import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -138,12 +139,13 @@ public class SimpleChildQuerySearchTests extends ElasticsearchIntegrationTest { // index simple data client().prepareIndex("test", "foo", "1").setSource("foo", 1).get(); - client().prepareIndex("test", "test").setSource("foo", 1).setParent("1").get(); + client().prepareIndex("test", "test", "2").setSource("foo", 1).setParent("1").get(); refresh(); - - SearchResponse searchResponse = client().prepareSearch("test").setSource("{\"query\":{\"filtered\":{\"filter\":{\"has_parent\":{\"type\":\"test\",\"query\":{\"bool\":{\"must\":[],\"must_not\":[],\"should\":[]}}},\"query\":[]}}}}").get(); + String query = copyToStringFromClasspath("/org/elasticsearch/search/child/bool-query-with-empty-clauses.json"); + SearchResponse searchResponse = client().prepareSearch("test").setSource(query).get(); assertNoFailures(searchResponse); - assertThat(searchResponse.getHits().totalHits(), equalTo(2l)); + assertThat(searchResponse.getHits().totalHits(), equalTo(1l)); + assertThat(searchResponse.getHits().getAt(0).getId(), equalTo("2")); } @Test diff --git a/src/test/java/org/elasticsearch/search/child/bool-query-with-empty-clauses.json b/src/test/java/org/elasticsearch/search/child/bool-query-with-empty-clauses.json new file mode 100644 index 00000000000..844b5915a48 --- /dev/null +++ b/src/test/java/org/elasticsearch/search/child/bool-query-with-empty-clauses.json @@ -0,0 +1,19 @@ +{ +"query": { + "filtered": { + "filter": { + "has_parent": { + "type": "foo", + "query": { + "bool": { + "must": [], + "must_not": [], + "should": [] + } + } + }, + "query": [] + } + } +} +} \ No newline at end of file diff --git a/src/test/java/org/elasticsearch/test/ElasticsearchSingleNodeTest.java b/src/test/java/org/elasticsearch/test/ElasticsearchSingleNodeTest.java index f08c4503cc2..fd9e1fd8fe4 100644 --- a/src/test/java/org/elasticsearch/test/ElasticsearchSingleNodeTest.java +++ b/src/test/java/org/elasticsearch/test/ElasticsearchSingleNodeTest.java @@ -160,7 +160,7 @@ public abstract class ElasticsearchSingleNodeTest extends ElasticsearchTestCase return createIndex(index, createIndexRequestBuilder); } - private static IndexService createIndex(String index, CreateIndexRequestBuilder createIndexRequestBuilder) { + protected static IndexService createIndex(String index, CreateIndexRequestBuilder createIndexRequestBuilder) { assertAcked(createIndexRequestBuilder.get()); // Wait for the index to be allocated so that cluster state updates don't override // changes that would have been done locally