Query Refactoring: ExistsQueryBuilder and Parser

Refactors ExistsQueryBuilder and Parser, splitting the parse() method into a parsing
and a query building part. Also moving newFilter() utility method from parser to query builder.

Changes in the BaseQueryTestCase include introduction of randomized version to test disabled
FieldNamesFieldMappers and also getting rid of the need for createEmptyBuilder() method by
using registered prototype constants.

Relates to #10217
Closes #11427
This commit is contained in:
Christoph Büscher 2015-05-29 17:49:54 +02:00
parent 4f4b1b74df
commit c928852d4a
13 changed files with 248 additions and 108 deletions

View File

@ -21,7 +21,7 @@ package org.apache.lucene.queryparser.classic;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.index.query.ExistsQueryParser;
import org.elasticsearch.index.query.ExistsQueryBuilder;
import org.elasticsearch.index.query.QueryParseContext;
/**
@ -33,6 +33,6 @@ public class ExistsFieldQueryExtension implements FieldQueryExtension {
@Override
public Query query(QueryParseContext parseContext, String queryText) {
return new ConstantScoreQuery(ExistsQueryParser.newFilter(parseContext, queryText, null));
return new ConstantScoreQuery(ExistsQueryBuilder.newFilter(parseContext, queryText, null));
}
}

View File

@ -19,18 +19,31 @@
package org.elasticsearch.index.query;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermRangeQuery;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper;
import java.io.IOException;
import java.util.Collection;
import java.util.Objects;
/**
* Constructs a query that only match on documents that the field has a value in them.
*/
public class ExistsQueryBuilder extends QueryBuilder {
public class ExistsQueryBuilder extends QueryBuilder<ExistsQueryBuilder> {
public static final String NAME = "exists";
private String name;
private final String name;
private String queryName;
@ -40,6 +53,13 @@ public class ExistsQueryBuilder extends QueryBuilder {
this.name = name;
}
/**
* @return the field name that has to exist for this query to match
*/
public String name() {
return this.name;
}
/**
* Sets the query name for the query that can be used when searching for matched_queries per hit.
*/
@ -48,6 +68,13 @@ public class ExistsQueryBuilder extends QueryBuilder {
return this;
}
/**
* @return the query name for the query that can be used when searching for matched_queries per hit.
*/
public String queryName() {
return this.queryName;
}
@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(NAME);
@ -58,6 +85,91 @@ public class ExistsQueryBuilder extends QueryBuilder {
builder.endObject();
}
@Override
public Query toQuery(QueryParseContext parseContext) throws QueryParsingException, IOException {
return newFilter(parseContext, name, queryName);
}
public static Query newFilter(QueryParseContext parseContext, String fieldPattern, String queryName) {
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType)parseContext.mapperService().fullName(FieldNamesFieldMapper.NAME);
if (fieldNamesFieldType == null) {
// can only happen when no types exist, so no docs exist either
return Queries.newMatchNoDocsQuery();
}
MapperService.SmartNameObjectMapper smartNameObjectMapper = parseContext.smartObjectMapper(fieldPattern);
if (smartNameObjectMapper != null && smartNameObjectMapper.hasMapper()) {
// automatic make the object mapper pattern
fieldPattern = fieldPattern + ".*";
}
Collection<String> fields = parseContext.simpleMatchToIndexNames(fieldPattern);
if (fields.isEmpty()) {
// no fields exists, so we should not match anything
return Queries.newMatchNoDocsQuery();
}
BooleanQuery boolFilter = new BooleanQuery();
for (String field : fields) {
MappedFieldType fieldType = parseContext.fieldMapper(field);
Query filter = null;
if (fieldNamesFieldType.isEnabled()) {
final String f;
if (fieldType != null) {
f = fieldType.names().indexName();
} else {
f = field;
}
filter = fieldNamesFieldType.termQuery(f, parseContext);
}
// if _field_names are not indexed, we need to go the slow way
if (filter == null && fieldType != null) {
filter = fieldType.rangeQuery(null, null, true, true, parseContext);
}
if (filter == null) {
filter = new TermRangeQuery(field, null, null, true, true);
}
boolFilter.add(filter, BooleanClause.Occur.SHOULD);
}
if (queryName != null) {
parseContext.addNamedQuery(queryName, boolFilter);
}
return new ConstantScoreQuery(boolFilter);
}
@Override
public int hashCode() {
return Objects.hash(name, queryName);
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null || getClass() != obj.getClass()) {
return false;
}
ExistsQueryBuilder other = (ExistsQueryBuilder) obj;
return Objects.equals(name, other.name) &&
Objects.equals(queryName, other.queryName);
}
@Override
public ExistsQueryBuilder readFrom(StreamInput in) throws IOException {
ExistsQueryBuilder newQuery = new ExistsQueryBuilder(in.readString());
newQuery.queryName = in.readOptionalString();
return newQuery;
}
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(name);
out.writeOptionalString(queryName);
}
@Override
public String queryId() {
return NAME;

View File

@ -19,22 +19,15 @@
package org.elasticsearch.index.query;
import org.apache.lucene.search.*;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper;
import java.io.IOException;
import java.util.Collection;
/**
*
*/
public class ExistsQueryParser extends BaseQueryParserTemp {
public class ExistsQueryParser extends BaseQueryParser {
@Inject
public ExistsQueryParser() {
@ -46,7 +39,7 @@ public class ExistsQueryParser extends BaseQueryParserTemp {
}
@Override
public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
XContentParser parser = parseContext.parser();
String fieldPattern = null;
@ -72,60 +65,14 @@ public class ExistsQueryParser extends BaseQueryParserTemp {
throw new QueryParsingException(parseContext, "exists must be provided with a [field]");
}
return newFilter(parseContext, fieldPattern, queryName);
}
public static Query newFilter(QueryParseContext parseContext, String fieldPattern, String queryName) {
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType)parseContext.mapperService().fullName(FieldNamesFieldMapper.NAME);
if (fieldNamesFieldType == null) {
// can only happen when no types exist, so no docs exist either
return Queries.newMatchNoDocsQuery();
}
MapperService.SmartNameObjectMapper smartNameObjectMapper = parseContext.smartObjectMapper(fieldPattern);
if (smartNameObjectMapper != null && smartNameObjectMapper.hasMapper()) {
// automatic make the object mapper pattern
fieldPattern = fieldPattern + ".*";
}
Collection<String> fields = parseContext.simpleMatchToIndexNames(fieldPattern);
if (fields.isEmpty()) {
// no fields exists, so we should not match anything
return Queries.newMatchNoDocsQuery();
}
BooleanQuery boolFilter = new BooleanQuery();
for (String field : fields) {
MappedFieldType fieldType = parseContext.fieldMapper(field);
Query filter = null;
if (fieldNamesFieldType.isEnabled()) {
final String f;
if (fieldType != null) {
f = fieldType.names().indexName();
} else {
f = field;
}
filter = fieldNamesFieldType.termQuery(f, parseContext);
}
// if _field_names are not indexed, we need to go the slow way
if (filter == null && fieldType != null) {
filter = fieldType.rangeQuery(null, null, true, true, parseContext);
}
if (filter == null) {
filter = new TermRangeQuery(field, null, null, true, true);
}
boolFilter.add(filter, BooleanClause.Occur.SHOULD);
}
if (queryName != null) {
parseContext.addNamedQuery(queryName, boolFilter);
}
return new ConstantScoreQuery(boolFilter);
ExistsQueryBuilder builder = new ExistsQueryBuilder(fieldPattern);
builder.queryName(queryName);
builder.validate();
return builder;
}
@Override
public ExistsQueryBuilder getBuilderPrototype() {
return ExistsQueryBuilder.PROTOTYPE;
}
}

View File

@ -158,7 +158,7 @@ public class GeoShapeQueryParser extends BaseQueryParserTemp {
// this strategy doesn't support disjoint anymore: but it did before, including creating lucene fieldcache (!)
// in this case, execute disjoint as exists && !intersects
BooleanQuery bool = new BooleanQuery();
Query exists = ExistsQueryParser.newFilter(parseContext, fieldName, null);
Query exists = ExistsQueryBuilder.newFilter(parseContext, fieldName, null);
Filter intersects = strategy.makeFilter(getArgs(shape, ShapeRelation.INTERSECTS));
bool.add(exists, BooleanClause.Occur.MUST);
bool.add(intersects, BooleanClause.Occur.MUST_NOT);

View File

@ -57,6 +57,7 @@ import org.elasticsearch.script.ScriptModule;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.test.TestSearchContext;
import org.elasticsearch.test.VersionUtils;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.threadpool.ThreadPoolModule;
import org.junit.After;
@ -75,11 +76,14 @@ import static org.hamcrest.Matchers.is;
@Ignore
public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> extends ElasticsearchTestCase {
protected static final String OBJECT_FIELD_NAME = "object";
protected static final String DATE_FIELD_NAME = "age";
protected static final String INT_FIELD_NAME = "price";
protected static final String STRING_FIELD_NAME = "text";
protected static final String DOUBLE_FIELD_NAME = "double";
protected static final String BOOLEAN_FIELD_NAME = "boolean";
protected static final String[] mappedFieldNames = new String[] { DATE_FIELD_NAME, INT_FIELD_NAME, STRING_FIELD_NAME,
DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, OBJECT_FIELD_NAME };
private static Injector injector;
private static IndexQueryParserService queryParserService;
@ -102,7 +106,8 @@ public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> extends Ela
Settings settings = Settings.settingsBuilder()
.put("name", BaseQueryTestCase.class.toString())
.put("path.home", createTempDir())
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetaData.SETTING_VERSION_CREATED, VersionUtils.randomVersionBetween(random(),
Version.V_1_0_0, Version.CURRENT))
.build();
index = new Index("test");
@ -138,7 +143,10 @@ public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> extends Ela
INT_FIELD_NAME, "type=integer",
DOUBLE_FIELD_NAME, "type=double",
BOOLEAN_FIELD_NAME, "type=boolean",
STRING_FIELD_NAME, "type=string").string()), false);
STRING_FIELD_NAME, "type=string",
OBJECT_FIELD_NAME, "type=object",
OBJECT_FIELD_NAME+"."+DATE_FIELD_NAME, "type=date",
OBJECT_FIELD_NAME+"."+INT_FIELD_NAME, "type=integer").string()), false);
currentTypes[i] = type;
}
namedWriteableRegistry = injector.getInstance(NamedWriteableRegistry.class);
@ -165,7 +173,7 @@ public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> extends Ela
}
} else {
if (randomBoolean()) {
types = new String[]{MetaData.ALL};
types = new String[] { MetaData.ALL };
} else {
types = new String[0];
}
@ -192,11 +200,6 @@ public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> extends Ela
*/
protected abstract QB createTestQueryBuilder();
/**
* Creates an empty builder of the type of query under test
*/
protected abstract QB createEmptyQueryBuilder();
/**
* Generic test that creates new query from the test query and checks both for equality
* and asserts equality on the two queries.
@ -259,7 +262,8 @@ public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> extends Ela
try (BytesStreamOutput output = new BytesStreamOutput()) {
testQuery.writeTo(output);
try (StreamInput in = new FilterStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
QB deserializedQuery = createEmptyQueryBuilder().readFrom(in);
QueryBuilder prototype = queryParserService.queryParser(testQuery.queryId()).getBuilderPrototype();
QueryBuilder deserializedQuery = prototype.readFrom(in);
assertEquals(deserializedQuery, testQuery);
assertEquals(deserializedQuery.hashCode(), testQuery.hashCode());
assertNotSame(deserializedQuery, testQuery);

View File

@ -83,11 +83,6 @@ public class BoolQueryBuilderTest extends BaseQueryTestCase<BoolQueryBuilder> {
return query;
}
@Override
protected BoolQueryBuilder createEmptyQueryBuilder() {
return new BoolQueryBuilder();
}
@Override
protected Query createExpectedQuery(BoolQueryBuilder queryBuilder, QueryParseContext context) throws IOException {
if (!queryBuilder.hasClauses()) {

View File

@ -0,0 +1,112 @@
/*
* 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.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermRangeQuery;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper;
import java.io.IOException;
import java.util.Collection;
import static org.hamcrest.Matchers.equalTo;
public class ExistsQueryBuilderTest extends BaseQueryTestCase<ExistsQueryBuilder> {
private Collection<String> getFieldNamePattern(String fieldName, QueryParseContext context) {
if (getCurrentTypes().length > 0 && fieldName.equals(BaseQueryTestCase.OBJECT_FIELD_NAME)) {
// "object" field has two inner fields (age, price), so if query hits that field, we
// extend field name with wildcard to match both nested fields. This is similar to what
// is done internally in ExistsQueryBuilder.toQuery()
fieldName = fieldName + ".*";
}
return context.simpleMatchToIndexNames(fieldName);
}
@Override
protected Query createExpectedQuery(ExistsQueryBuilder queryBuilder, QueryParseContext context) throws IOException {
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType)context.mapperService().fullName(FieldNamesFieldMapper.NAME);
Collection<String> fields = getFieldNamePattern(queryBuilder.name(), context);
if (fields.isEmpty() || fieldNamesFieldType == null) {
return Queries.newMatchNoDocsQuery();
}
BooleanQuery boolFilter = new BooleanQuery();
for (String field : fields) {
if (fieldNamesFieldType != null && fieldNamesFieldType.isEnabled()) {
boolFilter.add(fieldNamesFieldType.termQuery(field, context), BooleanClause.Occur.SHOULD);
} else {
MappedFieldType fieldType = context.fieldMapper(field);
if (fieldType == null) {
boolFilter.add(new TermRangeQuery(field, null, null, true, true), BooleanClause.Occur.SHOULD);
} else {
boolFilter.add(fieldType.rangeQuery(null, null, true, true, context), BooleanClause.Occur.SHOULD);
}
}
}
return new ConstantScoreQuery(boolFilter);
}
@Override
protected void assertLuceneQuery(ExistsQueryBuilder queryBuilder, Query query, QueryParseContext context) {
if (queryBuilder.queryName() != null) {
Query namedQuery = context.copyNamedFilters().get(queryBuilder.queryName());
Collection<String> fields = getFieldNamePattern(queryBuilder.name(), context);
if (fields.isEmpty()) {
assertNull(namedQuery);
} else {
query = ((ConstantScoreQuery) query).getQuery();
assertThat(namedQuery, equalTo(query));
}
}
}
@Override
protected ExistsQueryBuilder createTestQueryBuilder() {
String fieldPattern;
if (randomBoolean()) {
fieldPattern = randomFrom(mappedFieldNames);
} else {
fieldPattern = randomAsciiOfLengthBetween(1, 10);
}
// also sometimes test wildcard patterns
if (randomBoolean()) {
if (randomBoolean()) {
fieldPattern = fieldPattern + "*";
} else {
fieldPattern = MetaData.ALL;
}
}
ExistsQueryBuilder query = new ExistsQueryBuilder(fieldPattern);
if (randomBoolean()) {
query.queryName(randomAsciiOfLengthBetween(1, 10));
}
return query;
}
}

View File

@ -52,11 +52,6 @@ public class IdsQueryBuilderTest extends BaseQueryTestCase<IdsQueryBuilder> {
context.indexQueryParserService().queryParser("ids").fromXContent(context);
}
@Override
protected IdsQueryBuilder createEmptyQueryBuilder() {
return new IdsQueryBuilder();
}
@Override
protected Query createExpectedQuery(IdsQueryBuilder queryBuilder, QueryParseContext context) throws IOException {
Query expectedQuery;

View File

@ -30,11 +30,6 @@ public class LimitQueryBuilderTest extends BaseQueryTestCase<LimitQueryBuilder>
return Queries.newMatchAllQuery();
}
@Override
protected LimitQueryBuilder createEmptyQueryBuilder() {
return new LimitQueryBuilder(0);
}
/**
* @return a LimitQueryBuilder with random limit between 0 and 20
*/

View File

@ -31,11 +31,6 @@ public class MatchAllQueryBuilderTest extends BaseQueryTestCase<MatchAllQueryBui
return matchAllDocsQuery;
}
@Override
protected MatchAllQueryBuilder createEmptyQueryBuilder() {
return new MatchAllQueryBuilder();
}
/**
* @return a MatchAllQuery with random boost between 0.1f and 2.0f
*/

View File

@ -154,9 +154,4 @@ public class RangeQueryBuilderTest extends BaseQueryTestCase<RangeQueryBuilder>
query.from(1).to(10).timeZone("UTC");
query.toQuery(createContext());
}
@Override
protected RangeQueryBuilder createEmptyQueryBuilder() {
return new RangeQueryBuilder(null);
}
}

View File

@ -25,11 +25,6 @@ import org.apache.lucene.search.spans.SpanTermQuery;
public class SpanTermQueryBuilderTest extends BaseTermQueryTestCase<SpanTermQueryBuilder> {
@Override
protected SpanTermQueryBuilder createEmptyQueryBuilder() {
return new SpanTermQueryBuilder(null, null);
}
@Override
protected SpanTermQueryBuilder createQueryBuilder(String fieldName, Object value) {
return new SpanTermQueryBuilder(fieldName, value);

View File

@ -25,11 +25,6 @@ import org.apache.lucene.search.TermQuery;
public class TermQueryBuilderTest extends BaseTermQueryTestCase<TermQueryBuilder> {
@Override
protected TermQueryBuilder createEmptyQueryBuilder() {
return new TermQueryBuilder(null, null);
}
/**
* @return a TermQuery with random field name and value, optional random boost and queryname
*/