From 8bdc7362a61eaf984f5e279d98033a7a5c7970ef Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 11 Feb 2016 10:45:49 +0100 Subject: [PATCH 1/6] Add infrastructure to rewrite query buidlers QueryBuilders today do all their heavy lifting in toQuery() which can be too late for several operations. For instance if we want to fetch geo shapes on the coordinating node we need to do all this before we create the actual lucene query which happens on the shard itself. Also optimizations for request caching need to be done to the query builder rather than the query which then in-turn needs to be serialized again. This commit adds the basic infrastructure for query rewriting and moves the heavy lifting into the rewrite method for the following queries: * `WrapperQueryBuilder` * `GeoShapeQueryBuilder` * `TermsQueryBuilder` * `TemplateQueryBuilder` Other queries like `MoreLikeThisQueryBuilder` still need to be fixed / converted. The nice sideeffect of this is that queries like template queries will now also match the request cache if their non-template equivalent has been cached befoore. In the future this will allow to add optimizataion like rewriting time-based queries into primitives like `match_all_docs` or `match_no_docs` based on the currents shards bounds. This is especially appealing for indices that are read-only ie. never change. --- .../action/search/TransportSearchAction.java | 2 +- .../cluster/metadata/AliasValidator.java | 6 +- .../org/elasticsearch/index/IndexService.java | 2 +- .../index/query/BoolQueryBuilder.java | 37 +++++++++++ .../index/query/BoostingQueryBuilder.java | 13 ++++ .../query/ConstantScoreQueryBuilder.java | 9 +++ .../index/query/EmptyQueryBuilder.java | 64 ++++++------------- .../index/query/GeoShapeQueryBuilder.java | 25 +++++--- .../index/query/HasChildQueryBuilder.java | 15 ++++- .../index/query/HasParentQueryBuilder.java | 13 +++- .../index/query/IndicesQueryBuilder.java | 10 +++ .../index/query/MoreLikeThisQueryBuilder.java | 6 ++ .../index/query/NestedQueryBuilder.java | 8 +++ .../index/query/QueryBuilder.java | 24 +++++++ .../index/query/QueryParseContext.java | 2 +- .../index/query/QueryRewriteContext.java | 63 ++++++++++++++++++ .../index/query/QueryShardContext.java | 55 ++++------------ .../index/query/ScriptQueryBuilder.java | 2 + .../index/query/TemplateQueryBuilder.java | 32 +++++++--- .../index/query/TermsQueryBuilder.java | 36 +++++++---- .../index/query/WrapperQueryBuilder.java | 26 +++++--- .../FunctionScoreQueryBuilder.java | 14 ++++ .../support/NestedInnerQueryParseSupport.java | 7 +- .../elasticsearch/index/shard/IndexShard.java | 2 +- .../elasticsearch/script/ScriptService.java | 4 ++ .../elasticsearch/search/SearchService.java | 4 +- .../action/SearchServiceTransportAction.java | 1 - .../search/builder/SearchSourceBuilder.java | 14 ++++ .../search/highlight/HighlightBuilder.java | 2 +- .../search/rescore/QueryRescorerBuilder.java | 4 +- .../index/query/AbstractQueryTestCase.java | 41 ++++++++++-- .../index/query/QueryShardContextTests.java | 5 +- .../query/TemplateQueryBuilderTests.java | 2 +- .../PercolateDocumentParserTests.java | 2 +- .../builder/SearchSourceBuilderTests.java | 15 +++++ .../highlight/HighlightBuilderTests.java | 2 +- .../rescore/QueryRescoreBuilderTests.java | 4 +- .../phrase/DirectCandidateGeneratorTests.java | 2 +- .../messy/tests/TemplateQueryParserTests.java | 26 ++++++-- 39 files changed, 441 insertions(+), 160 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java diff --git a/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index c106cd1d4e5..77afd123644 100644 --- a/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -33,6 +33,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.IndexClosedException; +import org.elasticsearch.search.SearchService; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -90,7 +91,6 @@ public class TransportSearchAction extends HandledTransportAction queryBuilder = QueryBuilder.rewriteQuery(queryParseContext.parseInnerQueryBuilder(), queryShardContext); + queryBuilder.toFilter(queryShardContext); } finally { queryShardContext.reset(null); parser.close(); diff --git a/core/src/main/java/org/elasticsearch/index/IndexService.java b/core/src/main/java/org/elasticsearch/index/IndexService.java index 9ffb6c4d56c..0048f2a98ad 100644 --- a/core/src/main/java/org/elasticsearch/index/IndexService.java +++ b/core/src/main/java/org/elasticsearch/index/IndexService.java @@ -421,7 +421,7 @@ public final class IndexService extends AbstractIndexComponent implements IndexC * Creates a new QueryShardContext. The context has not types set yet, if types are required set them via {@link QueryShardContext#setTypes(String...)} */ public QueryShardContext newQueryShardContext() { - return new QueryShardContext(indexSettings, nodeServicesProvider.getClient(), indexCache.bitsetFilterCache(), indexFieldData, mapperService(), similarityService(), nodeServicesProvider.getScriptService(), nodeServicesProvider.getIndicesQueriesRegistry()); + return new QueryShardContext(indexSettings, indexCache.bitsetFilterCache(), indexFieldData, mapperService(), similarityService(), nodeServicesProvider.getScriptService(), nodeServicesProvider.getIndicesQueriesRegistry()); } ThreadPool getThreadPool() { diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index f7f4926d950..72e13c2c31a 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.function.Consumer; import static org.elasticsearch.common.lucene.search.Queries.fixNegativeQueryIfNeeded; @@ -346,4 +347,40 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { out.writeBoolean(disableCoord); out.writeOptionalString(minimumShouldMatch); } + + @Override + public QueryBuilder rewrite(QueryRewriteContext queryRewriteContext) throws IOException { + BoolQueryBuilder newBuilder = new BoolQueryBuilder(); + boolean changed = false; + final int clauses = mustClauses.size() + mustNotClauses.size() + filterClauses.size() + shouldClauses.size(); + if (clauses == 0) { + return new MatchAllQueryBuilder().boost(boost()).queryName(queryName()); + } + changed |= rewriteClauses(queryRewriteContext, mustClauses, newBuilder::must); + changed |= rewriteClauses(queryRewriteContext, mustNotClauses, newBuilder::mustNot); + changed |= rewriteClauses(queryRewriteContext, filterClauses, newBuilder::filter); + changed |= rewriteClauses(queryRewriteContext, shouldClauses, newBuilder::should); + + if (changed) { + newBuilder.adjustPureNegative = adjustPureNegative; + newBuilder.disableCoord = disableCoord; + newBuilder.minimumShouldMatch = minimumShouldMatch; + newBuilder.boost(boost()); + newBuilder.queryName(queryName()); + return newBuilder; + } + return this; + } + + private static boolean rewriteClauses(QueryRewriteContext queryRewriteContext, List> builders, Consumer> conusmer) throws IOException { + boolean changed = false; + for (QueryBuilder builder : builders) { + QueryBuilder result = builder.rewrite(queryRewriteContext); + if (result != builder) { + changed = true; + } + conusmer.accept(result); + } + return changed; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java index 50346c2d36e..507e85687dc 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java @@ -158,4 +158,17 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryBuilder positiveQuery = this.positiveQuery.rewrite(queryShardContext); + QueryBuilder negativeQuery = this.negativeQuery.rewrite(queryShardContext); + if (positiveQuery != this.positiveQuery || negativeQuery != this.negativeQuery) { + BoostingQueryBuilder newQueryBuilder = new BoostingQueryBuilder(positiveQuery, negativeQuery) + .boost(boost()).queryName(queryName()); + newQueryBuilder.negativeBoost = negativeBoost; + return newQueryBuilder; + } + return this; + } } 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 4b054a34d40..ad00bba28b0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -104,4 +104,13 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryBuilder rewrite = filterBuilder.rewrite(queryShardContext); + if (rewrite != filterBuilder) { + return new ConstantScoreQueryBuilder(rewrite).boost(boost()).queryName(queryName()); + } + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java index 7d1761a44b2..83f7abae7ae 100644 --- a/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java @@ -23,10 +23,12 @@ import org.apache.lucene.search.Query; import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; +import java.util.Objects; /** * A {@link QueryBuilder} that is a stand in replacement for an empty query clause in the DSL. @@ -37,75 +39,49 @@ import java.io.IOException; * intended to be used internally as a stand-in for nested queries that are left empty and should * be ignored upstream. */ -public class EmptyQueryBuilder extends ToXContentToBytes implements QueryBuilder { +public final class EmptyQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "empty_query"; /** the one and only empty query builder */ public static final EmptyQueryBuilder PROTOTYPE = new EmptyQueryBuilder(); - // prevent instances other than prototype - private EmptyQueryBuilder() { - super(XContentType.JSON); - } - @Override public String getWriteableName() { return NAME; } + @Override + protected Query doToQuery(QueryShardContext context) throws IOException { + return null; + } + @Override public String getName() { return getWriteableName(); } @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.endObject(); - return builder; + protected void doXContent(XContentBuilder builder, Params params) throws IOException { } @Override - public Query toQuery(QueryShardContext context) throws IOException { - // empty - return null; + protected void doWriteTo(StreamOutput out) throws IOException { + } + + + @Override + protected EmptyQueryBuilder doReadFrom(StreamInput in) throws IOException { + return new EmptyQueryBuilder(); } @Override - public Query toFilter(QueryShardContext context) throws IOException { - // empty - return null; + protected int doHashCode() { + return 31; } @Override - public void writeTo(StreamOutput out) throws IOException { - } - - @Override - public EmptyQueryBuilder readFrom(StreamInput in) throws IOException { - return EmptyQueryBuilder.PROTOTYPE; - } - - @Override - public EmptyQueryBuilder queryName(String queryName) { - //no-op - return this; - } - - @Override - public String queryName() { - return null; - } - - @Override - public float boost() { - return -1; - } - - @Override - public EmptyQueryBuilder boost(float boost) { - //no-op - return this; + protected boolean doEquals(EmptyQueryBuilder other) { + return true; } } diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index f7d8b22d785..db64aa3a7d4 100644 --- a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -43,7 +43,6 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.geo.GeoShapeFieldMapper; -import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.Objects; @@ -62,7 +61,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + if (this.shape == null) { + GetRequest getRequest = new GetRequest(indexedShapeIndex, indexedShapeType, indexedShapeId); + ShapeBuilder shape = fetch(queryShardContext.getClient(), getRequest, indexedShapePath); + return new GeoShapeQueryBuilder(this.fieldName, shape).relation(relation).strategy(strategy) + .boost(boost()).queryName(queryName()); + } + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java index c84883fe737..66c9d77534d 100644 --- a/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java @@ -26,7 +26,6 @@ import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.join.JoinUtil; import org.apache.lucene.search.join.ScoreMode; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.lucene.search.Queries; @@ -397,4 +396,18 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryBuilder rewrite = query.rewrite(queryShardContext); + if (rewrite != query) { + HasChildQueryBuilder hasChildQueryBuilder = new HasChildQueryBuilder(type, rewrite); + hasChildQueryBuilder.minChildren = minChildren; + hasChildQueryBuilder.maxChildren = maxChildren; + hasChildQueryBuilder.scoreMode = scoreMode; + hasChildQueryBuilder.queryInnerHits = queryInnerHits; + return hasChildQueryBuilder.queryName(queryName()).boost(boost()); + } + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java index 173d6aa00c6..10ed269a2bc 100644 --- a/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java @@ -22,7 +22,6 @@ import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.join.ScoreMode; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.lucene.search.Queries; @@ -256,4 +255,16 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryBuilder rewrite = query.rewrite(queryShardContext); + if (rewrite != query) { + HasParentQueryBuilder hasParentQueryBuilder = new HasParentQueryBuilder(type, rewrite); + hasParentQueryBuilder.score = score; + hasParentQueryBuilder.innerHit = innerHit; + return hasParentQueryBuilder.queryName(queryName()).boost(boost()); + } + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java index 5185dfda3b0..c366394976e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java @@ -140,4 +140,14 @@ public class IndicesQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryBuilder newInnnerQuery = innerQuery.rewrite(queryShardContext); + QueryBuilder newNoMatchQuery = noMatchQuery.rewrite(queryShardContext); + if (newInnnerQuery != innerQuery || newNoMatchQuery != noMatchQuery) { + return new IndicesQueryBuilder(innerQuery, indices).noMatchQuery(noMatchQuery).boost(boost()).queryName(queryName()); + } + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index 0e98b2042a2..c08cf205e62 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -1050,4 +1050,10 @@ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + // TODO this needs heavy cleanups before we can rewrite it + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java index 103f957b247..461ebdb52a0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java @@ -225,4 +225,12 @@ public class NestedQueryBuilder extends AbstractQueryBuilder return new ToParentBlockJoinQuery(Queries.filtered(innerQuery, childFilter), parentFilter, scoreMode); } + @Override + public QueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryBuilder rewrite = query.rewrite(queryShardContext); + if (rewrite != query) { + return new NestedQueryBuilder(path, rewrite).queryName(queryName()).boost(boost()).scoreMode(scoreMode); + } + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java index b75406c8641..3667905bacf 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java @@ -72,4 +72,28 @@ public interface QueryBuilder> extends NamedWriteabl * Returns the name that identifies uniquely the query */ String getName(); + + /** + * Rewrites this query builder into it's primitive form. By default this method return theb builder itself. If the builder + * did not change the identity reference must be returend otherwise the builder will be rewritten infinitely. + */ + default QueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + return this; + } + + /** + * Rewrites the given query into it's primitive form. Queries that for instance fetch resources from remote hosts or + * can simplify / optimize itself should do their heavy lifting duringt {@link #rewrite(QueryRewriteContext)}. This method + * rewrites the query until it doesn't change anymore. + * @throws IOException if an {@link IOException} occurs + */ + static QueryBuilder rewriteQuery(QueryBuilder original, QueryRewriteContext context) throws IOException { + QueryBuilder builder = original; + for (QueryBuilder rewrittenBuilder = builder.rewrite(context); rewrittenBuilder != builder; + rewrittenBuilder = builder.rewrite(context)) { + builder = rewrittenBuilder; + } + return builder; + } + } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java index 78d76d8292e..439e6533de5 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -106,7 +106,7 @@ public class QueryParseContext { token = parser.nextToken(); if (token == XContentParser.Token.END_OBJECT) { // empty query - return EmptyQueryBuilder.PROTOTYPE; + return new EmptyQueryBuilder(); } if (token != XContentParser.Token.FIELD_NAME) { throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no field after start_object"); diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java new file mode 100644 index 00000000000..425673cb548 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -0,0 +1,63 @@ +/* + * 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.index.query; + +import org.elasticsearch.client.Client; +import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.indices.query.IndicesQueriesRegistry; +import org.elasticsearch.script.ScriptService; + +/** + */ +public class QueryRewriteContext { + protected final ScriptService scriptService; + protected final IndexSettings indexSettings; + protected final IndicesQueriesRegistry indicesQueriesRegistry; + protected final QueryParseContext parseContext; + + public QueryRewriteContext(IndexSettings indexSettings, ScriptService scriptService, IndicesQueriesRegistry indicesQueriesRegistry) { + this.scriptService = scriptService; + this.indexSettings = indexSettings; + this.indicesQueriesRegistry = indicesQueriesRegistry; + this.parseContext = new QueryParseContext(indicesQueriesRegistry); + } + + public final Client getClient() { + return scriptService.getClient(); + } + + public final IndexSettings getIndexSettings() { + return indexSettings; + } + + public final ScriptService getScriptService() { + return scriptService; + } + + public QueryParseContext newParseContext() { + QueryParseContext queryParseContext = new QueryParseContext(indicesQueriesRegistry); + queryParseContext.parseFieldMatcher(parseContext.parseFieldMatcher()); + return queryParseContext; + } + + public boolean hasIndex() { + return indexSettings != null; + } + +} diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 4701cdfeecc..0e3348ac7a5 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -26,7 +26,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.join.BitSetProducer; import org.apache.lucene.search.similarities.Similarity; import org.elasticsearch.Version; -import org.elasticsearch.client.Client; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; @@ -52,10 +51,7 @@ import org.elasticsearch.index.query.support.InnerHitsQueryParserHelper; import org.elasticsearch.index.query.support.NestedScope; import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.indices.query.IndicesQueriesRegistry; -import org.elasticsearch.script.ExecutableScript; -import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; -import org.elasticsearch.script.Template; import org.elasticsearch.search.fetch.innerhits.InnerHitsContext; import org.elasticsearch.search.fetch.innerhits.InnerHitsSubSearchContext; import org.elasticsearch.search.internal.SearchContext; @@ -64,7 +60,6 @@ import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -73,15 +68,13 @@ import static java.util.Collections.unmodifiableMap; /** * Context object used to create lucene queries on the shard level. */ -public class QueryShardContext { +public class QueryShardContext extends QueryRewriteContext { private final MapperService mapperService; - private final ScriptService scriptService; private final SimilarityService similarityService; private final BitsetFilterCache bitsetFilterCache; private final IndexFieldDataService indexFieldDataService; private final IndexSettings indexSettings; - private final Client client; private String[] types = Strings.EMPTY_ARRAY; public void setTypes(String... types) { @@ -94,35 +87,31 @@ public class QueryShardContext { private final Map namedQueries = new HashMap<>(); private final MapperQueryParser queryParser = new MapperQueryParser(this); - private final IndicesQueriesRegistry indicesQueriesRegistry; private boolean allowUnmappedFields; private boolean mapUnmappedFieldAsString; private NestedScope nestedScope; - private QueryParseContext parseContext; boolean isFilter; // pkg private for testing - public QueryShardContext(IndexSettings indexSettings, Client client, BitsetFilterCache bitsetFilterCache, IndexFieldDataService indexFieldDataService, MapperService mapperService, SimilarityService similarityService, ScriptService scriptService, + public QueryShardContext(IndexSettings indexSettings, BitsetFilterCache bitsetFilterCache, IndexFieldDataService indexFieldDataService, MapperService mapperService, SimilarityService similarityService, ScriptService scriptService, final IndicesQueriesRegistry indicesQueriesRegistry) { + super(indexSettings, scriptService, indicesQueriesRegistry); this.indexSettings = indexSettings; - this.scriptService = scriptService; - this.client = client; this.similarityService = similarityService; this.mapperService = mapperService; this.bitsetFilterCache = bitsetFilterCache; this.indexFieldDataService = indexFieldDataService; this.allowUnmappedFields = indexSettings.isDefaultAllowUnmappedFields(); - this.indicesQueriesRegistry = indicesQueriesRegistry; - this.parseContext = new QueryParseContext(indicesQueriesRegistry); + } public QueryShardContext(QueryShardContext source) { - this(source.indexSettings, source.client, source.bitsetFilterCache, source.indexFieldDataService, source.mapperService, source.similarityService, source.scriptService, source.indicesQueriesRegistry); + this(source.indexSettings, source.bitsetFilterCache, source.indexFieldDataService, source.mapperService, source.similarityService, source.scriptService, source.indicesQueriesRegistry); this.types = source.getTypes(); } public QueryShardContext clone() { - return new QueryShardContext(indexSettings, client, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, scriptService, indicesQueriesRegistry); + return new QueryShardContext(indexSettings, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, scriptService, indicesQueriesRegistry); } public void parseFieldMatcher(ParseFieldMatcher parseFieldMatcher) { @@ -147,10 +136,6 @@ public class QueryShardContext { this.parseContext.reset(jp); } - public Index index() { - return this.mapperService.getIndexSettings().getIndex(); - } - public InnerHitsSubSearchContext getInnerHitsContext(XContentParser parser) throws IOException { return InnerHitsQueryParserHelper.parse(parser); } @@ -159,10 +144,6 @@ public class QueryShardContext { return mapperService.analysisService(); } - public ScriptService getScriptService() { - return scriptService; - } - public MapperService getMapperService() { return mapperService; } @@ -341,18 +322,6 @@ public class QueryShardContext { return false; } - /* - * Executes the given template, and returns the response. - */ - public BytesReference executeQueryTemplate(Template template) { - ExecutableScript executable = getScriptService().executable(template, ScriptContext.Standard.SEARCH, Collections.emptyMap()); - return (BytesReference) executable.run(); - } - - public Client getClient() { - return client; - } - public ParsedQuery parse(BytesReference source) { XContentParser parser = null; try { @@ -385,7 +354,7 @@ public class QueryShardContext { reset(parser); try { parseFieldMatcher(indexSettings.getParseFieldMatcher()); - Query filter = parseContext().parseInnerQueryBuilder().toFilter(this); + Query filter = QueryBuilder.rewriteQuery(parseContext().parseInnerQueryBuilder(), this).toFilter(this); if (filter == null) { return null; } @@ -426,12 +395,16 @@ public class QueryShardContext { } } - private static Query toQuery(QueryBuilder queryBuilder, QueryShardContext context) throws IOException { - Query query = queryBuilder.toQuery(context); + private static Query toQuery(final QueryBuilder queryBuilder, final QueryShardContext context) throws IOException { + final Query query = QueryBuilder.rewriteQuery(queryBuilder, context).toQuery(context); if (query == null) { - query = Queries.newMatchNoDocsQuery(); + return Queries.newMatchNoDocsQuery(); } return query; } + public final Index index() { + return indexSettings.getIndex(); + } + } diff --git a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java index 353dbd668ac..31484fe804a 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java @@ -25,6 +25,7 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.RandomAccessWeight; import org.apache.lucene.search.Weight; import org.apache.lucene.util.Bits; +import org.elasticsearch.client.Client; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -32,6 +33,7 @@ import org.elasticsearch.script.LeafSearchScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.Script.ScriptField; import org.elasticsearch.script.ScriptContext; +import org.elasticsearch.script.ScriptEngineService; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.lookup.SearchLookup; diff --git a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java index 02a9bc42e33..4067f6d87cb 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java @@ -25,11 +25,13 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.Template; -import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; +import java.util.Collections; import java.util.Map; import java.util.Objects; @@ -100,14 +102,7 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryRewriteContext) throws IOException { + ExecutableScript executable = queryRewriteContext.getScriptService().executable(template, + ScriptContext.Standard.SEARCH, Collections.emptyMap()); + BytesReference querySource = (BytesReference) executable.run(); + final QueryParseContext queryParseContext = queryRewriteContext.newParseContext(); + try (XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource)) { + queryParseContext.reset(qSourceParser); + final QueryBuilder queryBuilder = queryParseContext.parseInnerQueryBuilder(); + if (queryBuilder.boost() == DEFAULT_BOOST) { + queryBuilder.boost(boost()); // only pass down the boost if it has it's own boost + } + if (queryName() != null) { + queryBuilder.queryName(queryName()); + } + return queryBuilder; + } + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java index 326a6ed8b8e..0cf4b7acf0e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java @@ -38,7 +38,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.indices.cache.query.terms.TermsLookup; -import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.ArrayList; @@ -227,22 +226,13 @@ public class TermsQueryBuilder extends AbstractQueryBuilder { @Override protected Query doToQuery(QueryShardContext context) throws IOException { - List terms; - TermsLookup termsLookup = null; - if (this.termsLookup != null) { - termsLookup = new TermsLookup(this.termsLookup); - if (termsLookup.index() == null) { - termsLookup.index(context.index().getName()); - } - Client client = context.getClient(); - terms = fetch(termsLookup, client); - } else { - terms = values; + if (termsLookup != null) { + throw new UnsupportedOperationException("query must be rewritten first"); } - if (terms == null || terms.isEmpty()) { + if (values == null || values.isEmpty()) { return Queries.newMatchNoDocsQuery(); } - return handleTermsQuery(terms, fieldName, context); + return handleTermsQuery(values, fieldName, context); } private List fetch(TermsLookup termsLookup, Client client) { @@ -324,4 +314,22 @@ public class TermsQueryBuilder extends AbstractQueryBuilder { Objects.equals(values, other.values) && Objects.equals(termsLookup, other.termsLookup); } + + @Override + public QueryBuilder rewrite(QueryRewriteContext queryRewriteContext) throws IOException { + if (this.termsLookup != null) { + TermsLookup termsLookup = new TermsLookup(this.termsLookup); + if (termsLookup.index() == null) { // TODO this should go away? + if (queryRewriteContext.hasIndex()) { + termsLookup.index(queryRewriteContext.getIndexSettings().getIndex().getName()); + } else { + return this; // can't rewrite until we have index scope on the shard + } + } + List values = fetch(termsLookup, queryRewriteContext.getClient()); + return new TermsQueryBuilder(this.fieldName, values).boost(boost()).queryName(queryName()); + } + return this; + } + } diff --git a/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java index e908d763311..6ba07f766e7 100644 --- a/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java @@ -105,14 +105,7 @@ public class WrapperQueryBuilder extends AbstractQueryBuilder result = contextCopy.parseContext().parseInnerQueryBuilder(); - context.combineNamedQueries(contextCopy); - return result.toQuery(context); - } + throw new UnsupportedOperationException("this query must be rewritten first"); } @Override @@ -134,4 +127,21 @@ public class WrapperQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext context) throws IOException { + try (XContentParser qSourceParser = XContentFactory.xContent(source).createParser(source)) { + QueryParseContext parseContext = context.newParseContext(); + parseContext.reset(qSourceParser); + + final QueryBuilder queryBuilder = parseContext.parseInnerQueryBuilder(); + if (queryBuilder.boost() == DEFAULT_BOOST) { + queryBuilder.boost(boost()); + } + if (queryName() != null) { // we inherit the name + queryBuilder.queryName(queryName()); + } + return queryBuilder; + } + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java index 766911bb747..14a28260842 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java @@ -34,6 +34,7 @@ import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.EmptyQueryBuilder; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.functionscore.random.RandomScoreFunctionBuilder; @@ -394,4 +395,17 @@ public class FunctionScoreQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryBuilder queryBuilder = this.query.rewrite(queryShardContext); + if (queryBuilder != query) { + FunctionScoreQueryBuilder newQueryBuilder = new FunctionScoreQueryBuilder(queryBuilder, filterFunctionBuilders); + newQueryBuilder.scoreMode = scoreMode; + newQueryBuilder.minScore = minScore; + newQueryBuilder.maxBoost = maxBoost; + return newQueryBuilder.queryName(queryName()).boost(boost()); + } + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/support/NestedInnerQueryParseSupport.java b/core/src/main/java/org/elasticsearch/index/query/support/NestedInnerQueryParseSupport.java index 890961dd2a2..9923728e3bd 100644 --- a/core/src/main/java/org/elasticsearch/index/query/support/NestedInnerQueryParseSupport.java +++ b/core/src/main/java/org/elasticsearch/index/query/support/NestedInnerQueryParseSupport.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.object.ObjectMapper; +import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.QueryShardException; @@ -91,7 +92,8 @@ public class NestedInnerQueryParseSupport { if (path != null) { setPathLevel(); try { - innerFilter = parseContext.parseInnerQueryBuilder().toFilter(this.shardContext); + innerFilter = QueryBuilder.rewriteQuery(parseContext.parseInnerQueryBuilder(), + this.shardContext).toFilter(this.shardContext); } finally { resetPathLevel(); } @@ -147,7 +149,8 @@ public class NestedInnerQueryParseSupport { try { XContentParser innerParser = XContentHelper.createParser(source); parseContext.parser(innerParser); - innerFilter = parseContext.parseInnerQueryBuilder().toFilter(this.shardContext); + innerFilter = QueryBuilder.rewriteQuery(parseContext.parseInnerQueryBuilder(), + this.shardContext).toFilter(this.shardContext); filterParsed = true; return innerFilter; } finally { diff --git a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java index f2f1add2594..e131fa2dd0d 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -252,7 +252,7 @@ public class IndexShard extends AbstractIndexShardComponent { this.engineConfig = newEngineConfig(translogConfig, cachingPolicy); this.suspendableRefContainer = new SuspendableRefContainer(); this.searcherWrapper = indexSearcherWrapper; - QueryShardContext queryShardContext = new QueryShardContext(idxSettings, provider.getClient(), indexCache.bitsetFilterCache(), indexFieldDataService, mapperService, similarityService, provider.getScriptService(), provider.getIndicesQueriesRegistry()); + QueryShardContext queryShardContext = new QueryShardContext(idxSettings, indexCache.bitsetFilterCache(), indexFieldDataService, mapperService, similarityService, provider.getScriptService(), provider.getIndicesQueriesRegistry()); this.percolatorQueriesRegistry = new PercolatorQueriesRegistry(shardId, indexSettings, queryShardContext); } diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java index d21283d9cfa..8e1ac1c8d77 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptService.java @@ -489,6 +489,10 @@ public class ScriptService extends AbstractComponent implements Closeable { return scriptMetrics.stats(); } + public Client getClient() { + return client; + } + /** * A small listener for the script cache that calls each * {@code ScriptEngineService}'s {@code scriptRemoved} method when the diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index e0b30a2e346..7c6fa0da9bb 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -536,8 +536,10 @@ public class SearchService extends AbstractLifecycleComponent imp DefaultSearchContext context = new DefaultSearchContext(idGenerator.incrementAndGet(), request, shardTarget, engineSearcher, indexService, indexShard, scriptService, pageCacheRecycler, bigArrays, threadPool.estimatedTimeInMillisCounter(), parseFieldMatcher, defaultSearchTimeout); SearchContext.setCurrent(context); - try { + if (request.source() != null) { + request.source().rewrite(context.getQueryShardContext()); + } if (request.scroll() != null) { context.scrollContext(new ScrollContext()); context.scrollContext().scroll = request.scroll(); diff --git a/core/src/main/java/org/elasticsearch/search/action/SearchServiceTransportAction.java b/core/src/main/java/org/elasticsearch/search/action/SearchServiceTransportAction.java index 138b215e68f..81fa590908d 100644 --- a/core/src/main/java/org/elasticsearch/search/action/SearchServiceTransportAction.java +++ b/core/src/main/java/org/elasticsearch/search/action/SearchServiceTransportAction.java @@ -81,7 +81,6 @@ public class SearchServiceTransportAction extends AbstractComponent { super(settings); this.transportService = transportService; this.searchService = searchService; - transportService.registerRequestHandler(FREE_CONTEXT_SCROLL_ACTION_NAME, ScrollFreeContextRequest::new, ThreadPool.Names.SAME, new FreeContextTransportHandler<>()); transportService.registerRequestHandler(FREE_CONTEXT_ACTION_NAME, SearchFreeContextRequest::new, ThreadPool.Names.SAME, new FreeContextTransportHandler()); transportService.registerRequestHandler(CLEAR_SCROLL_CONTEXTS_ACTION_NAME, ClearScrollContextsRequest::new, ThreadPool.Names.SAME, new ClearScrollContextsTransportHandler()); 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 6a9d95deb3b..99ee939d15b 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; +import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.script.Script; import org.elasticsearch.search.searchafter.SearchAfterBuilder; import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; @@ -1433,4 +1434,17 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ && Objects.equals(version, other.version) && Objects.equals(profile, other.profile); } + + /** + * Rewrites the internal query builders in-place + */ + public void rewrite(QueryRewriteContext rewriteContext) throws IOException { + if (queryBuilder != null) { + queryBuilder = QueryBuilder.rewriteQuery(queryBuilder, rewriteContext); + } + if (postQueryBuilder != null) { + postQueryBuilder = QueryBuilder.rewriteQuery(postQueryBuilder, rewriteContext); + } + } + } diff --git a/core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java b/core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java index c0b1aeea3be..325541844ca 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java @@ -355,7 +355,7 @@ public class HighlightBuilder extends AbstractHighlighterBuilder { public QueryRescoreContext build(QueryShardContext context) throws IOException { org.elasticsearch.search.rescore.QueryRescorer rescorer = new org.elasticsearch.search.rescore.QueryRescorer(); QueryRescoreContext queryRescoreContext = new QueryRescoreContext(rescorer); - queryRescoreContext.setQuery(this.queryBuilder.toQuery(context)); + queryRescoreContext.setQuery(QueryBuilder.rewriteQuery(this.queryBuilder, context).toQuery(context)); queryRescoreContext.setQueryWeight(this.queryWeight); queryRescoreContext.setRescoreQueryWeight(this.rescoreQueryWeight); queryRescoreContext.setScoreMode(this.scoreMode); @@ -239,4 +239,4 @@ public class QueryRescorerBuilder extends RescoreBuilder { this.scoreMode = scoreMode; } } -} \ No newline at end of file +} diff --git a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java index e8f20cb855c..5b8550b7b1c 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -286,7 +286,7 @@ public abstract class AbstractQueryTestCase> } }); indicesQueriesRegistry = injector.getInstance(IndicesQueriesRegistry.class); - queryShardContext = new QueryShardContext(idxSettings, proxy, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, scriptService, indicesQueriesRegistry); + queryShardContext = new QueryShardContext(idxSettings, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, scriptService, indicesQueriesRegistry); //create some random type with some default field, those types will stick around for all of the subclasses currentTypes = new String[randomIntBetween(0, 5)]; for (int i = 0; i < currentTypes.length; i++) { @@ -516,7 +516,7 @@ public abstract class AbstractQueryTestCase> QB firstQuery = createTestQueryBuilder(); QB controlQuery = copyQuery(firstQuery); setSearchContext(randomTypes); // only set search context for toQuery to be more realistic - Query firstLuceneQuery = firstQuery.toQuery(context); + Query firstLuceneQuery = rewriteQuery(firstQuery, context).toQuery(context); assertLuceneQuery(firstQuery, firstLuceneQuery, context); SearchContext.removeCurrent(); // remove after assertLuceneQuery since the assertLuceneQuery impl might access the context as well assertTrue( @@ -534,7 +534,7 @@ public abstract class AbstractQueryTestCase> + randomAsciiOfLengthBetween(1, 10)); } setSearchContext(randomTypes); - Query secondLuceneQuery = secondQuery.toQuery(context); + Query secondLuceneQuery = rewriteQuery(secondQuery, context).toQuery(context); assertLuceneQuery(secondQuery, secondLuceneQuery, context); SearchContext.removeCurrent(); @@ -544,7 +544,7 @@ public abstract class AbstractQueryTestCase> if (firstLuceneQuery != null && supportsBoostAndQueryName()) { secondQuery.boost(firstQuery.boost() + 1f + randomFloat()); setSearchContext(randomTypes); - Query thirdLuceneQuery = secondQuery.toQuery(context); + Query thirdLuceneQuery = rewriteQuery(secondQuery, context).toQuery(context); SearchContext.removeCurrent(); assertThat("modifying the boost doesn't affect the corresponding lucene query", firstLuceneQuery, not(equalTo(thirdLuceneQuery))); @@ -552,6 +552,12 @@ public abstract class AbstractQueryTestCase> } } + private QueryBuilder rewriteQuery(QB queryBuilder, QueryRewriteContext rewriteContext) throws IOException { + QueryBuilder rewritten = QueryBuilder.rewriteQuery(queryBuilder, rewriteContext); + assertSerialization(rewritten); + return rewritten; + } + /** * Few queries allow you to set the boost and queryName on the java api, although the corresponding parser doesn't parse them as they are not supported. * This method allows to disable boost and queryName related tests for those queries. Those queries are easy to identify: their parsers @@ -625,11 +631,13 @@ public abstract class AbstractQueryTestCase> * Serialize the given query builder and asserts that both are equal */ @SuppressWarnings("unchecked") - protected QB assertSerialization(QB testQuery) throws IOException { + protected QB assertSerialization(QB testQuery) throws IOException { try (BytesStreamOutput output = new BytesStreamOutput()) { testQuery.writeTo(output); try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) { - QueryBuilder prototype = queryParser(testQuery.getName()).getBuilderPrototype(); + QueryParser queryParser = queryParser(testQuery.getName()); + assertNotNull("queryparser not found for query: [" + testQuery.getName() + "]", queryParser); + QueryBuilder prototype = queryParser.getBuilderPrototype(); QueryBuilder deserializedQuery = prototype.readFrom(in); assertEquals(deserializedQuery, testQuery); assertEquals(deserializedQuery.hashCode(), testQuery.hashCode()); @@ -674,7 +682,26 @@ public abstract class AbstractQueryTestCase> } private QueryParser queryParser(String queryId) { - return indicesQueriesRegistry.queryParsers().get(queryId); + QueryParser queryParser = indicesQueriesRegistry.queryParsers().get(queryId); + if (queryParser == null && EmptyQueryBuilder.NAME.equals(queryId)) { + return new QueryParser() { + @Override + public String[] names() { + return new String[] {EmptyQueryBuilder.NAME}; + } + + @Override + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + return new EmptyQueryBuilder(); + } + + @Override + public QueryBuilder getBuilderPrototype() { + return EmptyQueryBuilder.PROTOTYPE; + } + }; + } + return queryParser; } //we use the streaming infra to create a copy of the query provided as argument diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java b/core/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java index f78700d4a15..1c35642c51e 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java @@ -21,15 +21,12 @@ package org.elasticsearch.index.query; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.core.StringFieldMapper; import org.elasticsearch.test.ESTestCase; -import java.util.Collections; - import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; @@ -50,7 +47,7 @@ public class QueryShardContextTests extends ESTestCase { MapperService mapperService = mock(MapperService.class); when(mapperService.getIndexSettings()).thenReturn(indexSettings); QueryShardContext context = new QueryShardContext( - indexSettings, null, null, null, mapperService, null, null, null + indexSettings, null, null, mapperService, null, null, null ); context.setAllowUnmappedFields(false); diff --git a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java index df7eb3c697a..7a602f09418 100644 --- a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java @@ -56,7 +56,7 @@ public class TemplateQueryBuilderTests extends AbstractQueryTestCase> parsers = singletonMap("term", new TermQueryParser()); IndicesQueriesRegistry indicesQueriesRegistry = new IndicesQueriesRegistry(indexSettings.getSettings(), parsers); - queryShardContext = new QueryShardContext(indexSettings, null, null, null, mapperService, null, null, indicesQueriesRegistry); + queryShardContext = new QueryShardContext(indexSettings, null, null, mapperService, null, null, indicesQueriesRegistry); HighlightPhase highlightPhase = new HighlightPhase(Settings.EMPTY, new Highlighters()); AggregatorParsers aggregatorParsers = new AggregatorParsers(Collections.emptySet(), Collections.emptySet()); diff --git a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java index 786d5941827..3714f6a23fb 100644 --- a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java @@ -41,9 +41,14 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; import org.elasticsearch.index.query.AbstractQueryTestCase; +import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.EmptyQueryBuilder; +import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.QueryParseContext; +import org.elasticsearch.index.query.QueryRewriteContext; +import org.elasticsearch.index.query.TermQueryBuilder; +import org.elasticsearch.index.query.WrapperQueryBuilder; import org.elasticsearch.index.query.functionscore.ScoreFunctionParser; import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.script.Script; @@ -483,4 +488,14 @@ public class SearchSourceBuilderTests extends ESTestCase { String query = "{ \"post_filter\": {} }"; assertParseSearchSource(builder, new BytesArray(query)); } + + public void testRewrite() throws IOException { + SearchSourceBuilder builder = new SearchSourceBuilder(); + builder.query(new BoolQueryBuilder()); + TermQueryBuilder tqb = new TermQueryBuilder("foo", "bar"); + builder.postFilter(new WrapperQueryBuilder(tqb.toString())); + builder.rewrite(new QueryRewriteContext(null, null, indicesQueriesRegistry)); + assertEquals(new MatchAllQueryBuilder(), builder.query()); + assertEquals(tqb, builder.postFilter()); + } } diff --git a/core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java b/core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java index 5dc8528c00a..2cd81b0fdea 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java @@ -278,7 +278,7 @@ public class HighlightBuilderTests extends ESTestCase { Index index = new Index(randomAsciiOfLengthBetween(1, 10), "_na_"); IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(index, indexSettings); // shard context will only need indicesQueriesRegistry for building Query objects nested in highlighter - QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, null, indicesQueriesRegistry) { + QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, indicesQueriesRegistry) { @Override public MappedFieldType fieldMapper(String name) { StringFieldMapper.Builder builder = MapperBuilders.stringField(name); 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 01f7e332446..37f81e3d339 100644 --- a/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java @@ -160,7 +160,7 @@ public class QueryRescoreBuilderTests extends ESTestCase { .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(); IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(randomAsciiOfLengthBetween(1, 10), indexSettings); // shard context will only need indicesQueriesRegistry for building Query objects nested in query rescorer - QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, null, indicesQueriesRegistry) { + QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, indicesQueriesRegistry) { @Override public MappedFieldType fieldMapper(String name) { StringFieldMapper.Builder builder = MapperBuilders.stringField(name); @@ -170,7 +170,7 @@ public class QueryRescoreBuilderTests extends ESTestCase { for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) { RescoreBuilder rescoreBuilder = randomRescoreBuilder(); - QueryRescoreContext rescoreContext = (QueryRescoreContext) rescoreBuilder.build(mockShardContext); + QueryRescoreContext rescoreContext = rescoreBuilder.build(mockShardContext); XContentParser parser = createParser(rescoreBuilder); QueryRescoreContext parsedRescoreContext = (QueryRescoreContext) new RescoreParseElement().parseSingleRescoreContext(parser, mockShardContext); diff --git a/core/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java b/core/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java index 02826b9a7eb..ebf903d91a8 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java @@ -171,7 +171,7 @@ public class DirectCandidateGeneratorTests extends ESTestCase{ } }; - QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, mockMapperService, null, null, null) { + QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, mockMapperService, null, null, null) { @Override public MappedFieldType fieldMapper(String name) { StringFieldMapper.Builder builder = MapperBuilders.stringField(name); diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java index 86667515bd6..96ca5ac9aa5 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java @@ -46,6 +46,7 @@ import org.elasticsearch.index.cache.bitset.BitsetFilterCache; import org.elasticsearch.index.fielddata.IndexFieldDataCache; import org.elasticsearch.index.fielddata.IndexFieldDataService; import org.elasticsearch.index.mapper.MapperService; +import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.TemplateQueryParser; import org.elasticsearch.index.query.functionscore.ScoreFunctionParser; @@ -152,7 +153,7 @@ public class TemplateQueryParserTests extends ESTestCase { } }); IndicesQueriesRegistry indicesQueriesRegistry = injector.getInstance(IndicesQueriesRegistry.class); - context = new QueryShardContext(idxSettings, proxy, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, scriptService, indicesQueriesRegistry); + context = new QueryShardContext(idxSettings, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, scriptService, indicesQueriesRegistry); } @Override @@ -170,7 +171,7 @@ public class TemplateQueryParserTests extends ESTestCase { templateSourceParser.nextToken(); TemplateQueryParser parser = injector.getInstance(TemplateQueryParser.class); - Query query = parser.fromXContent(context.parseContext()).toQuery(context); + Query query = QueryBuilder.rewriteQuery(parser.fromXContent(context.parseContext()), context).toQuery(context); assertTrue("Parsing template query failed.", query instanceof MatchAllDocsQuery); } @@ -181,7 +182,7 @@ public class TemplateQueryParserTests extends ESTestCase { context.reset(templateSourceParser); TemplateQueryParser parser = injector.getInstance(TemplateQueryParser.class); - Query query = parser.fromXContent(context.parseContext()).toQuery(context); + Query query = QueryBuilder.rewriteQuery(parser.fromXContent(context.parseContext()), context).toQuery(context); assertTrue("Parsing template query failed.", query instanceof MatchAllDocsQuery); } @@ -199,7 +200,7 @@ public class TemplateQueryParserTests extends ESTestCase { TemplateQueryParser parser = injector.getInstance(TemplateQueryParser.class); try { - parser.fromXContent(context.parseContext()).toQuery(context); + parser.fromXContent(context.parseContext()).rewrite(context); fail("Expected ParsingException"); } catch (ParsingException e) { assertThat(e.getMessage(), containsString("query malformed, no field after start_object")); @@ -213,8 +214,23 @@ public class TemplateQueryParserTests extends ESTestCase { context.reset(templateSourceParser); templateSourceParser.nextToken(); + TemplateQueryParser parser = injector.getInstance(TemplateQueryParser.class); - Query query = parser.fromXContent(context.parseContext()).toQuery(context); + Query query = QueryBuilder.rewriteQuery(parser.fromXContent(context.parseContext()), context).toQuery(context); assertTrue("Parsing template query failed.", query instanceof MatchAllDocsQuery); } + + public void testMustRewrite() throws Exception { + String templateString = "{ \"file\": \"storedTemplate\" ,\"params\":{\"template\":\"all\" } } "; + + XContentParser templateSourceParser = XContentFactory.xContent(templateString).createParser(templateString); + context.reset(templateSourceParser); + templateSourceParser.nextToken(); + TemplateQueryParser parser = injector.getInstance(TemplateQueryParser.class); + try { + parser.fromXContent(context.parseContext()).toQuery(context); + } catch (UnsupportedOperationException ex) { + assertEquals("this query must be rewritten first", ex.getMessage()); + } + } } From b906c8a389b36206685b46d70f114d6cef82cb15 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Fri, 12 Feb 2016 12:53:12 +0100 Subject: [PATCH 2/6] apply fixes after review --- .../action/search/TransportSearchAction.java | 1 - .../index/query/AbstractQueryBuilder.java | 19 +++++ .../index/query/BoolQueryBuilder.java | 14 +++- .../index/query/BoostingQueryBuilder.java | 9 +-- .../query/ConstantScoreQueryBuilder.java | 6 +- .../index/query/EmptyQueryBuilder.java | 8 -- .../index/query/GeoShapeQueryBuilder.java | 7 +- .../index/query/HasChildQueryBuilder.java | 6 +- .../index/query/HasParentQueryBuilder.java | 4 +- .../index/query/IndicesQueryBuilder.java | 4 +- .../index/query/MoreLikeThisQueryBuilder.java | 2 +- .../index/query/NestedQueryBuilder.java | 6 +- .../index/query/QueryBuilder.java | 8 +- .../index/query/QueryRewriteContext.java | 19 +++-- .../index/query/QueryShardContext.java | 4 - .../index/query/ScriptQueryBuilder.java | 2 - .../index/query/TemplateQueryBuilder.java | 11 ++- .../index/query/TermsQueryBuilder.java | 6 +- .../index/query/WrapperQueryBuilder.java | 13 ++-- .../FunctionScoreQueryBuilder.java | 25 +++++-- .../index/query/AbstractQueryTestCase.java | 13 ++++ .../index/query/BoolQueryBuilderTests.java | 74 ++++++++++++++++++- .../query/BoostingQueryBuilderTests.java | 56 ++++++++------ .../GeoBoundingBoxQueryBuilderTests.java | 6 ++ .../query/GeoDistanceQueryBuilderTests.java | 6 ++ .../query/GeoDistanceRangeQueryTests.java | 6 ++ .../query/GeoPolygonQueryBuilderTests.java | 6 ++ .../query/GeoShapeQueryBuilderTests.java | 20 +++++ .../query/GeohashCellQueryBuilderTests.java | 6 ++ .../query/TemplateQueryBuilderTests.java | 38 ++++++++++ .../index/query/TermsQueryBuilderTests.java | 12 +++ .../index/query/WrapperQueryBuilderTests.java | 30 ++++++++ 32 files changed, 351 insertions(+), 96 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index 77afd123644..4eec61b3a63 100644 --- a/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -33,7 +33,6 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.IndexClosedException; -import org.elasticsearch.search.SearchService; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; diff --git a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index d2116ae3c05..39c835e4b09 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -258,4 +258,23 @@ public abstract class AbstractQueryBuilder> } return queries; } + + @Override + public final QueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryBuilder rewritten = doRewrite(queryShardContext); + if (rewritten == this) { + return rewritten; + } + if (queryName() != null && rewritten.queryName() == null) { // we inherit the name + rewritten.queryName(queryName()); + } + if (boost() != DEFAULT_BOOST && rewritten.boost() == DEFAULT_BOOST) { + rewritten.boost(boost()); + } + return rewritten; + } + + protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOException { + return this; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index 72e13c2c31a..261acff33ce 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -263,6 +263,13 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { @Override protected Query doToQuery(QueryShardContext context) throws IOException { + final int numClauses = mustNotClauses.size() + filterClauses.size() + shouldClauses.size() + mustClauses.size(); + if (numClauses == 1 && must().size() == 1 && boost() == DEFAULT_BOOST) { + // we really only optimize this for testing since we use this to rewrite + // named queries TemplateQueryBuilder and WrapperQueryBuilder. + // it's equivalent but will anyways happen later down the road in the BQ#rewrite method + return mustClauses.get(0).toQuery(context); + } BooleanQuery.Builder booleanQueryBuilder = new BooleanQuery.Builder(); booleanQueryBuilder.setDisableCoord(disableCoord); addBooleanClauses(context, booleanQueryBuilder, mustClauses, BooleanClause.Occur.MUST); @@ -273,6 +280,7 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { if (booleanQuery.clauses().isEmpty()) { return new MatchAllDocsQuery(); } + final String minimumShouldMatch; if (context.isFilter() && this.minimumShouldMatch == null && shouldClauses.size() > 0) { minimumShouldMatch = "1"; @@ -349,7 +357,7 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { } @Override - public QueryBuilder rewrite(QueryRewriteContext queryRewriteContext) throws IOException { + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { BoolQueryBuilder newBuilder = new BoolQueryBuilder(); boolean changed = false; final int clauses = mustClauses.size() + mustNotClauses.size() + filterClauses.size() + shouldClauses.size(); @@ -372,14 +380,14 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { return this; } - private static boolean rewriteClauses(QueryRewriteContext queryRewriteContext, List> builders, Consumer> conusmer) throws IOException { + private static boolean rewriteClauses(QueryRewriteContext queryRewriteContext, List> builders, Consumer> consumer) throws IOException { boolean changed = false; for (QueryBuilder builder : builders) { QueryBuilder result = builder.rewrite(queryRewriteContext); if (result != builder) { changed = true; } - conusmer.accept(result); + consumer.accept(result); } return changed; } diff --git a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java index 507e85687dc..d03d6894b09 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java @@ -160,12 +160,11 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { - QueryBuilder positiveQuery = this.positiveQuery.rewrite(queryShardContext); - QueryBuilder negativeQuery = this.negativeQuery.rewrite(queryShardContext); + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + QueryBuilder positiveQuery = this.positiveQuery.rewrite(queryRewriteContext); + QueryBuilder negativeQuery = this.negativeQuery.rewrite(queryRewriteContext); if (positiveQuery != this.positiveQuery || negativeQuery != this.negativeQuery) { - BoostingQueryBuilder newQueryBuilder = new BoostingQueryBuilder(positiveQuery, negativeQuery) - .boost(boost()).queryName(queryName()); + BoostingQueryBuilder newQueryBuilder = new BoostingQueryBuilder(positiveQuery, negativeQuery); newQueryBuilder.negativeBoost = negativeBoost; return newQueryBuilder; } 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 ad00bba28b0..ee6fa4c2c2c 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -106,10 +106,10 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { - QueryBuilder rewrite = filterBuilder.rewrite(queryShardContext); + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + QueryBuilder rewrite = filterBuilder.rewrite(queryRewriteContext); if (rewrite != filterBuilder) { - return new ConstantScoreQueryBuilder(rewrite).boost(boost()).queryName(queryName()); + return new ConstantScoreQueryBuilder(rewrite); } return this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java index 83f7abae7ae..e810292a95c 100644 --- a/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java @@ -20,24 +20,16 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.Query; -import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; -import java.util.Objects; /** * A {@link QueryBuilder} that is a stand in replacement for an empty query clause in the DSL. * The current DSL allows parsing inner queries / filters like "{ }", in order to have a * valid non-null representation of these clauses that actually do nothing we can use this class. - * - * This builder has no corresponding parser and it is not registered under the query name. It is - * intended to be used internally as a stand-in for nested queries that are left empty and should - * be ignored upstream. */ public final class EmptyQueryBuilder extends AbstractQueryBuilder { diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index db64aa3a7d4..dd186f92ba3 100644 --- a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -235,7 +235,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOException { if (this.shape == null) { GetRequest getRequest = new GetRequest(indexedShapeIndex, indexedShapeType, indexedShapeId); ShapeBuilder shape = fetch(queryShardContext.getClient(), getRequest, indexedShapePath); - return new GeoShapeQueryBuilder(this.fieldName, shape).relation(relation).strategy(strategy) - .boost(boost()).queryName(queryName()); + return new GeoShapeQueryBuilder(this.fieldName, shape).relation(relation).strategy(strategy); } return this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java index 66c9d77534d..940fa89f03a 100644 --- a/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java @@ -398,15 +398,15 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { - QueryBuilder rewrite = query.rewrite(queryShardContext); + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + QueryBuilder rewrite = query.rewrite(queryRewriteContext); if (rewrite != query) { HasChildQueryBuilder hasChildQueryBuilder = new HasChildQueryBuilder(type, rewrite); hasChildQueryBuilder.minChildren = minChildren; hasChildQueryBuilder.maxChildren = maxChildren; hasChildQueryBuilder.scoreMode = scoreMode; hasChildQueryBuilder.queryInnerHits = queryInnerHits; - return hasChildQueryBuilder.queryName(queryName()).boost(boost()); + return hasChildQueryBuilder; } return this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java index 10ed269a2bc..9a3637de3f9 100644 --- a/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java @@ -257,13 +257,13 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOException { QueryBuilder rewrite = query.rewrite(queryShardContext); if (rewrite != query) { HasParentQueryBuilder hasParentQueryBuilder = new HasParentQueryBuilder(type, rewrite); hasParentQueryBuilder.score = score; hasParentQueryBuilder.innerHit = innerHit; - return hasParentQueryBuilder.queryName(queryName()).boost(boost()); + return hasParentQueryBuilder; } return this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java index c366394976e..019e18d1344 100644 --- a/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java @@ -142,11 +142,11 @@ public class IndicesQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOException { QueryBuilder newInnnerQuery = innerQuery.rewrite(queryShardContext); QueryBuilder newNoMatchQuery = noMatchQuery.rewrite(queryShardContext); if (newInnnerQuery != innerQuery || newNoMatchQuery != noMatchQuery) { - return new IndicesQueryBuilder(innerQuery, indices).noMatchQuery(noMatchQuery).boost(boost()).queryName(queryName()); + return new IndicesQueryBuilder(innerQuery, indices).noMatchQuery(noMatchQuery); } return this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index c08cf205e62..9f6c8b24c4f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -1052,7 +1052,7 @@ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { // TODO this needs heavy cleanups before we can rewrite it return this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java index 461ebdb52a0..596c2499211 100644 --- a/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java @@ -226,10 +226,10 @@ public class NestedQueryBuilder extends AbstractQueryBuilder } @Override - public QueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { - QueryBuilder rewrite = query.rewrite(queryShardContext); + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + QueryBuilder rewrite = query.rewrite(queryRewriteContext); if (rewrite != query) { - return new NestedQueryBuilder(path, rewrite).queryName(queryName()).boost(boost()).scoreMode(scoreMode); + return new NestedQueryBuilder(path, rewrite).scoreMode(scoreMode); } return this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java index 3667905bacf..f8010e7b523 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java @@ -74,16 +74,16 @@ public interface QueryBuilder> extends NamedWriteabl String getName(); /** - * Rewrites this query builder into it's primitive form. By default this method return theb builder itself. If the builder - * did not change the identity reference must be returend otherwise the builder will be rewritten infinitely. + * Rewrites this query builder into its primitive form. By default this method return the builder itself. If the builder + * did not change the identity reference must be returned otherwise the builder will be rewritten infinitely. */ default QueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { return this; } /** - * Rewrites the given query into it's primitive form. Queries that for instance fetch resources from remote hosts or - * can simplify / optimize itself should do their heavy lifting duringt {@link #rewrite(QueryRewriteContext)}. This method + * Rewrites the given query into its primitive form. Queries that for instance fetch resources from remote hosts or + * can simplify / optimize itself should do their heavy lifting during {@link #rewrite(QueryRewriteContext)}. This method * rewrites the query until it doesn't change anymore. * @throws IOException if an {@link IOException} occurs */ diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index 425673cb548..e057aff06b1 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -24,6 +24,7 @@ import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.script.ScriptService; /** + * Context object used to rewrite {@link QueryBuilder} instances into simplified version. */ public class QueryRewriteContext { protected final ScriptService scriptService; @@ -38,26 +39,34 @@ public class QueryRewriteContext { this.parseContext = new QueryParseContext(indicesQueriesRegistry); } + /** + * Returns a clients to fetch resources from local or remove nodes. + */ public final Client getClient() { return scriptService.getClient(); } + /** + * Returns the index settings for this context. This might return null if the + * context has not index scope. + */ public final IndexSettings getIndexSettings() { return indexSettings; } + /** + * Returns a script service to fetch scripts. + */ public final ScriptService getScriptService() { return scriptService; } + /** + * Returns a new {@link QueryParseContext} to parse template or wrapped queries. + */ public QueryParseContext newParseContext() { QueryParseContext queryParseContext = new QueryParseContext(indicesQueriesRegistry); queryParseContext.parseFieldMatcher(parseContext.parseFieldMatcher()); return queryParseContext; } - - public boolean hasIndex() { - return indexSettings != null; - } - } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 09978122dca..83066fba7bc 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -191,10 +191,6 @@ public class QueryShardContext extends QueryRewriteContext { return unmodifiableMap(new HashMap<>(namedQueries)); } - public void combineNamedQueries(QueryShardContext context) { - namedQueries.putAll(context.namedQueries); - } - /** * Return whether we are currently parsing a filter or a query. */ diff --git a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java index 31484fe804a..353dbd668ac 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java @@ -25,7 +25,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.RandomAccessWeight; import org.apache.lucene.search.Weight; import org.apache.lucene.util.Bits; -import org.elasticsearch.client.Client; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -33,7 +32,6 @@ import org.elasticsearch.script.LeafSearchScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.Script.ScriptField; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptEngineService; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.lookup.SearchLookup; diff --git a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java index 4067f6d87cb..4b3e81ebf16 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java @@ -127,7 +127,7 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext queryRewriteContext) throws IOException { + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { ExecutableScript executable = queryRewriteContext.getScriptService().executable(template, ScriptContext.Standard.SEARCH, Collections.emptyMap()); BytesReference querySource = (BytesReference) executable.run(); @@ -135,11 +135,10 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder queryBuilder = queryParseContext.parseInnerQueryBuilder(); - if (queryBuilder.boost() == DEFAULT_BOOST) { - queryBuilder.boost(boost()); // only pass down the boost if it has it's own boost - } - if (queryName() != null) { - queryBuilder.queryName(queryName()); + if (boost() != DEFAULT_BOOST || queryName() != null) { + final BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); + boolQueryBuilder.must(queryBuilder); + return boolQueryBuilder; } return queryBuilder; } diff --git a/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java index 570fc76dab8..e75d982ebe0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java @@ -316,18 +316,18 @@ public class TermsQueryBuilder extends AbstractQueryBuilder { } @Override - public QueryBuilder rewrite(QueryRewriteContext queryRewriteContext) throws IOException { + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { if (this.termsLookup != null) { TermsLookup termsLookup = new TermsLookup(this.termsLookup); if (termsLookup.index() == null) { // TODO this should go away? - if (queryRewriteContext.hasIndex()) { + if (queryRewriteContext.getIndexSettings() != null) { termsLookup.index(queryRewriteContext.getIndexSettings().getIndex().getName()); } else { return this; // can't rewrite until we have index scope on the shard } } List values = fetch(termsLookup, queryRewriteContext.getClient()); - return new TermsQueryBuilder(this.fieldName, values).boost(boost()).queryName(queryName()); + return new TermsQueryBuilder(this.fieldName, values); } return this; } diff --git a/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java index 6ba07f766e7..6c7ac0e9084 100644 --- a/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java @@ -105,7 +105,7 @@ public class WrapperQueryBuilder extends AbstractQueryBuilder rewrite(QueryRewriteContext context) throws IOException { + protected QueryBuilder doRewrite(QueryRewriteContext context) throws IOException { try (XContentParser qSourceParser = XContentFactory.xContent(source).createParser(source)) { QueryParseContext parseContext = context.newParseContext(); parseContext.reset(qSourceParser); final QueryBuilder queryBuilder = parseContext.parseInnerQueryBuilder(); - if (queryBuilder.boost() == DEFAULT_BOOST) { - queryBuilder.boost(boost()); - } - if (queryName() != null) { // we inherit the name - queryBuilder.queryName(queryName()); + if (boost() != DEFAULT_BOOST || queryName() != null) { + final BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); + boolQueryBuilder.must(queryBuilder); + return boolQueryBuilder; } return queryBuilder; } diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java index 14a28260842..60959a9789e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java @@ -394,17 +394,32 @@ public class FunctionScoreQueryBuilder extends AbstractQueryBuilder rewrite = filter.rewrite(context); + if (rewrite != filter) { + return new FilterFunctionBuilder(rewrite, scoreFunction); + } + return this; + } } @Override - public QueryBuilder rewrite(QueryRewriteContext queryShardContext) throws IOException { - QueryBuilder queryBuilder = this.query.rewrite(queryShardContext); - if (queryBuilder != query) { - FunctionScoreQueryBuilder newQueryBuilder = new FunctionScoreQueryBuilder(queryBuilder, filterFunctionBuilders); + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + QueryBuilder queryBuilder = this.query.rewrite(queryRewriteContext); + FilterFunctionBuilder[] rewrittenBuilders = new FilterFunctionBuilder[this.filterFunctionBuilders.length]; + boolean rewritten = false; + for (int i = 0; i < rewrittenBuilders.length; i++) { + FilterFunctionBuilder rewrite = filterFunctionBuilders[i].rewrite(queryRewriteContext); + rewritten |= rewrite != filterFunctionBuilders[i]; + rewrittenBuilders[i] = rewrite; + } + if (queryBuilder != query || rewritten) { + FunctionScoreQueryBuilder newQueryBuilder = new FunctionScoreQueryBuilder(queryBuilder, rewrittenBuilders); newQueryBuilder.scoreMode = scoreMode; newQueryBuilder.minScore = minScore; newQueryBuilder.maxBoost = maxBoost; - return newQueryBuilder.queryName(queryName()).boost(boost()); + return newQueryBuilder; } return this; } diff --git a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java index 5b8550b7b1c..19e2cab044e 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -554,6 +554,7 @@ public abstract class AbstractQueryTestCase> private QueryBuilder rewriteQuery(QB queryBuilder, QueryRewriteContext rewriteContext) throws IOException { QueryBuilder rewritten = QueryBuilder.rewriteQuery(queryBuilder, rewriteContext); + // extra safety to fail fast - serialize the rewritten version to ensure it's serializable. assertSerialization(rewritten); return rewritten; } @@ -975,4 +976,16 @@ public abstract class AbstractQueryTestCase> } return ""; } + + /** + * This test ensures that queries that need to be rewritten have dedicated tests. + * These queries must override this method accordingly. + */ + public void testMustRewrite() throws IOException { + QueryShardContext context = createShardContext(); + context.setAllowUnmappedFields(true); + QB queryBuilder = createTestQueryBuilder(); + setSearchContext(randomTypes); // only set search context for toQuery to be more realistic + queryBuilder.toQuery(context); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 30dbcdf4b60..7b3f3ecb7fd 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -195,14 +195,14 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase rewritten = boolQueryBuilder.rewrite(queryShardContext()); + if (mustRewrite == false && boolQueryBuilder.must().isEmpty()) { + // if it's empty we rewrite to match all + assertEquals(rewritten, new MatchAllQueryBuilder()); + } else { + BoolQueryBuilder rewrite = (BoolQueryBuilder) rewritten; + if (mustRewrite) { + assertNotSame(rewrite, boolQueryBuilder); + if (boolQueryBuilder.must().isEmpty() == false) { + assertEquals(new TermsQueryBuilder("foo", "must"), rewrite.must().get(0)); + } + if (boolQueryBuilder.should().isEmpty() == false) { + assertEquals(new TermsQueryBuilder("foo", "should"), rewrite.should().get(0)); + } + if (boolQueryBuilder.mustNot().isEmpty() == false) { + assertEquals(new TermsQueryBuilder("foo", "must_not"), rewrite.mustNot().get(0)); + } + if (boolQueryBuilder.filter().isEmpty() == false) { + assertEquals(new TermsQueryBuilder("foo", "filter"), rewrite.filter().get(0)); + } + } else { + assertSame(rewrite, boolQueryBuilder); + if (boolQueryBuilder.must().isEmpty() == false) { + assertSame(boolQueryBuilder.must().get(0), rewrite.must().get(0)); + } + } + } + } + + public void testRewriteMultipleTimes() throws IOException { + BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); + boolQueryBuilder.must(new WrapperQueryBuilder(new WrapperQueryBuilder(new MatchAllQueryBuilder().toString()).toString())); + QueryBuilder rewritten = boolQueryBuilder.rewrite(queryShardContext()); + BoolQueryBuilder expected = new BoolQueryBuilder(); + expected.must(new WrapperQueryBuilder(new MatchAllQueryBuilder().toString())); + assertEquals(expected, rewritten); + + expected = new BoolQueryBuilder(); + expected.must(new MatchAllQueryBuilder()); + QueryBuilder rewrittenAgain = rewritten.rewrite(queryShardContext()); + assertEquals(rewrittenAgain, expected); + assertEquals(QueryBuilder.rewriteQuery(boolQueryBuilder, queryShardContext()), expected); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTests.java index f7f50136f77..414ab1f1131 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.query; import org.apache.lucene.queries.BoostingQuery; +import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import java.io.IOException; @@ -72,27 +73,27 @@ public class BoostingQueryBuilderTests extends AbstractQueryTestCase rewrite = qb.rewrite(queryShardContext()); + if (positive instanceof MatchAllQueryBuilder && negative instanceof MatchAllQueryBuilder) { + assertSame(rewrite, qb); + } else { + assertNotSame(rewrite, qb); + assertEquals(new BoostingQueryBuilder(positive.rewrite(queryShardContext()), negative.rewrite(queryShardContext())), rewrite); + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java index 290a05d1fa1..410db8fc04d 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java @@ -468,4 +468,10 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase 0); + super.testMustRewrite(); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java index e7b2d862f5b..3d1f02c2e6c 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java @@ -411,4 +411,10 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase 0); + super.testMustRewrite(); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoDistanceRangeQueryTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoDistanceRangeQueryTests.java index 6d9095017bd..f07e695a1a0 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoDistanceRangeQueryTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoDistanceRangeQueryTests.java @@ -316,4 +316,10 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase 0); + super.testMustRewrite(); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java index ea16187f6d4..7ab62753889 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java @@ -343,4 +343,10 @@ public class GeoPolygonQueryBuilderTests extends AbstractQueryTestCase 0); + super.testMustRewrite(); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java index 93193cf11e8..533ce3bfb05 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java @@ -240,4 +240,24 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase rewrite = sqb.rewrite(queryShardContext()); + GeoShapeQueryBuilder geoShapeQueryBuilder = new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, indexedShapeToReturn); + geoShapeQueryBuilder.strategy(sqb.strategy()); + geoShapeQueryBuilder.relation(sqb.relation()); + assertEquals(geoShapeQueryBuilder, rewrite); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/GeohashCellQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeohashCellQueryBuilderTests.java index bdc892605d5..bf71e73aa46 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeohashCellQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeohashCellQueryBuilderTests.java @@ -145,4 +145,10 @@ public class GeohashCellQueryBuilderTests extends AbstractQueryTestCase checkGeneratedJson(json, parsed); assertEquals(json, 3, parsed.precision().intValue()); } + + @Override + public void testMustRewrite() throws IOException { + assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0); + super.testMustRewrite(); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java index 7a602f09418..c111684b25b 100644 --- a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.Query; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.script.Script.ScriptParseException; import org.elasticsearch.script.ScriptService.ScriptType; @@ -29,6 +30,7 @@ import org.elasticsearch.script.Template; import org.junit.BeforeClass; import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -118,4 +120,40 @@ public class TemplateQueryBuilderTests extends AbstractQueryTestCase builder = new TemplateQueryBuilder(new Template(query, ScriptType.INLINE, "mockscript", + XContentType.JSON, Collections.emptyMap())); + try { + builder.toQuery(queryShardContext()); + fail(); + } catch (UnsupportedOperationException ex) { + assertEquals("this query must be rewritten first", ex.getMessage()); + } + assertEquals(new MatchAllQueryBuilder(), builder.rewrite(queryShardContext())); + } + + public void testRewriteWithInnerName() throws IOException { + final String query = "{ \"match_all\" : {\"_name\" : \"foobar\"}}"; + QueryBuilder builder = new TemplateQueryBuilder(new Template(query, ScriptType.INLINE, "mockscript", + XContentType.JSON, Collections.emptyMap())); + assertEquals(new MatchAllQueryBuilder().queryName("foobar"), builder.rewrite(queryShardContext())); + + builder = new TemplateQueryBuilder(new Template(query, ScriptType.INLINE, "mockscript", + XContentType.JSON, Collections.emptyMap())).queryName("outer"); + assertEquals(new BoolQueryBuilder().must(new MatchAllQueryBuilder().queryName("foobar")).queryName("outer"), builder.rewrite(queryShardContext())); + } + + public void testRewriteWithInnerBoost() throws IOException { + final TermQueryBuilder query = new TermQueryBuilder("foo", "bar").boost(2); + QueryBuilder builder = new TemplateQueryBuilder(new Template(query.toString(), ScriptType.INLINE, "mockscript", + XContentType.JSON, Collections.emptyMap())); + assertEquals(query, builder.rewrite(queryShardContext())); + + builder = new TemplateQueryBuilder(new Template(query.toString(), ScriptType.INLINE, "mockscript", + XContentType.JSON, Collections.emptyMap())).boost(3); + assertEquals(new BoolQueryBuilder().must(query).boost(3), builder.rewrite(queryShardContext())); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java index 72241330289..9156108c63b 100644 --- a/core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java @@ -272,5 +272,17 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase rewrite = qb.rewrite(queryShardContext()); + assertEquals(tqb, rewrite); + } + + public void testRewriteWithInnerName() throws IOException { + QueryBuilder builder = new WrapperQueryBuilder("{ \"match_all\" : {\"_name\" : \"foobar\"}}"); + assertEquals(new MatchAllQueryBuilder().queryName("foobar"), builder.rewrite(queryShardContext())); + builder = new WrapperQueryBuilder("{ \"match_all\" : {\"_name\" : \"foobar\"}}").queryName("outer"); + assertEquals(new BoolQueryBuilder().must(new MatchAllQueryBuilder().queryName("foobar")).queryName("outer"), builder.rewrite(queryShardContext())); + } + + public void testRewriteWithInnerBoost() throws IOException { + final TermQueryBuilder query = new TermQueryBuilder("foo", "bar").boost(2); + QueryBuilder builder = new WrapperQueryBuilder(query.toString()); + assertEquals(query, builder.rewrite(queryShardContext())); + builder = new WrapperQueryBuilder(query.toString()).boost(3); + assertEquals(new BoolQueryBuilder().must(query).boost(3), builder.rewrite(queryShardContext())); + } + } From 297fb241123a00895d1e691be9e8a9802caa0c22 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Fri, 12 Feb 2016 12:53:18 +0100 Subject: [PATCH 3/6] apply fixes after review --- .../org/elasticsearch/messy/tests/TemplateQueryParserTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java index 96ca5ac9aa5..78c40088575 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java @@ -229,6 +229,7 @@ public class TemplateQueryParserTests extends ESTestCase { TemplateQueryParser parser = injector.getInstance(TemplateQueryParser.class); try { parser.fromXContent(context.parseContext()).toQuery(context); + fail(); } catch (UnsupportedOperationException ex) { assertEquals("this query must be rewritten first", ex.getMessage()); } From 387a55d5f9e1330d3cb6f6a9fb6c5d557e1a741e Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Fri, 12 Feb 2016 13:03:21 +0100 Subject: [PATCH 4/6] wrap lines --- .../elasticsearch/index/query/TemplateQueryBuilderTests.java | 3 ++- .../elasticsearch/index/query/WrapperQueryBuilderTests.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java index c111684b25b..d2842bc1efe 100644 --- a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java @@ -143,7 +143,8 @@ public class TemplateQueryBuilderTests extends AbstractQueryTestCase builder = new WrapperQueryBuilder("{ \"match_all\" : {\"_name\" : \"foobar\"}}"); assertEquals(new MatchAllQueryBuilder().queryName("foobar"), builder.rewrite(queryShardContext())); builder = new WrapperQueryBuilder("{ \"match_all\" : {\"_name\" : \"foobar\"}}").queryName("outer"); - assertEquals(new BoolQueryBuilder().must(new MatchAllQueryBuilder().queryName("foobar")).queryName("outer"), builder.rewrite(queryShardContext())); + assertEquals(new BoolQueryBuilder().must(new MatchAllQueryBuilder().queryName("foobar")).queryName("outer"), + builder.rewrite(queryShardContext())); } public void testRewriteWithInnerBoost() throws IOException { From 8d568ce3e195a725d81da7798373a8ec1bfe6a28 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Fri, 12 Feb 2016 15:14:28 +0100 Subject: [PATCH 5/6] Move optimization out of BoolQueryBuilder into tests --- .../index/query/BoolQueryBuilder.java | 7 ----- .../index/query/EmptyQueryBuilder.java | 1 + .../index/query/WrapperQueryBuilder.java | 2 ++ .../index/query/AbstractQueryTestCase.java | 12 ++++++-- .../index/query/BoolQueryBuilderTests.java | 8 ++--- .../query/TemplateQueryBuilderTests.java | 16 +++++++++- .../index/query/WrapperQueryBuilderTests.java | 28 ++++++++++------- .../FunctionScoreQueryBuilderTests.java | 30 +++++++++++++++++++ 8 files changed, 78 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index 261acff33ce..d9c277653e2 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -263,13 +263,6 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { @Override protected Query doToQuery(QueryShardContext context) throws IOException { - final int numClauses = mustNotClauses.size() + filterClauses.size() + shouldClauses.size() + mustClauses.size(); - if (numClauses == 1 && must().size() == 1 && boost() == DEFAULT_BOOST) { - // we really only optimize this for testing since we use this to rewrite - // named queries TemplateQueryBuilder and WrapperQueryBuilder. - // it's equivalent but will anyways happen later down the road in the BQ#rewrite method - return mustClauses.get(0).toQuery(context); - } BooleanQuery.Builder booleanQueryBuilder = new BooleanQuery.Builder(); booleanQueryBuilder.setDisableCoord(disableCoord); addBooleanClauses(context, booleanQueryBuilder, mustClauses, BooleanClause.Occur.MUST); diff --git a/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java index e810292a95c..b95befc0da8 100644 --- a/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; diff --git a/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java index 6c7ac0e9084..95086715f57 100644 --- a/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/WrapperQueryBuilder.java @@ -143,4 +143,6 @@ public class WrapperQueryBuilder extends AbstractQueryBuilder> assertLuceneQuery(secondQuery, secondLuceneQuery, context); SearchContext.removeCurrent(); - assertThat("two equivalent query builders lead to different lucene queries", secondLuceneQuery, equalTo(firstLuceneQuery)); + 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()) { @@ -546,8 +547,8 @@ public abstract class AbstractQueryTestCase> setSearchContext(randomTypes); Query thirdLuceneQuery = rewriteQuery(secondQuery, context).toQuery(context); SearchContext.removeCurrent(); - assertThat("modifying the boost doesn't affect the corresponding lucene query", firstLuceneQuery, - not(equalTo(thirdLuceneQuery))); + assertNotEquals("modifying the boost doesn't affect the corresponding lucene query", rewrite(firstLuceneQuery), + rewrite(thirdLuceneQuery)); } } } @@ -988,4 +989,9 @@ public abstract class AbstractQueryTestCase> setSearchContext(randomTypes); // only set search context for toQuery to be more realistic queryBuilder.toQuery(context); } + + protected Query rewrite(Query query) throws IOException { + return query; + } + } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 7b3f3ecb7fd..ccdb09eb507 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -195,14 +195,14 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase { @Override @@ -56,13 +54,9 @@ public class WrapperQueryBuilderTests extends AbstractQueryTestCase innerQuery = contextCopy.parseContext().parseInnerQueryBuilder(); - Query expected = innerQuery.toQuery(context); - assertThat(query, equalTo(expected)); - } + QueryBuilder innerQuery = queryBuilder.rewrite(queryShardContext()); + Query expected = rewrite(innerQuery.toQuery(context)); + assertEquals(rewrite(query), expected); } public void testIllegalArgument() { @@ -164,4 +158,16 @@ public class WrapperQueryBuilderTests extends AbstractQueryTestCase 0); + super.testMustRewrite(); + } + + public void testRewrite() throws IOException { + FunctionScoreQueryBuilder functionScoreQueryBuilder = new FunctionScoreQueryBuilder(new WrapperQueryBuilder(new TermQueryBuilder("foo", "bar").toString())); + FunctionScoreQueryBuilder rewrite = (FunctionScoreQueryBuilder) functionScoreQueryBuilder.rewrite(queryShardContext()); + assertNotSame(functionScoreQueryBuilder, rewrite); + assertEquals(rewrite.query(), new TermQueryBuilder("foo", "bar")); + } + + public void testRewriteWithFunction() throws IOException { + TermQueryBuilder secondFunction = new TermQueryBuilder("tq", "2"); + QueryBuilder queryBuilder = randomBoolean() ? new WrapperQueryBuilder(new TermQueryBuilder("foo", "bar").toString()) : new TermQueryBuilder("foo", "bar"); + FunctionScoreQueryBuilder functionScoreQueryBuilder = new FunctionScoreQueryBuilder(queryBuilder, + new FunctionScoreQueryBuilder.FilterFunctionBuilder[]{ + new FunctionScoreQueryBuilder.FilterFunctionBuilder(new WrapperQueryBuilder(new TermQueryBuilder("tq", "1").toString()), new RandomScoreFunctionBuilder()), + new FunctionScoreQueryBuilder.FilterFunctionBuilder(secondFunction, new RandomScoreFunctionBuilder()) + + }); + FunctionScoreQueryBuilder rewrite = (FunctionScoreQueryBuilder) functionScoreQueryBuilder.rewrite(queryShardContext()); + assertNotSame(functionScoreQueryBuilder, rewrite); + assertEquals(rewrite.query(), new TermQueryBuilder("foo", "bar")); + assertEquals(rewrite.filterFunctionBuilders()[0].getFilter(), new TermQueryBuilder("tq", "1")); + assertSame(rewrite.filterFunctionBuilders()[1].getFilter(), secondFunction); + } } From 84d91c87a1089c1b50d1132ab80f2ad41c1772ce Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Fri, 12 Feb 2016 15:34:50 +0100 Subject: [PATCH 6/6] fix test --- .../org/elasticsearch/index/query/TermsQueryBuilderTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java index 9156108c63b..4b6931bf5ad 100644 --- a/core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java @@ -41,6 +41,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.stream.Collectors; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -282,7 +283,8 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase x != null).collect(Collectors.toList()))); // terms lookup removes null values } }