From 74aca756b80ff5ad5e633ab54f184cc3ec1ba7a0 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 3 Dec 2018 11:49:11 +0100 Subject: [PATCH] Remove the distinction between query and filter context in QueryBuilders (#35354) When building a query Lucene distinguishes two cases, queries that require to produce a score and queries that only need to match. We cloned this mechanism in the QueryBuilders in order to be able to produce different queries based on whether they need to produce a score or not. However the only case in es that require this distinction is the BoolQueryBuilder that sets a different minimum_should_match when a `bool` query is built in a filter context.. This behavior doesn't seem right because it makes the matching of `should` clauses different when the score is not required. Closes #35293 --- .../migration/migrate_7_0/search.asciidoc | 11 +++ docs/reference/query-dsl/bool-query.asciidoc | 17 +---- .../cluster/metadata/AliasValidator.java | 2 +- .../index/query/AbstractQueryBuilder.java | 13 ---- .../index/query/BoolQueryBuilder.java | 18 +---- .../query/ConstantScoreQueryBuilder.java | 2 +- .../index/query/FuzzyQueryBuilder.java | 3 - .../index/query/QueryBuilder.java | 10 --- .../index/query/QueryShardContext.java | 28 -------- .../index/query/SpanNearQueryBuilder.java | 5 -- .../search/DefaultSearchContext.java | 2 +- .../AdjacencyMatrixAggregatorFactory.java | 2 +- .../filter/FilterAggregatorFactory.java | 2 +- .../filter/FiltersAggregatorFactory.java | 2 +- .../SignificantTermsAggregatorFactory.java | 2 +- .../search/sort/SortBuilder.java | 4 +- .../index/query/BoolQueryBuilderTests.java | 45 ------------ .../query/SpanMultiTermQueryBuilderTests.java | 5 -- .../query/plugin/CustomQueryParserIT.java | 68 ------------------- .../index/query/plugin/DummyQueryBuilder.java | 4 +- .../query/plugin/DummyQueryParserPlugin.java | 5 -- .../bucket/filter/FilterAggregatorTests.java | 25 ------- .../bucket/filter/FiltersAggregatorTests.java | 24 ------- .../SignificantTermsAggregatorTests.java | 28 -------- .../aggregations/AggregatorTestCase.java | 3 - .../test/AbstractQueryTestCase.java | 6 -- .../SecurityIndexSearcherWrapper.java | 2 +- ...yIndexSearcherWrapperIntegrationTests.java | 2 +- 28 files changed, 26 insertions(+), 314 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc index 348763762aa..5774f17c3e3 100644 --- a/docs/reference/migration/migrate_7_0/search.asciidoc +++ b/docs/reference/migration/migrate_7_0/search.asciidoc @@ -159,3 +159,14 @@ Negative scores in the Function Score Query are deprecated in 6.x, and are not allowed in this version. If a negative score is produced as a result of computation (e.g. in `script_score` or `field_value_factor` functions), an error will be thrown. + +[float] +==== The filter context has been removed + +The `filter` context has been removed from Elasticsearch's query builders, +the distinction between queries and filters is now decided in Lucene depending +on whether queries need to access score or not. As a result `bool` queries with +`should` clauses that don't need to access the score will no longer set their +`minimum_should_match` to 1. This behavior has been deprecated in the previous +major version. + diff --git a/docs/reference/query-dsl/bool-query.asciidoc b/docs/reference/query-dsl/bool-query.asciidoc index a7092aaaab1..d4b59194548 100644 --- a/docs/reference/query-dsl/bool-query.asciidoc +++ b/docs/reference/query-dsl/bool-query.asciidoc @@ -17,15 +17,7 @@ contribute to the score. in <>, meaning that scoring is ignored and clauses are considered for caching. -|`should` |The clause (query) should appear in the matching document. If the -`bool` query is in a <> and has a `must` or -`filter` clause then a document will match the `bool` query even if none of the -`should` queries match. In this case these clauses are only used to influence -the score. If the `bool` query is a <> -or has neither `must` or `filter` then at least one of the `should` queries -must match a document for it to match the `bool` query. This behavior may be -explicitly controlled by settings the -<> parameter. +|`should` |The clause (query) should appear in the matching document. |`must_not` |The clause (query) must not appear in the matching documents. Clauses are executed in <> meaning @@ -33,13 +25,6 @@ that scoring is ignored and clauses are considered for caching. Because scoring ignored, a score of `0` for all documents is returned. |======================================================================= -[IMPORTANT] -.Bool query in filter context -======================================================================== -If this query is used in a filter context and it has `should` -clauses then at least one `should` clause is required to match. -======================================================================== - The `bool` query takes a _more-matches-is-better_ approach, so the score from each matching `must` or `should` clause will be added together to provide the final `_score` for each document. diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java index c9258806d51..ebcffacf0a6 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java @@ -139,6 +139,6 @@ public class AliasValidator { private static void validateAliasFilter(XContentParser parser, QueryShardContext queryShardContext) throws IOException { QueryBuilder parseInnerQueryBuilder = parseInnerQueryBuilder(parser); QueryBuilder queryBuilder = Rewriteable.rewrite(parseInnerQueryBuilder, queryShardContext, true); - queryBuilder.toFilter(queryShardContext); + queryBuilder.toQuery(queryShardContext); } } diff --git a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index c0fb00128e5..cca1ca0fcc0 100644 --- a/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -111,19 +111,6 @@ public abstract class AbstractQueryBuilder> return query; } - @Override - public final Query toFilter(QueryShardContext context) throws IOException { - Query result; - final boolean originalIsFilter = context.isFilter(); - try { - context.setIsFilter(true); - result = toQuery(context); - } finally { - context.setIsFilter(originalIsFilter); - } - return result; - } - protected abstract Query doToQuery(QueryShardContext context) throws IOException; /** diff --git a/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index ee1779d3d51..4c3c8944cf3 100644 --- a/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -384,12 +384,6 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { return new MatchAllDocsQuery(); } - final String minimumShouldMatch; - if (context.isFilter() && this.minimumShouldMatch == null && shouldClauses.size() > 0) { - minimumShouldMatch = "1"; - } else { - minimumShouldMatch = this.minimumShouldMatch; - } Query query = Queries.applyMinimumShouldMatch(booleanQuery, minimumShouldMatch); return adjustPureNegative ? fixNegativeQueryIfNeeded(query) : query; } @@ -397,17 +391,7 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { private static void addBooleanClauses(QueryShardContext context, BooleanQuery.Builder booleanQueryBuilder, List clauses, Occur occurs) throws IOException { for (QueryBuilder query : clauses) { - Query luceneQuery = null; - switch (occurs) { - case MUST: - case SHOULD: - luceneQuery = query.toQuery(context); - break; - case FILTER: - case MUST_NOT: - luceneQuery = query.toFilter(context); - break; - } + Query luceneQuery = query.toQuery(context); booleanQueryBuilder.add(new BooleanClause(luceneQuery, occurs)); } } diff --git a/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java index e8d98a6b00b..9752265a701 100644 --- a/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -129,7 +129,7 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder i protected Query doToQuery(QueryShardContext context) throws IOException { Query query = null; String rewrite = this.rewrite; - if (rewrite == null && context.isFilter()) { - rewrite = QueryParsers.CONSTANT_SCORE.getPreferredName(); - } MappedFieldType fieldType = context.fieldMapper(fieldName); if (fieldType != null) { query = fieldType.fuzzyQuery(value, fuzziness, prefixLength, maxExpansions, transpositions); diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/QueryBuilder.java index 5b765c5cbda..19adfebafab 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryBuilder.java @@ -37,16 +37,6 @@ public interface QueryBuilder extends NamedWriteable, ToXContentObject, Rewritea */ Query toQuery(QueryShardContext context) throws IOException; - /** - * Converts this QueryBuilder to an unscored lucene {@link Query} that acts as a filter. - * Returns {@code null} if this query should be ignored in the context of - * parent queries. - * - * @param context additional information needed to construct the queries - * @return the {@link Query} or {@code null} if this query should be ignored upstream - */ - Query toFilter(QueryShardContext context) throws IOException; - /** * Sets the arbitrary name to be assigned to the query (see named queries). * Implementers should return the concrete type of the diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index ac19298ae32..82bae93e84d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -97,7 +97,6 @@ public class QueryShardContext extends QueryRewriteContext { private boolean allowUnmappedFields; private boolean mapUnmappedFieldAsString; private NestedScope nestedScope; - private boolean isFilter; public QueryShardContext(int shardId, IndexSettings indexSettings, BitsetFilterCache bitsetFilterCache, BiFunction> indexFieldDataLookup, MapperService mapperService, @@ -132,7 +131,6 @@ public class QueryShardContext extends QueryRewriteContext { this.lookup = null; this.namedQueries.clear(); this.nestedScope = new NestedScope(); - this.isFilter = false; } public IndexAnalyzers getIndexAnalyzers() { @@ -178,22 +176,6 @@ public class QueryShardContext extends QueryRewriteContext { return unmodifiableMap(new HashMap<>(namedQueries)); } - /** - * Return whether we are currently parsing a filter or a query. - */ - public boolean isFilter() { - return isFilter; - } - - /** - * Public for testing only! - * - * Sets whether we are currently parsing a filter or a query - */ - public void setIsFilter(boolean isFilter) { - this.isFilter = isFilter; - } - /** * Returns all the fields that match a given pattern. If prefixed with a * type then the fields will be returned with a type prefix. @@ -289,16 +271,6 @@ public class QueryShardContext extends QueryRewriteContext { return indexSettings.getIndexVersionCreated(); } - public ParsedQuery toFilter(QueryBuilder queryBuilder) { - return toQuery(queryBuilder, q -> { - Query filter = q.toFilter(this); - if (filter == null) { - return null; - } - return filter; - }); - } - public ParsedQuery toQuery(QueryBuilder queryBuilder) { return toQuery(queryBuilder, q -> { Query query = q.toQuery(this); diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java index d43c8120fe0..087718ed1bc 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java @@ -346,11 +346,6 @@ public class SpanNearQueryBuilder extends AbstractQueryBuilder parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, context, parent, subFactoriesBuilder, metaData); - filter = filterBuilder.toFilter(context.getQueryShardContext()); + filter = filterBuilder.toQuery(context.getQueryShardContext()); } /** diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java index 81a78632d4b..cc299765621 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorFactory.java @@ -56,7 +56,7 @@ public class FiltersAggregatorFactory extends AggregatorFactory> implements NamedWrit assert nestedFilter == Rewriteable.rewrite(nestedFilter, context) : "nested filter is not rewritten"; if (parentQuery == null) { // this is for back-compat, original single level nested sorting never applied a nested type filter - childQuery = nestedFilter.toFilter(context); + childQuery = nestedFilter.toQuery(context); } else { - childQuery = Queries.filtered(nestedObjectMapper.nestedTypeFilter(), nestedFilter.toFilter(context)); + childQuery = Queries.filtered(nestedObjectMapper.nestedTypeFilter(), nestedFilter.toQuery(context)); } } else { childQuery = nestedObjectMapper.nestedTypeFilter(); diff --git a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 46c9f56c33a..6288c3c9566 100644 --- a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -21,7 +21,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParsingException; @@ -41,7 +40,6 @@ import java.util.List; import java.util.Map; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; -import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; @@ -176,26 +174,6 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase { throw new UnsupportedOperationException(); }, null); } - - //see #11120 - public void testConstantScoreParsesFilter() throws Exception { - Query q = constantScoreQuery(new DummyQueryBuilder()).toQuery(queryShardContext()); - Query inner = ((ConstantScoreQuery) q).getQuery(); - assertThat(inner, instanceOf(DummyQueryParserPlugin.DummyQuery.class)); - assertEquals(true, ((DummyQueryParserPlugin.DummyQuery) inner).isFilter); - } - - //see #11120 - public void testBooleanParsesFilter() throws Exception { - // single clause, serialized as inner object - Query q = boolQuery() - .should(new DummyQueryBuilder()) - .must(new DummyQueryBuilder()) - .filter(new DummyQueryBuilder()) - .mustNot(new DummyQueryBuilder()).toQuery(queryShardContext()); - assertThat(q, instanceOf(BooleanQuery.class)); - BooleanQuery bq = (BooleanQuery) q; - assertEquals(4, bq.clauses().size()); - for (BooleanClause clause : bq.clauses()) { - DummyQueryParserPlugin.DummyQuery dummy = (DummyQueryParserPlugin.DummyQuery) clause.getQuery(); - switch (clause.getOccur()) { - case FILTER: - case MUST_NOT: - assertEquals(true, dummy.isFilter); - break; - case MUST: - case SHOULD: - assertEquals(false, dummy.isFilter); - break; - default: - throw new AssertionError(); - } - } - - // multiple clauses, serialized as inner arrays - q = boolQuery() - .should(new DummyQueryBuilder()).should(new DummyQueryBuilder()) - .must(new DummyQueryBuilder()).must(new DummyQueryBuilder()) - .filter(new DummyQueryBuilder()).filter(new DummyQueryBuilder()) - .mustNot(new DummyQueryBuilder()).mustNot(new DummyQueryBuilder()).toQuery(queryShardContext()); - assertThat(q, instanceOf(BooleanQuery.class)); - bq = (BooleanQuery) q; - assertEquals(8, bq.clauses().size()); - for (BooleanClause clause : bq.clauses()) { - DummyQueryParserPlugin.DummyQuery dummy = (DummyQueryParserPlugin.DummyQuery) clause.getQuery(); - switch (clause.getOccur()) { - case FILTER: - case MUST_NOT: - assertEquals(true, dummy.isFilter); - break; - case MUST: - case SHOULD: - assertEquals(false, dummy.isFilter); - break; - default: - throw new AssertionError(); - } - } - } } diff --git a/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java b/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java index cbd76877ce4..16f7726a877 100644 --- a/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java +++ b/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java @@ -58,7 +58,7 @@ public class DummyQueryBuilder extends AbstractQueryBuilder { @Override protected Query doToQuery(QueryShardContext context) throws IOException { - return new DummyQuery(context.isFilter()); + return new DummyQuery(); } @Override @@ -75,4 +75,4 @@ public class DummyQueryBuilder extends AbstractQueryBuilder { public String getWriteableName() { return NAME; } -} \ No newline at end of file +} diff --git a/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java b/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java index 02653dcfd0e..3f57712a51b 100644 --- a/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java +++ b/server/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java @@ -40,13 +40,8 @@ public class DummyQueryParserPlugin extends Plugin implements SearchPlugin { } public static class DummyQuery extends Query { - public final boolean isFilter; private final Query matchAllDocsQuery = new MatchAllDocsQuery(); - public DummyQuery(boolean isFilter) { - this.isFilter = isFilter; - } - @Override public String toString(String field) { return getClass().getSimpleName(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java index f3d057d8e8c..d92fb7ff62e 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregatorTests.java @@ -23,24 +23,17 @@ 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.hamcrest.Matchers; import org.junit.Before; -import java.io.IOException; - public class FilterAggregatorTests extends AggregatorTestCase { private MappedFieldType fieldType; @@ -107,22 +100,4 @@ 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.getWeight().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()); - } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java index 6fdf207249f..05ed0919782 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java @@ -23,23 +23,17 @@ 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.hamcrest.Matchers; import org.junit.Before; -import java.io.IOException; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -203,22 +197,4 @@ 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.getWeights()[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()); - } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java index 0485d4f5855..72403f8f782 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsAggregatorTests.java @@ -28,11 +28,8 @@ import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; -import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.Term; -import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.IndexSearcher; -import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; @@ -46,12 +43,9 @@ import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType; import org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType; 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.significant.SignificantTermsAggregatorFactory.ExecutionMode; import org.elasticsearch.search.aggregations.bucket.terms.IncludeExclude; -import org.elasticsearch.search.aggregations.support.ValueType; -import org.hamcrest.Matchers; import org.junit.Before; import java.io.IOException; @@ -86,28 +80,6 @@ public class SignificantTermsAggregatorTests extends AggregatorTestCase { Function.identity())); } - 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()); - } - /** * Uses the significant terms aggregation to find the keywords in text fields */ diff --git a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java index cdf4a68b52f..3d230bf67f0 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java @@ -76,7 +76,6 @@ import org.elasticsearch.search.lookup.SearchLookup; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.InternalAggregationTestCase; import org.junit.After; -import org.mockito.Matchers; import java.io.IOException; import java.util.ArrayList; @@ -306,8 +305,6 @@ public abstract class AggregatorTestCase extends ESTestCase { QueryShardContext queryShardContext = mock(QueryShardContext.class); when(queryShardContext.getMapperService()).thenReturn(mapperService); NestedScope nestedScope = new NestedScope(); - when(queryShardContext.isFilter()).thenCallRealMethod(); - Mockito.doCallRealMethod().when(queryShardContext).setIsFilter(Matchers.anyBoolean()); when(queryShardContext.nestedScope()).thenReturn(nestedScope); return queryShardContext; } 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 2f46b8a887c..40c23b9d5b3 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -466,12 +466,6 @@ public abstract class AbstractQueryTestCase> assertNotEquals("modifying the boost doesn't affect the corresponding lucene query", rewrite(firstLuceneQuery), rewrite(thirdLuceneQuery)); } - - // check that context#isFilter is not changed by invoking toQuery/rewrite - boolean filterFlag = randomBoolean(); - context.setIsFilter(filterFlag); - rewriteQuery(firstQuery, context).toQuery(context); - assertEquals("isFilter should be unchanged", filterFlag, context.isFilter()); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java index dbb359bb70f..a8651701448 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapper.java @@ -136,7 +136,7 @@ public class SecurityIndexSearcherWrapper extends IndexSearcherWrapper { QueryBuilder queryBuilder = queryShardContext.parseInnerQueryBuilder(parser); verifyRoleQuery(queryBuilder); failIfQueryUsesClient(queryBuilder, queryShardContext); - Query roleQuery = queryShardContext.toFilter(queryBuilder).query(); + Query roleQuery = queryShardContext.toQuery(queryBuilder).query(); filter.add(roleQuery, SHOULD); if (queryShardContext.getMapperService().hasNested()) { NestedHelper nestedHelper = new NestedHelper(queryShardContext.getMapperService()); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperIntegrationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperIntegrationTests.java index 90b0c6ee773..ff132894af8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperIntegrationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperIntegrationTests.java @@ -142,7 +142,7 @@ public class SecurityIndexSearcherWrapperIntegrationTests extends ESTestCase { for (int i = 0; i < numValues; i++) { ParsedQuery parsedQuery = new ParsedQuery(new TermQuery(new Term("field", values[i]))); doReturn(new TermQueryBuilder("field", values[i])).when(queryShardContext).parseInnerQueryBuilder(any(XContentParser.class)); - when(queryShardContext.toFilter(new TermsQueryBuilder("field", values[i]))).thenReturn(parsedQuery); + when(queryShardContext.toQuery(new TermsQueryBuilder("field", values[i]))).thenReturn(parsedQuery); DirectoryReader wrappedDirectoryReader = wrapper.wrap(directoryReader); IndexSearcher indexSearcher = wrapper.wrap(new IndexSearcher(wrappedDirectoryReader));