From aae7faa88e55db81d3fa61a6fdb91fe14558efbd Mon Sep 17 00:00:00 2001 From: javanna Date: Thu, 24 Sep 2015 17:21:34 +0200 Subject: [PATCH] Query refactoring: minor cleanups before we merge into master Remove ParseFieldMatcher duplication query in both contexts. QueryParseContext is still contained in QueryShardContext, as parsing still happens in the shards here and there. Most of the norelease comments have been removed simply because the scope of the refactoring has become smaller. Some could only be removed once everything, the whole search request, gets parsed on the coordinating node. We will get there eventually. --- .../percolator/PercolatorQueriesRegistry.java | 1 - .../index/query/IndexQueryParserService.java | 30 +++++-------------- .../index/query/QueryParseContext.java | 11 +------ .../index/query/QueryShardContext.java | 11 ++----- .../index/query/AbstractQueryTestCase.java | 5 +++- 5 files changed, 14 insertions(+), 44 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java b/core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java index 9dc25b2c4ea..06d04f60d61 100644 --- a/core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java +++ b/core/src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java @@ -184,7 +184,6 @@ public class PercolatorQueriesRegistry extends AbstractIndexShardComponent imple } } - //norelease this method parses from xcontent to lucene query, need to re-investigate how to split context here private Query parseQuery(String type, XContentParser parser) { String[] previousTypes = null; if (type != null) { diff --git a/core/src/main/java/org/elasticsearch/index/query/IndexQueryParserService.java b/core/src/main/java/org/elasticsearch/index/query/IndexQueryParserService.java index 0d4d22ad5a0..d4f7491fb11 100644 --- a/core/src/main/java/org/elasticsearch/index/query/IndexQueryParserService.java +++ b/core/src/main/java/org/elasticsearch/index/query/IndexQueryParserService.java @@ -154,11 +154,7 @@ public class IndexQueryParserService extends AbstractIndexComponent { } public ParsedQuery parse(BytesReference source) { - return parse(cache.get(), source); - } - - //norelease - public ParsedQuery parse(QueryShardContext context, BytesReference source) { + QueryShardContext context = cache.get(); XContentParser parser = null; try { parser = XContentFactory.xContent(source).createParser(source); @@ -186,11 +182,11 @@ public class IndexQueryParserService extends AbstractIndexComponent { * Parses an inner filter, returning null if the filter should be ignored. */ @Nullable - //norelease public ParsedQuery parseInnerFilter(XContentParser parser) throws IOException { QueryShardContext context = cache.get(); context.reset(parser); try { + context.parseFieldMatcher(parseFieldMatcher); Query filter = context.parseContext().parseInnerQueryBuilder().toFilter(context); if (filter == null) { return null; @@ -202,13 +198,6 @@ public class IndexQueryParserService extends AbstractIndexComponent { } @Nullable - public QueryBuilder parseInnerQueryBuilder(QueryParseContext parseContext) throws IOException { - parseContext.parseFieldMatcher(parseFieldMatcher); - return parseContext.parseInnerQueryBuilder(); - } - - @Nullable - //norelease public Query parseInnerQuery(QueryShardContext context) throws IOException { Query query = context.parseContext().parseInnerQueryBuilder().toQuery(context); if (query == null) { @@ -265,25 +254,20 @@ public class IndexQueryParserService extends AbstractIndexComponent { } } - //norelease private ParsedQuery innerParse(QueryShardContext context, XContentParser parser) throws IOException, QueryShardException { context.reset(parser); try { context.parseFieldMatcher(parseFieldMatcher); - return innerParse(context, context.parseContext().parseInnerQueryBuilder()); + Query query = context.parseContext().parseInnerQueryBuilder().toQuery(context); + if (query == null) { + query = Queries.newMatchNoDocsQuery(); + } + return new ParsedQuery(query, context.copyNamedQueries()); } finally { context.reset(null); } } - private static ParsedQuery innerParse(QueryShardContext context, QueryBuilder queryBuilder) throws IOException, QueryShardException { - Query query = queryBuilder.toQuery(context); - if (query == null) { - query = Queries.newMatchNoDocsQuery(); - } - return new ParsedQuery(query, context.copyNamedQueries()); - } - public ParseFieldMatcher parseFieldMatcher() { return parseFieldMatcher; } 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 5accd2823fe..de85abd347a 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -33,21 +33,12 @@ public class QueryParseContext { private static final ParseField CACHE_KEY = new ParseField("_cache_key").withAllDeprecated("Filters are always used as cache keys"); private XContentParser parser; - //norelease this flag is also used in the QueryShardContext, we need to make sure we set it there correctly in doToQuery() private ParseFieldMatcher parseFieldMatcher = ParseFieldMatcher.EMPTY; - //norelease this can eventually be deleted when context() method goes away - private final QueryShardContext shardContext; private IndicesQueriesRegistry indicesQueriesRegistry; public QueryParseContext(IndicesQueriesRegistry registry) { this.indicesQueriesRegistry = registry; - this.shardContext = null; - } - - QueryParseContext(QueryShardContext context) { - this.shardContext = context; - this.indicesQueriesRegistry = context.indexQueryParserService().indicesQueriesRegistry(); } public void reset(XContentParser jp) { @@ -129,7 +120,7 @@ public class QueryParseContext { * @param name the name of the parser to retrieve * @return the query parser */ - QueryParser queryParser(String name) { + private QueryParser queryParser(String name) { return indicesQueriesRegistry.queryParsers().get(name); } } 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 38d27f3f625..23fc2bb864c 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -89,15 +89,12 @@ public class QueryShardContext { private final MapperQueryParser queryParser = new MapperQueryParser(this); - private ParseFieldMatcher parseFieldMatcher; - private boolean allowUnmappedFields; private boolean mapUnmappedFieldAsString; private NestedScope nestedScope; - //norelease this should be possible to remove once query context are completely separated private QueryParseContext parseContext; boolean isFilter; @@ -106,17 +103,15 @@ public class QueryShardContext { this.index = index; this.indexVersionCreated = Version.indexCreated(indexQueryParser.indexSettings()); this.indexQueryParser = indexQueryParser; - this.parseContext = new QueryParseContext(this); + this.parseContext = new QueryParseContext(indexQueryParser.indicesQueriesRegistry()); } public void parseFieldMatcher(ParseFieldMatcher parseFieldMatcher) { - //norelease ParseFieldMatcher is currently duplicated, this should be cleaned up - this.parseFieldMatcher = parseFieldMatcher; this.parseContext.parseFieldMatcher(parseFieldMatcher); } public ParseFieldMatcher parseFieldMatcher() { - return parseFieldMatcher; + return parseContext.parseFieldMatcher(); } public void reset() { @@ -127,7 +122,6 @@ public class QueryShardContext { this.nestedScope = new NestedScope(); } - //norelease remove parser argument once query contexts are separated public void reset(XContentParser jp) { this.reset(); this.parseContext.reset(jp); @@ -137,7 +131,6 @@ public class QueryShardContext { return this.index; } - //norelease we might be able to avoid exposing the service to the outside world once all queries are refactored public IndexQueryParserService indexQueryParserService() { return indexQueryParser; } 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 b9c984975f9..930dbd90af7 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -482,7 +482,10 @@ public abstract class AbstractQueryTestCase> * @return a new {@link QueryParseContext} based on the base test index and queryParserService */ protected static QueryParseContext createParseContext() { - return createShardContext().parseContext(); + QueryParseContext queryParseContext = new QueryParseContext(queryParserService.indicesQueriesRegistry()); + queryParseContext.reset(null); + queryParseContext.parseFieldMatcher(ParseFieldMatcher.STRICT); + return queryParseContext; } /**