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.
This commit is contained in:
javanna 2015-09-24 17:21:34 +02:00 committed by Luca Cavanna
parent 1968d3e0f0
commit aae7faa88e
5 changed files with 14 additions and 44 deletions

View File

@ -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) { private Query parseQuery(String type, XContentParser parser) {
String[] previousTypes = null; String[] previousTypes = null;
if (type != null) { if (type != null) {

View File

@ -154,11 +154,7 @@ public class IndexQueryParserService extends AbstractIndexComponent {
} }
public ParsedQuery parse(BytesReference source) { public ParsedQuery parse(BytesReference source) {
return parse(cache.get(), source); QueryShardContext context = cache.get();
}
//norelease
public ParsedQuery parse(QueryShardContext context, BytesReference source) {
XContentParser parser = null; XContentParser parser = null;
try { try {
parser = XContentFactory.xContent(source).createParser(source); 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. * Parses an inner filter, returning null if the filter should be ignored.
*/ */
@Nullable @Nullable
//norelease
public ParsedQuery parseInnerFilter(XContentParser parser) throws IOException { public ParsedQuery parseInnerFilter(XContentParser parser) throws IOException {
QueryShardContext context = cache.get(); QueryShardContext context = cache.get();
context.reset(parser); context.reset(parser);
try { try {
context.parseFieldMatcher(parseFieldMatcher);
Query filter = context.parseContext().parseInnerQueryBuilder().toFilter(context); Query filter = context.parseContext().parseInnerQueryBuilder().toFilter(context);
if (filter == null) { if (filter == null) {
return null; return null;
@ -202,13 +198,6 @@ public class IndexQueryParserService extends AbstractIndexComponent {
} }
@Nullable @Nullable
public QueryBuilder parseInnerQueryBuilder(QueryParseContext parseContext) throws IOException {
parseContext.parseFieldMatcher(parseFieldMatcher);
return parseContext.parseInnerQueryBuilder();
}
@Nullable
//norelease
public Query parseInnerQuery(QueryShardContext context) throws IOException { public Query parseInnerQuery(QueryShardContext context) throws IOException {
Query query = context.parseContext().parseInnerQueryBuilder().toQuery(context); Query query = context.parseContext().parseInnerQueryBuilder().toQuery(context);
if (query == null) { if (query == null) {
@ -265,23 +254,18 @@ public class IndexQueryParserService extends AbstractIndexComponent {
} }
} }
//norelease
private ParsedQuery innerParse(QueryShardContext context, XContentParser parser) throws IOException, QueryShardException { private ParsedQuery innerParse(QueryShardContext context, XContentParser parser) throws IOException, QueryShardException {
context.reset(parser); context.reset(parser);
try { try {
context.parseFieldMatcher(parseFieldMatcher); context.parseFieldMatcher(parseFieldMatcher);
return innerParse(context, context.parseContext().parseInnerQueryBuilder()); Query query = context.parseContext().parseInnerQueryBuilder().toQuery(context);
} finally {
context.reset(null);
}
}
private static ParsedQuery innerParse(QueryShardContext context, QueryBuilder queryBuilder) throws IOException, QueryShardException {
Query query = queryBuilder.toQuery(context);
if (query == null) { if (query == null) {
query = Queries.newMatchNoDocsQuery(); query = Queries.newMatchNoDocsQuery();
} }
return new ParsedQuery(query, context.copyNamedQueries()); return new ParsedQuery(query, context.copyNamedQueries());
} finally {
context.reset(null);
}
} }
public ParseFieldMatcher parseFieldMatcher() { public ParseFieldMatcher parseFieldMatcher() {

View File

@ -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 static final ParseField CACHE_KEY = new ParseField("_cache_key").withAllDeprecated("Filters are always used as cache keys");
private XContentParser parser; 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; private ParseFieldMatcher parseFieldMatcher = ParseFieldMatcher.EMPTY;
//norelease this can eventually be deleted when context() method goes away
private final QueryShardContext shardContext;
private IndicesQueriesRegistry indicesQueriesRegistry; private IndicesQueriesRegistry indicesQueriesRegistry;
public QueryParseContext(IndicesQueriesRegistry registry) { public QueryParseContext(IndicesQueriesRegistry registry) {
this.indicesQueriesRegistry = registry; this.indicesQueriesRegistry = registry;
this.shardContext = null;
}
QueryParseContext(QueryShardContext context) {
this.shardContext = context;
this.indicesQueriesRegistry = context.indexQueryParserService().indicesQueriesRegistry();
} }
public void reset(XContentParser jp) { public void reset(XContentParser jp) {
@ -129,7 +120,7 @@ public class QueryParseContext {
* @param name the name of the parser to retrieve * @param name the name of the parser to retrieve
* @return the query parser * @return the query parser
*/ */
QueryParser queryParser(String name) { private QueryParser queryParser(String name) {
return indicesQueriesRegistry.queryParsers().get(name); return indicesQueriesRegistry.queryParsers().get(name);
} }
} }

View File

@ -89,15 +89,12 @@ public class QueryShardContext {
private final MapperQueryParser queryParser = new MapperQueryParser(this); private final MapperQueryParser queryParser = new MapperQueryParser(this);
private ParseFieldMatcher parseFieldMatcher;
private boolean allowUnmappedFields; private boolean allowUnmappedFields;
private boolean mapUnmappedFieldAsString; private boolean mapUnmappedFieldAsString;
private NestedScope nestedScope; private NestedScope nestedScope;
//norelease this should be possible to remove once query context are completely separated
private QueryParseContext parseContext; private QueryParseContext parseContext;
boolean isFilter; boolean isFilter;
@ -106,17 +103,15 @@ public class QueryShardContext {
this.index = index; this.index = index;
this.indexVersionCreated = Version.indexCreated(indexQueryParser.indexSettings()); this.indexVersionCreated = Version.indexCreated(indexQueryParser.indexSettings());
this.indexQueryParser = indexQueryParser; this.indexQueryParser = indexQueryParser;
this.parseContext = new QueryParseContext(this); this.parseContext = new QueryParseContext(indexQueryParser.indicesQueriesRegistry());
} }
public void parseFieldMatcher(ParseFieldMatcher parseFieldMatcher) { public void parseFieldMatcher(ParseFieldMatcher parseFieldMatcher) {
//norelease ParseFieldMatcher is currently duplicated, this should be cleaned up
this.parseFieldMatcher = parseFieldMatcher;
this.parseContext.parseFieldMatcher(parseFieldMatcher); this.parseContext.parseFieldMatcher(parseFieldMatcher);
} }
public ParseFieldMatcher parseFieldMatcher() { public ParseFieldMatcher parseFieldMatcher() {
return parseFieldMatcher; return parseContext.parseFieldMatcher();
} }
public void reset() { public void reset() {
@ -127,7 +122,6 @@ public class QueryShardContext {
this.nestedScope = new NestedScope(); this.nestedScope = new NestedScope();
} }
//norelease remove parser argument once query contexts are separated
public void reset(XContentParser jp) { public void reset(XContentParser jp) {
this.reset(); this.reset();
this.parseContext.reset(jp); this.parseContext.reset(jp);
@ -137,7 +131,6 @@ public class QueryShardContext {
return this.index; 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() { public IndexQueryParserService indexQueryParserService() {
return indexQueryParser; return indexQueryParser;
} }

View File

@ -482,7 +482,10 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
* @return a new {@link QueryParseContext} based on the base test index and queryParserService * @return a new {@link QueryParseContext} based on the base test index and queryParserService
*/ */
protected static QueryParseContext createParseContext() { protected static QueryParseContext createParseContext() {
return createShardContext().parseContext(); QueryParseContext queryParseContext = new QueryParseContext(queryParserService.indicesQueriesRegistry());
queryParseContext.reset(null);
queryParseContext.parseFieldMatcher(ParseFieldMatcher.STRICT);
return queryParseContext;
} }
/** /**