Skip to content

Commit

Permalink
Query refactoring: SpanMultiTermQueryBuilder and Parser
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cbuescher committed Jul 22, 2015
1 parent 37cdc13 commit 966c02b
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
* 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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -41,6 +61,42 @@ protected void doXContent(XContentBuilder builder, Params params)
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,7 +28,7 @@
/**
*
*/
public class SpanMultiTermQueryParser extends BaseQueryParserTemp {
public class SpanMultiTermQueryParser extends BaseQueryParser {

public static final String MATCH_NAME = "match";

Expand All @@ -45,7 +42,7 @@ public String[] names() {
}

@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();
Expand All @@ -58,13 +55,13 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -36,20 +37,37 @@ 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:
return new TermQueryBuilderTest().createTestQueryBuilder();
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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()));
}
}
}
}

0 comments on commit 966c02b

Please sign in to comment.