honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

The `filter` and `significant_terms` aggregations should parse the `filter` as a filter, not as a query. (#23797) 

This is important for some queries like `bool`, which are parsed differently
depending on whether we want to get a query or a filter.
This commit is contained in:
Adrien Grand 2017-04-05 16:46:21 +02:00 committed by GitHub
parent adccdbb3cf
commit d5d0f140d6
8 changed files with 149 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
buildSrc/src/main/resources/checkstyle_suppressions.xml
View File

@ -3089,7 +3089,7 @@
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]DiversifiedSamplerIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]DoubleTermsIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]FilterIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]FiltersAggregatorTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]filters[/\\]FiltersAggregatorTests.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]FiltersIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]GeoDistanceIT.java" checks="LineLength" />
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]aggregations[/\\]bucket[/\\]GeoHashGridIT.java" checks="LineLength" />

4
core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorFactory.java
View File

@ -35,13 +35,13 @@ import java.util.Map;
public class FilterAggregatorFactory extends AggregatorFactory<FilterAggregatorFactory> {
private final Weight weight;
final Weight weight;
public FilterAggregatorFactory(String name, QueryBuilder filterBuilder, SearchContext context,
AggregatorFactory<?> parent, AggregatorFactories.Builder subFactoriesBuilder, Map<String, Object> metaData) throws IOException {
super(name, context, parent, subFactoriesBuilder, metaData);
IndexSearcher contextSearcher = context.searcher();
Query filter = filterBuilder.toQuery(context.getQueryShardContext());
Query filter = filterBuilder.toFilter(context.getQueryShardContext());
weight = contextSearcher.createNormalizedWeight(filter, false);
}

2
core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorFactory.java
View File

@ -36,7 +36,7 @@ import java.util.Map;
public class FiltersAggregatorFactory extends AggregatorFactory<FiltersAggregatorFactory> {
private final String[] keys;
private final Weight[] weights;
final Weight[] weights;
private final boolean keyed;
private final boolean otherBucket;
private final String otherBucketKey;

4
core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorFactory.java
View File

@ -65,7 +65,7 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac
private MappedFieldType fieldType;
private FilterableTermsEnum termsEnum;
private int numberOfAggregatorsCreated;
private final Query filter;
final Query filter;
private final int supersetNumDocs;
private final TermsAggregator.BucketCountThresholds bucketCountThresholds;
private final SignificanceHeuristic significanceHeuristic;
@ -79,7 +79,7 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac
this.executionHint = executionHint;
this.filter = filterBuilder == null
? null
: filterBuilder.toQuery(context.getQueryShardContext());
: filterBuilder.toFilter(context.getQueryShardContext());
IndexSearcher searcher = context.searcher();
this.supersetNumDocs = filter == null
// Important - need to use the doc count that includes deleted docs

19
core/src/test/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java
View File

@ -51,6 +51,7 @@ import org.elasticsearch.index.query.support.NestedScope;
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache;
import org.elasticsearch.mock.orig.Mockito;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsFetchSubPhase;
import org.elasticsearch.search.fetch.subphase.FetchSourceSubPhase;
@ -58,6 +59,7 @@ import org.elasticsearch.search.internal.ContextIndexSearcher;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.test.ESTestCase;
import org.mockito.Matchers;
import java.io.IOException;
import java.util.ArrayList;
@ -80,9 +82,10 @@ public abstract class AggregatorTestCase extends ESTestCase {
private static final String NESTEDFIELD_PREFIX = "nested_";
private List<Releasable> releasables = new ArrayList<>();
protected <A extends Aggregator, B extends AggregationBuilder> A createAggregator(B aggregationBuilder,
IndexSearcher indexSearcher,
MappedFieldType... fieldTypes) throws IOException {
/** Create a factory for the given aggregation builder. */
protected AggregatorFactory<?> createAggregatorFactory(AggregationBuilder aggregationBuilder,
IndexSearcher indexSearcher,
MappedFieldType... fieldTypes) throws IOException {
IndexSettings indexSettings = createIndexSettings();
SearchContext searchContext = createSearchContext(indexSearcher, indexSettings);
CircuitBreakerService circuitBreakerService = new NoneCircuitBreakerService();
@ -102,8 +105,14 @@ public abstract class AggregatorTestCase extends ESTestCase {
QueryShardContext queryShardContext = queryShardContextMock(fieldTypes, indexSettings, circuitBreakerService);
when(searchContext.getQueryShardContext()).thenReturn(queryShardContext);
return aggregationBuilder.build(searchContext, null);
}
protected <A extends Aggregator> A createAggregator(AggregationBuilder aggregationBuilder,
IndexSearcher indexSearcher,
MappedFieldType... fieldTypes) throws IOException {
@SuppressWarnings("unchecked")
A aggregator = (A) aggregationBuilder.build(searchContext, null).create(null, true);
A aggregator = (A) createAggregatorFactory(aggregationBuilder, indexSearcher, fieldTypes).create(null, true);
return aggregator;
}
@ -176,6 +185,8 @@ public abstract class AggregatorTestCase extends ESTestCase {
new IndexFieldDataCache.None(), circuitBreakerService, mock(MapperService.class)));
}
NestedScope nestedScope = new NestedScope();
when(queryShardContext.isFilter()).thenCallRealMethod();
Mockito.doCallRealMethod().when(queryShardContext).setIsFilter(Matchers.anyBoolean());
when(queryShardContext.nestedScope()).thenReturn(nestedScope);
return queryShardContext;
}

28
core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterAggregatorTests.java → core/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java
View File

@ -16,26 +16,34 @@
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.search.aggregations.bucket;
package org.elasticsearch.search.aggregations.bucket.filter;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.MultiReader;
import org.apache.lucene.index.RandomIndexWriter;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.store.Directory;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.aggregations.AggregatorFactory;
import org.elasticsearch.search.aggregations.AggregatorTestCase;
import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregatorFactory;
import org.elasticsearch.search.aggregations.bucket.filter.InternalFilter;
import org.hamcrest.Matchers;
import org.junit.Before;
import java.io.IOException;
public class FilterAggregatorTests extends AggregatorTestCase {
private MappedFieldType fieldType;
@ -102,4 +110,22 @@ public class FilterAggregatorTests extends AggregatorTestCase {
indexReader.close();
directory.close();
}
public void testParsedAsFilter() throws IOException {
IndexReader indexReader = new MultiReader();
IndexSearcher indexSearcher = newSearcher(indexReader);
QueryBuilder filter = QueryBuilders.boolQuery()
.must(QueryBuilders.termQuery("field", "foo"))
.should(QueryBuilders.termQuery("field", "bar"));
FilterAggregationBuilder builder = new FilterAggregationBuilder("test", filter);
AggregatorFactory<?> factory = createAggregatorFactory(builder, indexSearcher, fieldType);
assertThat(factory, Matchers.instanceOf(FilterAggregatorFactory.class));
FilterAggregatorFactory filterFactory = (FilterAggregatorFactory) factory;
Query parsedQuery = filterFactory.weight.getQuery();
assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class));
assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size());
// means the bool query has been parsed as a filter, if it was a query minShouldMatch would
// be 0
assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch());
}
}

27
core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersAggregatorTests.java → core/src/test/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregatorTests.java
View File

@ -16,27 +16,34 @@
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.search.aggregations.bucket;
package org.elasticsearch.search.aggregations.bucket.filters;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.MultiReader;
import org.apache.lucene.index.RandomIndexWriter;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.store.Directory;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.aggregations.AggregatorFactory;
import org.elasticsearch.search.aggregations.AggregatorTestCase;
import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregator;
import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregatorFactory;
import org.elasticsearch.search.aggregations.bucket.filters.InternalFilters;
import org.hamcrest.Matchers;
import org.junit.Before;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@ -200,4 +207,22 @@ public class FiltersAggregatorTests extends AggregatorTestCase {
indexReader.close();
directory.close();
}
public void testParsedAsFilter() throws IOException {
IndexReader indexReader = new MultiReader();
IndexSearcher indexSearcher = newSearcher(indexReader);
QueryBuilder filter = QueryBuilders.boolQuery()
.must(QueryBuilders.termQuery("field", "foo"))
.should(QueryBuilders.termQuery("field", "bar"));
FiltersAggregationBuilder builder = new FiltersAggregationBuilder("test", filter);
AggregatorFactory<?> factory = createAggregatorFactory(builder, indexSearcher, fieldType);
assertThat(factory, Matchers.instanceOf(FiltersAggregatorFactory.class));
FiltersAggregatorFactory filtersFactory = (FiltersAggregatorFactory) factory;
Query parsedQuery = filtersFactory.weights[0].getQuery();
assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class));
assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size());
// means the bool query has been parsed as a filter, if it was a query minShouldMatch would
// be 0
assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch());
}
}

75
core/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java Normal file
View File

@ -0,0 +1,75 @@
/*
* 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.significant;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.MultiReader;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.aggregations.AggregatorFactory;
import org.elasticsearch.search.aggregations.AggregatorTestCase;
import org.elasticsearch.search.aggregations.support.ValueType;
import org.hamcrest.Matchers;
import org.junit.Before;
import java.io.IOException;
public class SignificantTermsAggregatorTests extends AggregatorTestCase {
private MappedFieldType fieldType;
@Before
public void setUpTest() throws Exception {
super.setUp();
fieldType = new KeywordFieldMapper.KeywordFieldType();
fieldType.setHasDocValues(true);
fieldType.setIndexOptions(IndexOptions.DOCS);
fieldType.setName("field");
}
public void testParsedAsFilter() throws IOException {
IndexReader indexReader = new MultiReader();
IndexSearcher indexSearcher = newSearcher(indexReader);
QueryBuilder filter = QueryBuilders.boolQuery()
.must(QueryBuilders.termQuery("field", "foo"))
.should(QueryBuilders.termQuery("field", "bar"));
SignificantTermsAggregationBuilder builder = new SignificantTermsAggregationBuilder(
"test", ValueType.STRING)
.field("field")
.backgroundFilter(filter);
AggregatorFactory<?> factory = createAggregatorFactory(builder, indexSearcher, fieldType);
assertThat(factory, Matchers.instanceOf(SignificantTermsAggregatorFactory.class));
SignificantTermsAggregatorFactory sigTermsFactory =
(SignificantTermsAggregatorFactory) factory;
Query parsedQuery = sigTermsFactory.filter;
assertThat(parsedQuery, Matchers.instanceOf(BooleanQuery.class));
assertEquals(2, ((BooleanQuery) parsedQuery).clauses().size());
// means the bool query has been parsed as a filter, if it was a query minShouldMatch would
// be 0
assertEquals(1, ((BooleanQuery) parsedQuery).getMinimumNumberShouldMatch());
}
}