Skip to content

Commit

Permalink
Query refactoring: Introduce toQuery() and fromXContent() in QueryBui…
Browse files Browse the repository at this point in the history
…lders and QueryParsers

The planed refactoring of search queries layed out in elastic#9901 requires to split the "parse()"
method in QueryParsers into two methods, first a "fromXContent(...)" method that allows parsing
to an intermediate query representation (currently called FooQueryBuilder) and second a
"Query toQuery(...)" method on these intermediate representations that create the actual lucene queries.

This PR is a first step in that direction as it introduces the interface changes necessary for the further
refactoring. It introduces the new interface methods while for now keeping the old Builder/Parsers still
in place by delegating the new "toQuery()" implementations to the existing "parse()" methods, and by
introducing a "catch-all" "fromXContent()" implementation in a BaseQueryParser that returns a temporary
QueryBuilder wrapper implementation. This allows us to refactor the existing QueryBuilders step by step
while already beeing able to start refactoring queries with nested inner queries.

Closes elastic#10580
  • Loading branch information
Christoph Büscher committed Apr 17, 2015
1 parent 5af7a5f commit 2fd3e79
Show file tree
Hide file tree
Showing 81 changed files with 471 additions and 84 deletions.
10 changes: 10 additions & 0 deletions src/main/java/org/elasticsearch/index/query/BaseQueryBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.index.query;

import org.apache.lucene.search.Query;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down Expand Up @@ -68,5 +69,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
return builder;
}

public Query toQuery(QueryParseContext parseContext) throws QueryParsingException, IOException {
return parseContext.indexQueryParserService().queryParser(parserName()).parse(parseContext);
}

/**
* @return the name of the parser class the default toQuery() method delegates to
*/
protected abstract String parserName();

protected abstract void doXContent(XContentBuilder builder, Params params) throws IOException;
}
37 changes: 37 additions & 0 deletions src/main/java/org/elasticsearch/index/query/BaseQueryParser.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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.apache.lucene.search.Query;

import java.io.IOException;

/**
* Class used during the query parsers refactoring.
* All query parsers that have a refactored "fromXContent" method can be changed to extend this instead of {@link BaseQueryParserTemp}.
* Keeps old {@link QueryParser#parse(QueryParseContext)} method as a stub delegating to
* {@link QueryParser#fromXContent(QueryParseContext)} and {@link QueryBuilder#toQuery(QueryParseContext)}}
*/
public abstract class BaseQueryParser implements QueryParser {

public final Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
return fromXContent(parseContext).toQuery(parseContext);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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.apache.lucene.search.Query;

import java.io.IOException;

/**
* This class with method impl is an intermediate step in the refactoring.
* Method should be should be overwritten for all queries that already implement the toQuery/fromXContent split correctly.
* To be removed once all queries are refactored.
*/
public abstract class BaseQueryParserTemp implements QueryParser {

public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
Query query = parse(parseContext);
return new QueryWrappingQueryBuilder(query);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class BoolQueryBuilder extends BaseQueryBuilder implements BoostableQuery
private Boolean disableCoord;

private String minimumShouldMatch;

private Boolean adjustPureNegative;

private String queryName;
Expand Down Expand Up @@ -126,7 +126,7 @@ public BoolQueryBuilder minimumShouldMatch(String minimumShouldMatch) {
public boolean hasClauses() {
return !(mustClauses.isEmpty() && shouldClauses.isEmpty() && mustNotClauses.isEmpty());
}

/**
* If a boolean query contains only negative ("must not") clauses should the
* BooleanQuery be enhanced with a {@link MatchAllDocsQuery} in order to act
Expand Down Expand Up @@ -185,4 +185,7 @@ private void doXArrayContent(String field, List<QueryBuilder> clauses, XContentB
}
}

final protected String parserName() {
return BoolQueryParser.NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
/**
*
*/
public class BoolQueryParser implements QueryParser {
public class BoolQueryParser extends BaseQueryParserTemp {

public static final String NAME = "bool";

Expand All @@ -62,7 +62,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
List<BooleanClause> clauses = newArrayList();
boolean adjustPureNegative = true;
String queryName = null;

String currentFieldName = null;
XContentParser.Token token;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,8 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
}
builder.endObject();
}

final protected String parserName() {
return BoostingQueryParser.NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
/**
*
*/
public class BoostingQueryParser implements QueryParser {
public class BoostingQueryParser extends BaseQueryParserTemp {

public static final String NAME = "boosting";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

package org.elasticsearch.index.query;

import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.similarities.Similarity;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.common.xcontent.XContentBuilder;

Expand Down Expand Up @@ -198,4 +201,8 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio
builder.endObject();
builder.endObject();
}

final protected String parserName() {
return CommonTermsQueryParser.NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
/**
*
*/
public class CommonTermsQueryParser implements QueryParser {
public class CommonTermsQueryParser extends BaseQueryParserTemp {

public static final String NAME = "common";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.index.query;

import org.apache.lucene.search.Query;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;
Expand All @@ -35,7 +36,7 @@ public class ConstantScoreQueryBuilder extends BaseQueryBuilder implements Boost
private final QueryBuilder queryBuilder;

private float boost = -1;


/**
* A query that wraps a filter and simply returns a constant score equal to the
Expand All @@ -56,7 +57,7 @@ public ConstantScoreQueryBuilder(FilterBuilder filterBuilder) {
public ConstantScoreQueryBuilder(QueryBuilder queryBuilder) {
this.filterBuilder = null;
this.queryBuilder = queryBuilder;
}
}

/**
* Sets the boost for this query. Documents matching this query will (in addition to the normal
Expand All @@ -77,12 +78,16 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
queryBuilder.toXContent(builder, params);
} else {
builder.field("filter");
filterBuilder.toXContent(builder, params);
filterBuilder.toXContent(builder, params);
}

if (boost != -1) {
builder.field("boost", boost);
}
builder.endObject();
}
}

final protected String parserName() {
return ConstantScoreQueryParser.NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
/**
*
*/
public class ConstantScoreQueryParser implements QueryParser {
public class ConstantScoreQueryParser extends BaseQueryParserTemp {

public static final String NAME = "constant_score";

Expand Down Expand Up @@ -108,4 +108,4 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
query.setBoost(boost);
return query;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.index.query;

import org.apache.lucene.search.Query;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;
Expand Down Expand Up @@ -99,4 +100,8 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
builder.endArray();
builder.endObject();
}
}

final protected String parserName() {
return DisMaxQueryParser.NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
/**
*
*/
public class DisMaxQueryParser implements QueryParser {
public class DisMaxQueryParser extends BaseQueryParserTemp {

public static final String NAME = "dis_max";

Expand Down Expand Up @@ -113,4 +113,4 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
}
return query;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,8 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
}
builder.endObject();
}

final protected String parserName() {
return FieldMaskingSpanQueryParser.NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
/**
*
*/
public class FieldMaskingSpanQueryParser implements QueryParser {
public class FieldMaskingSpanQueryParser extends BaseQueryParserTemp {

public static final String NAME = "field_masking_span";

Expand Down Expand Up @@ -101,4 +101,4 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
}
return query;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,8 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
}
builder.endObject();
}

final protected String parserName() {
return FilteredQueryParser.NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
/**
*
*/
public class FilteredQueryParser implements QueryParser {
public class FilteredQueryParser extends BaseQueryParserTemp {

public static final String NAME = "filtered";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class FuzzyQueryBuilder extends BaseQueryBuilder implements MultiTermQuer
private Integer prefixLength;

private Integer maxExpansions;

//LUCENE 4 UPGRADE we need a testcase for this + documentation
private Boolean transpositions;

Expand Down Expand Up @@ -83,7 +83,7 @@ public FuzzyQueryBuilder maxExpansions(int maxExpansions) {
this.maxExpansions = maxExpansions;
return this;
}

public FuzzyQueryBuilder transpositions(boolean transpositions) {
this.transpositions = transpositions;
return this;
Expand Down Expand Up @@ -127,4 +127,8 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio
}
builder.endObject();
}
}

final protected String parserName() {
return FuzzyQueryParser.NAME;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
/**
*
*/
public class FuzzyQueryParser implements QueryParser {
public class FuzzyQueryParser extends BaseQueryParserTemp {

public static final String NAME = "fuzzy";
private static final Fuzziness DEFAULT_FUZZINESS = Fuzziness.AUTO;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,7 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
builder.endObject();
}

final protected String parserName() {
return GeoShapeQueryParser.NAME;
}
}
Loading

0 comments on commit 2fd3e79

Please sign in to comment.