Skip to content

Commit c928852

Browse files
cbueschercbuescher
committed
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
1 parent 4f4b1b7 commit c928852

13 files changed

+248
-108
lines changed

‎core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
import org.apache.lucene.search.ConstantScoreQuery;
2323
import org.apache.lucene.search.Query;
24-
import org.elasticsearch.index.query.ExistsQueryParser;
24+
import org.elasticsearch.index.query.ExistsQueryBuilder;
2525
import org.elasticsearch.index.query.QueryParseContext;
2626

2727
/**
@@ -33,6 +33,6 @@ public class ExistsFieldQueryExtension implements FieldQueryExtension {
3333

3434
@Override
3535
public Query query(QueryParseContext parseContext, String queryText) {
36-
return new ConstantScoreQuery(ExistsQueryParser.newFilter(parseContext, queryText, null));
36+
return new ConstantScoreQuery(ExistsQueryBuilder.newFilter(parseContext, queryText, null));
3737
}
3838
}

‎core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java

Lines changed: 114 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,31 @@
1919

2020
package org.elasticsearch.index.query;
2121

22+
import org.apache.lucene.search.BooleanClause;
23+
import org.apache.lucene.search.BooleanQuery;
24+
import org.apache.lucene.search.ConstantScoreQuery;
25+
import org.apache.lucene.search.Query;
26+
import org.apache.lucene.search.TermRangeQuery;
27+
import org.elasticsearch.common.io.stream.StreamInput;
28+
import org.elasticsearch.common.io.stream.StreamOutput;
29+
import org.elasticsearch.common.lucene.search.Queries;
2230
import org.elasticsearch.common.xcontent.XContentBuilder;
31+
import org.elasticsearch.index.mapper.MappedFieldType;
32+
import org.elasticsearch.index.mapper.MapperService;
33+
import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper;
2334

2435
import java.io.IOException;
36+
import java.util.Collection;
37+
import java.util.Objects;
2538

2639
/**
2740
* Constructs a query that only match on documents that the field has a value in them.
2841
*/
29-
public class ExistsQueryBuilder extends QueryBuilder {
42+
public class ExistsQueryBuilder extends QueryBuilder<ExistsQueryBuilder> {
3043

3144
public static final String NAME = "exists";
3245

33-
private String name;
46+
private final String name;
3447

3548
private String queryName;
3649

@@ -40,6 +53,13 @@ public ExistsQueryBuilder(String name) {
4053
this.name = name;
4154
}
4255

56+
/**
57+
* @return the field name that has to exist for this query to match
58+
*/
59+
public String name() {
60+
return this.name;
61+
}
62+
4363
/**
4464
* Sets the query name for the query that can be used when searching for matched_queries per hit.
4565
*/
@@ -48,6 +68,13 @@ public ExistsQueryBuilder queryName(String queryName) {
4868
return this;
4969
}
5070

71+
/**
72+
* @return the query name for the query that can be used when searching for matched_queries per hit.
73+
*/
74+
public String queryName() {
75+
return this.queryName;
76+
}
77+
5178
@Override
5279
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
5380
builder.startObject(NAME);
@@ -58,6 +85,91 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
5885
builder.endObject();
5986
}
6087

88+
89+
@Override
90+
public Query toQuery(QueryParseContext parseContext) throws QueryParsingException, IOException {
91+
return newFilter(parseContext, name, queryName);
92+
}
93+
94+
public static Query newFilter(QueryParseContext parseContext, String fieldPattern, String queryName) {
95+
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType)parseContext.mapperService().fullName(FieldNamesFieldMapper.NAME);
96+
if (fieldNamesFieldType == null) {
97+
// can only happen when no types exist, so no docs exist either
98+
return Queries.newMatchNoDocsQuery();
99+
}
100+
101+
MapperService.SmartNameObjectMapper smartNameObjectMapper = parseContext.smartObjectMapper(fieldPattern);
102+
if (smartNameObjectMapper != null && smartNameObjectMapper.hasMapper()) {
103+
// automatic make the object mapper pattern
104+
fieldPattern = fieldPattern + ".*";
105+
}
106+
107+
Collection<String> fields = parseContext.simpleMatchToIndexNames(fieldPattern);
108+
if (fields.isEmpty()) {
109+
// no fields exists, so we should not match anything
110+
return Queries.newMatchNoDocsQuery();
111+
}
112+
113+
BooleanQuery boolFilter = new BooleanQuery();
114+
for (String field : fields) {
115+
MappedFieldType fieldType = parseContext.fieldMapper(field);
116+
Query filter = null;
117+
if (fieldNamesFieldType.isEnabled()) {
118+
final String f;
119+
if (fieldType != null) {
120+
f = fieldType.names().indexName();
121+
} else {
122+
f = field;
123+
}
124+
filter = fieldNamesFieldType.termQuery(f, parseContext);
125+
}
126+
// if _field_names are not indexed, we need to go the slow way
127+
if (filter == null && fieldType != null) {
128+
filter = fieldType.rangeQuery(null, null, true, true, parseContext);
129+
}
130+
if (filter == null) {
131+
filter = new TermRangeQuery(field, null, null, true, true);
132+
}
133+
boolFilter.add(filter, BooleanClause.Occur.SHOULD);
134+
}
135+
136+
if (queryName != null) {
137+
parseContext.addNamedQuery(queryName, boolFilter);
138+
}
139+
return new ConstantScoreQuery(boolFilter);
140+
}
141+
142+
@Override
143+
public int hashCode() {
144+
return Objects.hash(name, queryName);
145+
}
146+
147+
@Override
148+
public boolean equals(Object obj) {
149+
if (this == obj) {
150+
return true;
151+
}
152+
if (obj == null || getClass() != obj.getClass()) {
153+
return false;
154+
}
155+
ExistsQueryBuilder other = (ExistsQueryBuilder) obj;
156+
return Objects.equals(name, other.name) &&
157+
Objects.equals(queryName, other.queryName);
158+
}
159+
160+
@Override
161+
public ExistsQueryBuilder readFrom(StreamInput in) throws IOException {
162+
ExistsQueryBuilder newQuery = new ExistsQueryBuilder(in.readString());
163+
newQuery.queryName = in.readOptionalString();
164+
return newQuery;
165+
}
166+
167+
@Override
168+
public void writeTo(StreamOutput out) throws IOException {
169+
out.writeString(name);
170+
out.writeOptionalString(queryName);
171+
}
172+
61173
@Override
62174
public String queryId() {
63175
return NAME;

‎core/src/main/java/org/elasticsearch/index/query/ExistsQueryParser.java

Lines changed: 6 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,15 @@
1919

2020
package org.elasticsearch.index.query;
2121

22-
import org.apache.lucene.search.*;
2322
import org.elasticsearch.common.inject.Inject;
24-
import org.elasticsearch.common.lucene.search.Queries;
2523
import org.elasticsearch.common.xcontent.XContentParser;
26-
import org.elasticsearch.index.mapper.FieldMapper;
27-
import org.elasticsearch.index.mapper.MappedFieldType;
28-
import org.elasticsearch.index.mapper.MapperService;
29-
import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper;
3024

3125
import java.io.IOException;
32-
import java.util.Collection;
3326

3427
/**
3528
*
3629
*/
37-
public class ExistsQueryParser extends BaseQueryParserTemp {
30+
public class ExistsQueryParser extends BaseQueryParser {
3831

3932
@Inject
4033
public ExistsQueryParser() {
@@ -46,7 +39,7 @@ public String[] names() {
4639
}
4740

4841
@Override
49-
public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
42+
public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
5043
XContentParser parser = parseContext.parser();
5144

5245
String fieldPattern = null;
@@ -72,60 +65,14 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
7265
throw new QueryParsingException(parseContext, "exists must be provided with a [field]");
7366
}
7467

75-
return newFilter(parseContext, fieldPattern, queryName);
76-
}
77-
78-
public static Query newFilter(QueryParseContext parseContext, String fieldPattern, String queryName) {
79-
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType)parseContext.mapperService().fullName(FieldNamesFieldMapper.NAME);
80-
if (fieldNamesFieldType == null) {
81-
// can only happen when no types exist, so no docs exist either
82-
return Queries.newMatchNoDocsQuery();
83-
}
84-
85-
MapperService.SmartNameObjectMapper smartNameObjectMapper = parseContext.smartObjectMapper(fieldPattern);
86-
if (smartNameObjectMapper != null && smartNameObjectMapper.hasMapper()) {
87-
// automatic make the object mapper pattern
88-
fieldPattern = fieldPattern + ".*";
89-
}
90-
91-
Collection<String> fields = parseContext.simpleMatchToIndexNames(fieldPattern);
92-
if (fields.isEmpty()) {
93-
// no fields exists, so we should not match anything
94-
return Queries.newMatchNoDocsQuery();
95-
}
96-
97-
BooleanQuery boolFilter = new BooleanQuery();
98-
for (String field : fields) {
99-
MappedFieldType fieldType = parseContext.fieldMapper(field);
100-
Query filter = null;
101-
if (fieldNamesFieldType.isEnabled()) {
102-
final String f;
103-
if (fieldType != null) {
104-
f = fieldType.names().indexName();
105-
} else {
106-
f = field;
107-
}
108-
filter = fieldNamesFieldType.termQuery(f, parseContext);
109-
}
110-
// if _field_names are not indexed, we need to go the slow way
111-
if (filter == null && fieldType != null) {
112-
filter = fieldType.rangeQuery(null, null, true, true, parseContext);
113-
}
114-
if (filter == null) {
115-
filter = new TermRangeQuery(field, null, null, true, true);
116-
}
117-
boolFilter.add(filter, BooleanClause.Occur.SHOULD);
118-
}
119-
120-
if (queryName != null) {
121-
parseContext.addNamedQuery(queryName, boolFilter);
122-
}
123-
return new ConstantScoreQuery(boolFilter);
68+
ExistsQueryBuilder builder = new ExistsQueryBuilder(fieldPattern);
69+
builder.queryName(queryName);
70+
builder.validate();
71+
return builder;
12472
}
12573

12674
@Override
12775
public ExistsQueryBuilder getBuilderPrototype() {
12876
return ExistsQueryBuilder.PROTOTYPE;
12977
}
130-
13178
}

‎core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
158158
// this strategy doesn't support disjoint anymore: but it did before, including creating lucene fieldcache (!)
159159
// in this case, execute disjoint as exists && !intersects
160160
BooleanQuery bool = new BooleanQuery();
161-
Query exists = ExistsQueryParser.newFilter(parseContext, fieldName, null);
161+
Query exists = ExistsQueryBuilder.newFilter(parseContext, fieldName, null);
162162
Filter intersects = strategy.makeFilter(getArgs(shape, ShapeRelation.INTERSECTS));
163163
bool.add(exists, BooleanClause.Occur.MUST);
164164
bool.add(intersects, BooleanClause.Occur.MUST_NOT);

‎core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.elasticsearch.search.internal.SearchContext;
5858
import org.elasticsearch.test.ElasticsearchTestCase;
5959
import org.elasticsearch.test.TestSearchContext;
60+
import org.elasticsearch.test.VersionUtils;
6061
import org.elasticsearch.threadpool.ThreadPool;
6162
import org.elasticsearch.threadpool.ThreadPoolModule;
6263
import org.junit.After;
@@ -75,11 +76,14 @@
7576
@Ignore
7677
public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> extends ElasticsearchTestCase {
7778

79+
protected static final String OBJECT_FIELD_NAME = "object";
7880
protected static final String DATE_FIELD_NAME = "age";
7981
protected static final String INT_FIELD_NAME = "price";
8082
protected static final String STRING_FIELD_NAME = "text";
8183
protected static final String DOUBLE_FIELD_NAME = "double";
8284
protected static final String BOOLEAN_FIELD_NAME = "boolean";
85+
protected static final String[] mappedFieldNames = new String[] { DATE_FIELD_NAME, INT_FIELD_NAME, STRING_FIELD_NAME,
86+
DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, OBJECT_FIELD_NAME };
8387

8488
private static Injector injector;
8589
private static IndexQueryParserService queryParserService;
@@ -102,7 +106,8 @@ public static void init() throws IOException {
102106
Settings settings = Settings.settingsBuilder()
103107
.put("name", BaseQueryTestCase.class.toString())
104108
.put("path.home", createTempDir())
105-
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
109+
.put(IndexMetaData.SETTING_VERSION_CREATED, VersionUtils.randomVersionBetween(random(),
110+
Version.V_1_0_0, Version.CURRENT))
106111
.build();
107112

108113
index = new Index("test");
@@ -138,7 +143,10 @@ protected void configure() {
138143
INT_FIELD_NAME, "type=integer",
139144
DOUBLE_FIELD_NAME, "type=double",
140145
BOOLEAN_FIELD_NAME, "type=boolean",
141-
STRING_FIELD_NAME, "type=string").string()), false);
146+
STRING_FIELD_NAME, "type=string",
147+
OBJECT_FIELD_NAME, "type=object",
148+
OBJECT_FIELD_NAME+"."+DATE_FIELD_NAME, "type=date",
149+
OBJECT_FIELD_NAME+"."+INT_FIELD_NAME, "type=integer").string()), false);
142150
currentTypes[i] = type;
143151
}
144152
namedWriteableRegistry = injector.getInstance(NamedWriteableRegistry.class);
@@ -165,7 +173,7 @@ public void beforeTest() {
165173
}
166174
} else {
167175
if (randomBoolean()) {
168-
types = new String[]{MetaData.ALL};
176+
types = new String[] { MetaData.ALL };
169177
} else {
170178
types = new String[0];
171179
}
@@ -192,11 +200,6 @@ public void afterTest() {
192200
*/
193201
protected abstract QB createTestQueryBuilder();
194202

195-
/**
196-
* Creates an empty builder of the type of query under test
197-
*/
198-
protected abstract QB createEmptyQueryBuilder();
199-
200203
/**
201204
* Generic test that creates new query from the test query and checks both for equality
202205
* and asserts equality on the two queries.
@@ -259,7 +262,8 @@ public void testSerialization() throws IOException {
259262
try (BytesStreamOutput output = new BytesStreamOutput()) {
260263
testQuery.writeTo(output);
261264
try (StreamInput in = new FilterStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
262-
QB deserializedQuery = createEmptyQueryBuilder().readFrom(in);
265+
QueryBuilder prototype = queryParserService.queryParser(testQuery.queryId()).getBuilderPrototype();
266+
QueryBuilder deserializedQuery = prototype.readFrom(in);
263267
assertEquals(deserializedQuery, testQuery);
264268
assertEquals(deserializedQuery.hashCode(), testQuery.hashCode());
265269
assertNotSame(deserializedQuery, testQuery);

‎core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,6 @@ protected BoolQueryBuilder createTestQueryBuilder() {
8383
return query;
8484
}
8585

86-
@Override
87-
protected BoolQueryBuilder createEmptyQueryBuilder() {
88-
return new BoolQueryBuilder();
89-
}
90-
9186
@Override
9287
protected Query createExpectedQuery(BoolQueryBuilder queryBuilder, QueryParseContext context) throws IOException {
9388
if (!queryBuilder.hasClauses()) {

0 commit comments

Comments
 (0)