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
This commit is contained in:
Britta Weber 2014-08-19 22:48:15 +02:00
parent a92300c5b5
commit 238efe505b
9 changed files with 193 additions and 6 deletions

View File

@ -21,6 +21,7 @@ package org.elasticsearch.index.query;
import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.Query; import org.apache.lucene.search.Query;
import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.lucene.search.Queries;
@ -131,7 +132,7 @@ public class BoolQueryParser implements QueryParser {
} }
if (clauses.isEmpty()) { if (clauses.isEmpty()) {
return null; return new MatchAllDocsQuery();
} }
BooleanQuery booleanQuery = new BooleanQuery(disableCoord); BooleanQuery booleanQuery = new BooleanQuery(disableCoord);

View File

@ -45,15 +45,20 @@ import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.unit.Fuzziness; 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.cache.filter.support.CacheKeyFilter;
import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.core.NumberFieldMapper; import org.elasticsearch.index.mapper.core.NumberFieldMapper;
import org.elasticsearch.index.search.NumericRangeFieldDataFilter; 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.GeoDistanceFilter;
import org.elasticsearch.index.search.geo.GeoPolygonFilter; import org.elasticsearch.index.search.geo.GeoPolygonFilter;
import org.elasticsearch.index.search.geo.InMemoryGeoBoundingBoxFilter; import org.elasticsearch.index.search.geo.InMemoryGeoBoundingBoxFilter;
import org.elasticsearch.index.search.morelikethis.MoreLikeThisFetchService; import org.elasticsearch.index.search.morelikethis.MoreLikeThisFetchService;
import org.elasticsearch.index.service.IndexService; import org.elasticsearch.index.service.IndexService;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.ElasticsearchSingleNodeTest; import org.elasticsearch.test.ElasticsearchSingleNodeTest;
import org.hamcrest.Matchers; import org.hamcrest.Matchers;
import org.junit.Before; 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.copyToBytesFromClasspath;
import static org.elasticsearch.common.io.Streams.copyToStringFromClasspath; 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.FilterBuilders.*;
import static org.elasticsearch.index.query.QueryBuilders.*; import static org.elasticsearch.index.query.QueryBuilders.*;
import static org.elasticsearch.index.query.RegexpFlag.*; import static org.elasticsearch.index.query.RegexpFlag.*;
@ -2289,5 +2295,42 @@ public class SimpleIndexQueryParserTests extends ElasticsearchSingleNodeTest {
assertThat( ((FuzzyQuery) parsedQuery).getTranspositions(), equalTo(false)); 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();
}
} }

View File

@ -0,0 +1,17 @@
{
"filtered": {
"filter": {
"has_parent": {
"type": "foo",
"query": {
"bool": {
"must": [],
"must_not": [],
"should": []
}
}
},
"query": []
}
}
}

View File

@ -0,0 +1,16 @@
{
"fquery": {
"query": {
"filtered": {
"query": {
"bool": {}
},
"filter": {
"term": {
"text": "apache"
}
}
}
}
}
}

View File

@ -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));
}
}

View File

@ -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"
}
}
}
}
}
}
}
}
}
}
}

View File

@ -59,6 +59,7 @@ import java.util.concurrent.atomic.AtomicReference;
import static com.google.common.collect.Maps.newHashMap; import static com.google.common.collect.Maps.newHashMap;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; 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.builder;
import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@ -138,12 +139,13 @@ public class SimpleChildQuerySearchTests extends ElasticsearchIntegrationTest {
// index simple data // index simple data
client().prepareIndex("test", "foo", "1").setSource("foo", 1).get(); 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(); refresh();
String query = copyToStringFromClasspath("/org/elasticsearch/search/child/bool-query-with-empty-clauses.json");
SearchResponse searchResponse = client().prepareSearch("test").setSource("{\"query\":{\"filtered\":{\"filter\":{\"has_parent\":{\"type\":\"test\",\"query\":{\"bool\":{\"must\":[],\"must_not\":[],\"should\":[]}}},\"query\":[]}}}}").get(); SearchResponse searchResponse = client().prepareSearch("test").setSource(query).get();
assertNoFailures(searchResponse); assertNoFailures(searchResponse);
assertThat(searchResponse.getHits().totalHits(), equalTo(2l)); assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
assertThat(searchResponse.getHits().getAt(0).getId(), equalTo("2"));
} }
@Test @Test

View File

@ -0,0 +1,19 @@
{
"query": {
"filtered": {
"filter": {
"has_parent": {
"type": "foo",
"query": {
"bool": {
"must": [],
"must_not": [],
"should": []
}
}
},
"query": []
}
}
}
}

View File

@ -160,7 +160,7 @@ public abstract class ElasticsearchSingleNodeTest extends ElasticsearchTestCase
return createIndex(index, createIndexRequestBuilder); return createIndex(index, createIndexRequestBuilder);
} }
private static IndexService createIndex(String index, CreateIndexRequestBuilder createIndexRequestBuilder) { protected static IndexService createIndex(String index, CreateIndexRequestBuilder createIndexRequestBuilder) {
assertAcked(createIndexRequestBuilder.get()); assertAcked(createIndexRequestBuilder.get());
// Wait for the index to be allocated so that cluster state updates don't override // Wait for the index to be allocated so that cluster state updates don't override
// changes that would have been done locally // changes that would have been done locally