From a3c294d4e9415445ebb451aff2433a99efe2c4a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 5 Aug 2015 19:19:45 +0200 Subject: [PATCH] Move flag to track filter context to QueryShardContext Currently there is a flag in the QueryParseContext that keeps track of whether an inner query sits inside a filter and should therefore produce an unscored lucene query. This is done in the parseInnerFilter...() methods that are called in the fromXContent() methods or the parse() methods we haven't refactored yet. This needs to move to the toQuery() method in the refactored builders, since the query builders themselves have no information about the parent query they might be nested in. This PR moves the isFilter flag from the QueryParseContext to the re- cently introduces QueryShardContext. The parseInnerFilter... methods need to stay in the QueryParseContext for now, but they already delegate to the flag that is kept in QueryShardContext. For refactored queries (like BoolQueryBuilder) references to isFilter() are moved from the parser to the corresponding builder. Builders where the inner query was previously parsed using parseInnerFilter...() now use a newly introduces toFilter(shardContext) method that produces the nested lucene query with the filter context flag switched on. Closes #12731 --- .../index/query/AbstractQueryBuilder.java | 13 +++++++++++++ .../index/query/AndQueryBuilder.java | 2 +- .../index/query/BoolQueryBuilder.java | 18 ++++++++++++++++-- .../index/query/BoolQueryParser.java | 12 ------------ .../query/ConstantScoreQueryBuilder.java | 4 ++-- .../index/query/EmptyQueryBuilder.java | 7 +++++++ .../index/query/FQueryFilterBuilder.java | 1 - .../index/query/FilteredQueryBuilder.java | 2 +- .../index/query/FuzzyQueryParser.java | 2 +- .../index/query/NotQueryBuilder.java | 2 +- .../index/query/OrQueryBuilder.java | 2 +- .../index/query/QueryBuilder.java | 14 +++++++++++++- .../index/query/QueryParseContext.java | 19 ++++++++----------- .../index/query/QueryShardContext.java | 2 +- .../index/query/TermsQueryParser.java | 2 +- .../query/plugin/DummyQueryParserPlugin.java | 2 +- 16 files changed, 67 insertions(+), 37 deletions(-) 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 e54b4844412..25d380b09eb 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -79,6 +79,19 @@ public abstract class AbstractQueryBuilder exte return query; } + @Override + public final Query toFilter(QueryShardContext context) throws IOException { + Query result = null; + final boolean originalIsFilter = context.isFilter; + try { + context.isFilter = true; + result = toQuery(context); + } finally { + context.isFilter = originalIsFilter; + } + return result; + } + //norelease to be made abstract once all query builders override doToQuery providing their own specific implementation. protected Query doToQuery(QueryShardContext context) throws IOException { return context.indexQueryParserService().queryParser(getName()).parse(context); diff --git a/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java index 05ef359f7ef..477f34d272f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java @@ -92,7 +92,7 @@ public class AndQueryBuilder extends AbstractQueryBuilder { BooleanQuery query = new BooleanQuery(); for (QueryBuilder f : filters) { - Query innerQuery = f.toQuery(context); + Query innerQuery = f.toFilter(context); // ignore queries that are null if (innerQuery != null) { query.add(innerQuery, Occur.MUST); 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 9ce5a396735..32d9c852640 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -279,9 +279,23 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { return validationException; } - private static void addBooleanClauses(QueryShardContext context, BooleanQuery booleanQuery, List clauses, Occur occurs) throws IOException { + private void addBooleanClauses(QueryShardContext context, BooleanQuery booleanQuery, List clauses, Occur occurs) throws IOException { for (QueryBuilder query : clauses) { - Query luceneQuery = query.toQuery(context); + Query luceneQuery = null; + switch (occurs) { + case SHOULD: + if (context.isFilter() && minimumShouldMatch == null) { + minimumShouldMatch = "1"; + } + luceneQuery = query.toQuery(context); + break; + case FILTER: + case MUST_NOT: + luceneQuery = query.toFilter(context); + break; + case MUST: + luceneQuery = query.toQuery(context); + } if (luceneQuery != null) { booleanQuery.add(new BooleanClause(luceneQuery, occurs)); } diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java index ad919de1f0f..8858c70aaa8 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java @@ -76,12 +76,6 @@ public class BoolQueryParser extends BaseQueryParser { case "should": query = parseContext.parseInnerQueryBuilder(); shouldClauses.add(query); - // EmptyQueryBuilder does not add lucene query later, skip setting minuminShouldMatch - if (query != EmptyQueryBuilder.PROTOTYPE) { - if (parseContext.isFilter() && minimumShouldMatch == null) { - minimumShouldMatch = "1"; - } - } break; case "filter": query = parseContext.parseInnerFilterToQueryBuilder(); @@ -105,12 +99,6 @@ public class BoolQueryParser extends BaseQueryParser { case "should": query = parseContext.parseInnerQueryBuilder(); shouldClauses.add(query); - // EmptyQueryBuilder does not add lucene query later, skip setting minuminShouldMatch - if (query != EmptyQueryBuilder.PROTOTYPE) { - if (parseContext.isFilter() && minimumShouldMatch == null) { - minimumShouldMatch = "1"; - } - } break; case "filter": query = parseContext.parseInnerFilterToQueryBuilder(); 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 9648db8cd7f..99a7a31627f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -68,12 +68,12 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder { @Override protected Query doToQuery(QueryShardContext context) throws IOException { - Query luceneQuery = filter.toQuery(context); + Query luceneQuery = filter.toFilter(context); if (luceneQuery == null) { return null; } diff --git a/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java index 84452f64ccb..03951102eb5 100644 --- a/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java @@ -89,7 +89,7 @@ public class OrQueryBuilder extends AbstractQueryBuilder { BooleanQuery query = new BooleanQuery(); for (QueryBuilder f : filters) { - Query innerQuery = f.toQuery(context); + Query innerQuery = f.toFilter(context); // ignore queries that are null if (innerQuery != null) { query.add(innerQuery, Occur.SHOULD); 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 02bbfb68f1b..3f693756f59 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryBuilder.java @@ -29,7 +29,7 @@ import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; public interface QueryBuilder extends NamedWriteable, ToXContent { - + /** * Validate the query. * @return a {@link QueryValidationException} containing error messages, {@code null} if query is valid. @@ -49,6 +49,18 @@ public interface QueryBuilder extends NamedWriteablenull if this query should be ignored in the context of + * parent queries. + * + * @param context additional information needed to construct the queries + * @return the {@link Query} or null if this query should be ignored upstream + * @throws QueryShardException + * @throws IOException + */ + Query toFilter(QueryShardContext context) throws IOException; + /** * Returns a {@link org.elasticsearch.common.bytes.BytesReference} * containing the {@link ToXContent} output in binary format. 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 de108778d7d..90b45e335c1 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -37,7 +37,6 @@ public class QueryParseContext { private XContentParser parser; private final Index index; //norelease this flag is also used in the QueryShardContext, we need to make sure we set it there correctly in doToQuery() - private boolean isFilter; private ParseFieldMatcher parseFieldMatcher; //norelease this can eventually be deleted when context() method goes away @@ -172,34 +171,32 @@ public class QueryParseContext { * @throws IOException */ @Nullable + //norelease setting and checking the isFilter Flag should completely be moved to toQuery/toFilter after query refactoring public QueryBuilder parseInnerFilterToQueryBuilder() throws IOException { - final boolean originalIsFilter = isFilter; + final boolean originalIsFilter = this.shardContext.isFilter; try { - isFilter = true; + this.shardContext.isFilter = true; return parseInnerQueryBuilder(); } finally { - isFilter = originalIsFilter; + this.shardContext.isFilter = originalIsFilter; } } + //norelease setting and checking the isFilter Flag should completely be moved to toQuery/toFilter after query refactoring QueryBuilder parseInnerFilterToQueryBuilder(String queryName) throws IOException, QueryParsingException { - final boolean originalIsFilter = isFilter; + final boolean originalIsFilter = this.shardContext.isFilter; try { - isFilter = true; + this.shardContext.isFilter = true; QueryParser queryParser = queryParser(queryName); if (queryParser == null) { throw new QueryParsingException(this, "No query registered for [" + queryName + "]"); } return queryParser.fromXContent(this); } finally { - isFilter = originalIsFilter; + this.shardContext.isFilter = originalIsFilter; } } - public boolean isFilter() { - return this.isFilter; - } - public ParseFieldMatcher parseFieldMatcher() { return parseFieldMatcher; } 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 773fbcb7f86..a55d545d919 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -103,7 +103,7 @@ public class QueryShardContext { //norelease this should be possible to remove once query context are completely separated private QueryParseContext parseContext; - private boolean isFilter; + boolean isFilter; public QueryShardContext(Index index, IndexQueryParserService indexQueryParser) { this.index = index; diff --git a/core/src/main/java/org/elasticsearch/index/query/TermsQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/TermsQueryParser.java index fec398f298e..0bbbef8f670 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TermsQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/TermsQueryParser.java @@ -180,7 +180,7 @@ public class TermsQueryParser extends BaseQueryParserTemp { } Query query; - if (parseContext.isFilter()) { + if (context.isFilter()) { if (fieldType != null) { query = fieldType.termsQuery(terms, context); } else { diff --git a/core/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java b/core/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java index 7421a114d21..d8b7fc97db1 100644 --- a/core/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java +++ b/core/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java @@ -81,7 +81,7 @@ public class DummyQueryParserPlugin extends AbstractPlugin { public Query parse(QueryShardContext context) throws IOException, QueryShardException { XContentParser.Token token = context.parseContext().parser().nextToken(); assert token == XContentParser.Token.END_OBJECT; - return new DummyQuery(context.parseContext().isFilter()); + return new DummyQuery(context.isFilter()); } @Override