Query refactoring: SpanMultiTermQueryBuilder and Parser

Moving the query building functionality from the parser to the builders
new toQuery() method analogous to other recent query refactorings.

Relates to #10217
Closes #12182
This commit is contained in:
Christoph Büscher 2015-07-08 10:56:50 +02:00
parent 37cdc1344a
commit 966c02b1ac
6 changed files with 200 additions and 37 deletions

View File

@ -29,7 +29,7 @@ import java.io.IOException;
* Doesn't support conversion to {@link org.elasticsearch.common.xcontent.XContent} via {@link #doXContent(XContentBuilder, Params)}.
*/
//norelease to be removed once all queries support separate fromXContent and toQuery methods. Make AbstractQueryBuilder#toQuery final as well then.
public class QueryWrappingQueryBuilder extends AbstractQueryBuilder<QueryWrappingQueryBuilder> {
public class QueryWrappingQueryBuilder extends AbstractQueryBuilder<QueryWrappingQueryBuilder> implements MultiTermQueryBuilder<QueryWrappingQueryBuilder> {
private Query query;

View File

@ -18,18 +18,38 @@
*/
package org.elasticsearch.index.query;
import org.apache.lucene.search.MultiTermQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import java.io.IOException;
import java.util.Objects;
/**
* Query that allows wraping a {@link MultiTermQueryBuilder} (one of wildcard, fuzzy, prefix, term, range or regexp query)
* as a {@link SpanQueryBuilder} so it can be nested.
*/
public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder<SpanMultiTermQueryBuilder> implements SpanQueryBuilder<SpanMultiTermQueryBuilder> {
public static final String NAME = "span_multi";
private MultiTermQueryBuilder multiTermQueryBuilder;
static final SpanMultiTermQueryBuilder PROTOTYPE = new SpanMultiTermQueryBuilder(null);
private final MultiTermQueryBuilder multiTermQueryBuilder;
static final SpanMultiTermQueryBuilder PROTOTYPE = new SpanMultiTermQueryBuilder();
public SpanMultiTermQueryBuilder(MultiTermQueryBuilder multiTermQueryBuilder) {
this.multiTermQueryBuilder = multiTermQueryBuilder;
this.multiTermQueryBuilder = Objects.requireNonNull(multiTermQueryBuilder);
}
/**
* only used for prototype
*/
private SpanMultiTermQueryBuilder() {
this.multiTermQueryBuilder = null;
}
public MultiTermQueryBuilder multiTermQueryBuilder() {
return this.multiTermQueryBuilder;
}
@Override
@ -41,6 +61,42 @@ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder<SpanMultiTer
builder.endObject();
}
@Override
protected Query doToQuery(QueryParseContext parseContext) throws IOException {
Query subQuery = multiTermQueryBuilder.toQuery(parseContext);
if (subQuery instanceof MultiTermQuery == false) {
throw new UnsupportedOperationException("unsupported inner query, should be " + MultiTermQuery.class.getName() +" but was "
+ subQuery.getClass().getName());
}
return new SpanMultiTermQueryWrapper<>((MultiTermQuery) subQuery);
}
@Override
public QueryValidationException validate() {
return validateInnerQuery(multiTermQueryBuilder, null);
}
@Override
protected SpanMultiTermQueryBuilder doReadFrom(StreamInput in) throws IOException {
MultiTermQueryBuilder multiTermBuilder = in.readNamedWriteable();
return new SpanMultiTermQueryBuilder(multiTermBuilder);
}
@Override
protected void doWriteTo(StreamOutput out) throws IOException {
out.writeNamedWriteable(multiTermQueryBuilder);
}
@Override
protected int doHashCode() {
return Objects.hash(multiTermQueryBuilder);
}
@Override
protected boolean doEquals(SpanMultiTermQueryBuilder other) {
return Objects.equals(multiTermQueryBuilder, other.multiTermQueryBuilder);
}
@Override
public String getName() {
return NAME;

View File

@ -18,9 +18,6 @@
*/
package org.elasticsearch.index.query;
import org.apache.lucene.search.MultiTermQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.xcontent.XContentParser;
@ -31,7 +28,7 @@ import java.io.IOException;
/**
*
*/
public class SpanMultiTermQueryParser extends BaseQueryParserTemp {
public class SpanMultiTermQueryParser extends BaseQueryParser {
public static final String MATCH_NAME = "match";
@ -45,7 +42,7 @@ public class SpanMultiTermQueryParser extends BaseQueryParserTemp {
}
@Override
public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
XContentParser parser = parseContext.parser();
Token token = parser.nextToken();
@ -58,13 +55,13 @@ public class SpanMultiTermQueryParser extends BaseQueryParserTemp {
throw new QueryParsingException(parseContext, "spanMultiTerm must have [" + MATCH_NAME + "] multi term query clause");
}
Query subQuery = parseContext.parseInnerQuery();
if (!(subQuery instanceof MultiTermQuery)) {
QueryBuilder subQuery = parseContext.parseInnerQueryBuilder();
if (subQuery instanceof MultiTermQueryBuilder == false) {
throw new QueryParsingException(parseContext, "spanMultiTerm [" + MATCH_NAME + "] must be of type multi term query");
}
parser.nextToken();
return new SpanMultiTermQueryWrapper<>((MultiTermQuery) subQuery);
return new SpanMultiTermQueryBuilder((MultiTermQueryBuilder) subQuery);
}
@Override

View File

@ -20,6 +20,7 @@
package org.elasticsearch.index.query;
import com.carrotsearch.randomizedtesting.generators.RandomInts;
import com.carrotsearch.randomizedtesting.generators.RandomStrings;
import java.util.Random;
@ -36,7 +37,7 @@ public class RandomQueryBuilder {
* @return a random {@link QueryBuilder}
*/
public static QueryBuilder createQuery(Random r) {
switch (RandomInts.randomIntBetween(r, 0, 3)) {
switch (RandomInts.randomIntBetween(r, 0, 4)) {
case 0:
return new MatchAllQueryBuilderTest().createTestQueryBuilder();
case 1:
@ -44,12 +45,29 @@ public class RandomQueryBuilder {
case 2:
return new IdsQueryBuilderTest().createTestQueryBuilder();
case 3:
return createMultiTermQuery(r);
case 4:
return EmptyQueryBuilder.PROTOTYPE;
default:
throw new UnsupportedOperationException();
}
}
/**
* Create a new multi term query of a random type
* @param r random seed
* @return a random {@link MultiTermQueryBuilder}
*/
public static MultiTermQueryBuilder createMultiTermQuery(Random r) {
// for now, only use String Rangequeries for MultiTerm test, numeric and date makes little sense
// see issue #12123 for discussion
// Prefix / Fuzzy / RegEx / Wildcard can go here later once refactored and they have random query generators
RangeQueryBuilder query = new RangeQueryBuilder(BaseQueryTestCase.STRING_FIELD_NAME);
query.from("a" + RandomStrings.randomAsciiOfLengthBetween(r, 1, 10));
query.to("z" + RandomStrings.randomAsciiOfLengthBetween(r, 1, 10));
return query;
}
/**
* Create a new invalid query of a random type
* @param r random seed

View File

@ -45,32 +45,42 @@ public class RangeQueryBuilderTest extends BaseQueryTestCase<RangeQueryBuilder>
protected RangeQueryBuilder doCreateTestQueryBuilder() {
RangeQueryBuilder query;
// switch between numeric and date ranges
if (randomBoolean()) {
if (randomBoolean()) {
// use mapped integer field for numeric range queries
query = new RangeQueryBuilder(INT_FIELD_NAME);
query.from(randomIntBetween(1, 100));
query.to(randomIntBetween(101, 200));
} else {
// use unmapped field for numeric range queries
query = new RangeQueryBuilder(randomAsciiOfLengthBetween(1, 10));
query.from(0.0 - randomDouble());
query.to(randomDouble());
}
} else {
// use mapped date field, using date string representation
query = new RangeQueryBuilder(DATE_FIELD_NAME);
query.from(new DateTime(System.currentTimeMillis() - randomIntBetween(0, 1000000), DateTimeZone.UTC).toString());
query.to(new DateTime(System.currentTimeMillis() + randomIntBetween(0, 1000000), DateTimeZone.UTC).toString());
// Create timestamp option only then we have a date mapper, otherwise we could trigger exception.
if (createContext().mapperService().smartNameFieldType(DATE_FIELD_NAME) != null) {
switch (randomIntBetween(0, 2)) {
case 0:
if (randomBoolean()) {
query.timeZone(TIMEZONE_IDS.get(randomIntBetween(0, TIMEZONE_IDS.size() - 1)));
// use mapped integer field for numeric range queries
query = new RangeQueryBuilder(INT_FIELD_NAME);
query.from(randomIntBetween(1, 100));
query.to(randomIntBetween(101, 200));
} else {
// use unmapped field for numeric range queries
query = new RangeQueryBuilder(randomAsciiOfLengthBetween(1, 10));
query.from(0.0 - randomDouble());
query.to(randomDouble());
}
if (randomBoolean()) {
query.format("yyyy-MM-dd'T'HH:mm:ss.SSSZZ");
break;
case 1:
// use mapped date field, using date string representation
query = new RangeQueryBuilder(DATE_FIELD_NAME);
query.from(new DateTime(System.currentTimeMillis() - randomIntBetween(0, 1000000), DateTimeZone.UTC).toString());
query.to(new DateTime(System.currentTimeMillis() + randomIntBetween(0, 1000000), DateTimeZone.UTC).toString());
// Create timestamp option only then we have a date mapper,
// otherwise we could trigger exception.
if (createContext().mapperService().smartNameFieldType(DATE_FIELD_NAME) != null) {
if (randomBoolean()) {
query.timeZone(TIMEZONE_IDS.get(randomIntBetween(0, TIMEZONE_IDS.size() - 1)));
}
if (randomBoolean()) {
query.format("yyyy-MM-dd'T'HH:mm:ss.SSSZZ");
}
}
}
break;
case 2:
default:
query = new RangeQueryBuilder(STRING_FIELD_NAME);
query.from("a" + randomAsciiOfLengthBetween(1, 10));
query.to("z" + randomAsciiOfLengthBetween(1, 10));
break;
}
query.includeLower(randomBoolean()).includeUpper(randomBoolean());
if (randomBoolean()) {

View File

@ -0,0 +1,82 @@
/*
* 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.MultiTermQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper;
import org.junit.Test;
import java.io.IOException;
public class SpanMultiTermQueryBuilderTest extends BaseQueryTestCase<SpanMultiTermQueryBuilder> {
@Override
protected Query doCreateExpectedQuery(SpanMultiTermQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException {
Query multiTermQuery = testQueryBuilder.multiTermQueryBuilder().toQuery(context);
return new SpanMultiTermQueryWrapper<>((MultiTermQuery) multiTermQuery);
}
@Override
protected SpanMultiTermQueryBuilder doCreateTestQueryBuilder() {
MultiTermQueryBuilder multiTermQueryBuilder = RandomQueryBuilder.createMultiTermQuery(random());
return new SpanMultiTermQueryBuilder(multiTermQueryBuilder);
}
@Test
public void testValidate() {
int totalExpectedErrors = 0;
MultiTermQueryBuilder multiTermQueryBuilder;
if (randomBoolean()) {
multiTermQueryBuilder = new RangeQueryBuilder("");
totalExpectedErrors++;
} else {
multiTermQueryBuilder = new RangeQueryBuilder("field");
}
SpanMultiTermQueryBuilder queryBuilder = new SpanMultiTermQueryBuilder(multiTermQueryBuilder);
assertValidate(queryBuilder, totalExpectedErrors);
}
@Test(expected = NullPointerException.class)
public void testInnerQueryNull() {
new SpanMultiTermQueryBuilder(null);
}
/**
* test checks that we throw an {@link UnsupportedOperationException} if the query wrapped
* by {@link SpanMultiTermQueryBuilder} does not generate a lucene {@link MultiTermQuery}.
* This is currently the case for {@link RangeQueryBuilder} when the target field is mapped
* to a date.
*/
@Test
public void testUnsupportedInnerQueryType() throws IOException {
QueryParseContext parseContext = createContext();
// test makes only sense if date field is mapped
if (parseContext.fieldMapper(DATE_FIELD_NAME) != null) {
try {
RangeQueryBuilder query = new RangeQueryBuilder(DATE_FIELD_NAME);
new SpanMultiTermQueryBuilder(query).toQuery(createContext());
fail("Exception expected, range query on date fields should not generate a lucene " + MultiTermQuery.class.getName());
} catch (UnsupportedOperationException e) {
assert(e.getMessage().contains("unsupported inner query, should be " + MultiTermQuery.class.getName()));
}
}
}
}