Query refactoring: SpanNotQueryBuilder and Parser

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

Relates to #10217
Closes #12365
This commit is contained in:
Christoph Büscher 2015-07-20 18:39:45 +02:00
parent 22dcc77062
commit 7f88cc596b
7 changed files with 387 additions and 86 deletions

View File

@ -316,8 +316,8 @@ public abstract class QueryBuilders {
return new SpanNearQueryBuilder(slop);
}
public static SpanNotQueryBuilder spanNotQuery() {
return new SpanNotQueryBuilder();
public static SpanNotQueryBuilder spanNotQuery(SpanQueryBuilder include, SpanQueryBuilder exclude) {
return new SpanNotQueryBuilder(include, exclude);
}
public static SpanOrQueryBuilder spanOrQuery() {

View File

@ -19,82 +19,172 @@
package org.elasticsearch.index.query;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanNotQuery;
import org.apache.lucene.search.spans.SpanQuery;
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;
public class SpanNotQueryBuilder extends AbstractQueryBuilder<SpanNotQueryBuilder> implements SpanQueryBuilder<SpanNotQueryBuilder> {
public static final String NAME = "span_not";
private SpanQueryBuilder include;
/** the default pre parameter size */
public static final int DEFAULT_PRE = 0;
/** the default post parameter size */
public static final int DEFAULT_POST = 0;
private SpanQueryBuilder exclude;
private final SpanQueryBuilder include;
private Integer dist;
private final SpanQueryBuilder exclude;
private Integer pre;
private int pre = DEFAULT_PRE;
private Integer post;
private int post = DEFAULT_POST;
static final SpanNotQueryBuilder PROTOTYPE = new SpanNotQueryBuilder();
public SpanNotQueryBuilder include(SpanQueryBuilder include) {
this.include = include;
return this;
/**
* Construct a span query matching spans from <code>include</code> which
* have no overlap with spans from <code>exclude</code>.
* @param include the span query whose matches are filtered
* @param exclude the span query whose matches must not overlap
*/
public SpanNotQueryBuilder(SpanQueryBuilder include, SpanQueryBuilder exclude) {
this.include = Objects.requireNonNull(include);
this.exclude = Objects.requireNonNull(exclude);
}
public SpanNotQueryBuilder exclude(SpanQueryBuilder exclude) {
this.exclude = exclude;
return this;
// only used for prototype
private SpanNotQueryBuilder() {
this.include = null;
this.exclude = null;
}
/**
* @return the span query whose matches are filtered
*/
public SpanQueryBuilder include() {
return this.include;
}
/**
* @return the span query whose matches must not overlap
*/
public SpanQueryBuilder exclude() {
return this.exclude;
}
/**
* @param dist the amount of tokens from within the include span cant have overlap with the exclude span.
* Equivalent to setting both pre and post parameter.
*/
public SpanNotQueryBuilder dist(int dist) {
this.dist = dist;
pre(dist);
post(dist);
return this;
}
/**
* @param pre the amount of tokens before the include span that cant have overlap with the exclude span. Values
* smaller than 0 will be ignored and 0 used instead.
*/
public SpanNotQueryBuilder pre(int pre) {
this.pre = (pre >=0) ? pre : 0;
this.pre = (pre >= 0) ? pre : 0;
return this;
}
/**
* @return the amount of tokens before the include span that cant have overlap with the exclude span.
* @see SpanNotQueryBuilder#pre(int)
*/
public Integer pre() {
return this.pre;
}
/**
* @param post the amount of tokens after the include span that cant have overlap with the exclude span.
*/
public SpanNotQueryBuilder post(int post) {
this.post = (post >= 0) ? post : 0;
return this;
}
/**
* @return the amount of tokens after the include span that cant have overlap with the exclude span.
* @see SpanNotQueryBuilder#post(int)
*/
public Integer post() {
return this.post;
}
@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
if (include == null) {
throw new IllegalArgumentException("Must specify include when using spanNot query");
}
if (exclude == null) {
throw new IllegalArgumentException("Must specify exclude when using spanNot query");
}
if (dist != null && (pre != null || post != null)) {
throw new IllegalArgumentException("spanNot can either use [dist] or [pre] & [post] (or none)");
}
builder.startObject(NAME);
builder.field("include");
include.toXContent(builder, params);
builder.field("exclude");
exclude.toXContent(builder, params);
if (dist != null) {
builder.field("dist", dist);
}
if (pre != null) {
builder.field("pre", pre);
}
if (post != null) {
builder.field("post", post);
}
builder.field("pre", pre);
builder.field("post", post);
printBoostAndQueryName(builder);
builder.endObject();
}
@Override
protected Query doToQuery(QueryParseContext parseContext) throws IOException {
Query includeQuery = this.include.toQuery(parseContext);
assert includeQuery instanceof SpanQuery;
Query excludeQuery = this.exclude.toQuery(parseContext);
assert excludeQuery instanceof SpanQuery;
SpanNotQuery query = new SpanNotQuery((SpanQuery) includeQuery, (SpanQuery) excludeQuery, pre, post);
return query;
}
@Override
public QueryValidationException validate() {
QueryValidationException validationExceptions = validateInnerQuery(include, null);
validationExceptions = validateInnerQuery(exclude, validationExceptions);
return validationExceptions;
}
@Override
protected SpanNotQueryBuilder doReadFrom(StreamInput in) throws IOException {
SpanQueryBuilder include = in.readNamedWriteable();
SpanQueryBuilder exclude = in.readNamedWriteable();
SpanNotQueryBuilder queryBuilder = new SpanNotQueryBuilder(include, exclude);
queryBuilder.pre(in.readVInt());
queryBuilder.post(in.readVInt());
return queryBuilder;
}
@Override
protected void doWriteTo(StreamOutput out) throws IOException {
out.writeNamedWriteable(include);
out.writeNamedWriteable(exclude);
out.writeVInt(pre);
out.writeVInt(post);
}
@Override
protected int doHashCode() {
return Objects.hash(include, exclude, pre, post);
}
@Override
protected boolean doEquals(SpanNotQueryBuilder other) {
return Objects.equals(include, other.include) &&
Objects.equals(exclude, other.exclude) &&
(pre == other.pre) &&
(post == other.post);
}
@Override
public String getName() {
return NAME;

View File

@ -19,9 +19,6 @@
package org.elasticsearch.index.query;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanNotQuery;
import org.apache.lucene.search.spans.SpanQuery;
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 SpanNotQueryParser extends BaseQueryParserTemp {
public class SpanNotQueryParser extends BaseQueryParser {
@Inject
public SpanNotQueryParser() {
@ -43,13 +40,13 @@ public class SpanNotQueryParser extends BaseQueryParserTemp {
}
@Override
public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
XContentParser parser = parseContext.parser();
float boost = AbstractQueryBuilder.DEFAULT_BOOST;
SpanQuery include = null;
SpanQuery exclude = null;
SpanQueryBuilder include = null;
SpanQueryBuilder exclude = null;
Integer dist = null;
Integer pre = null;
@ -64,17 +61,17 @@ public class SpanNotQueryParser extends BaseQueryParserTemp {
currentFieldName = parser.currentName();
} else if (token == XContentParser.Token.START_OBJECT) {
if ("include".equals(currentFieldName)) {
Query query = parseContext.parseInnerQuery();
if (!(query instanceof SpanQuery)) {
QueryBuilder query = parseContext.parseInnerQueryBuilder();
if (!(query instanceof SpanQueryBuilder)) {
throw new QueryParsingException(parseContext, "spanNot [include] must be of type span query");
}
include = (SpanQuery) query;
include = (SpanQueryBuilder) query;
} else if ("exclude".equals(currentFieldName)) {
Query query = parseContext.parseInnerQuery();
if (!(query instanceof SpanQuery)) {
QueryBuilder query = parseContext.parseInnerQueryBuilder();
if (!(query instanceof SpanQueryBuilder)) {
throw new QueryParsingException(parseContext, "spanNot [exclude] must be of type span query");
}
exclude = (SpanQuery) query;
exclude = (SpanQueryBuilder) query;
} else {
throw new QueryParsingException(parseContext, "[span_not] query does not support [" + currentFieldName + "]");
}
@ -104,27 +101,19 @@ public class SpanNotQueryParser extends BaseQueryParserTemp {
throw new QueryParsingException(parseContext, "spanNot can either use [dist] or [pre] & [post] (or none)");
}
// set appropriate defaults
if (pre != null && post == null) {
post = 0;
} else if (pre == null && post != null){
pre = 0;
SpanNotQueryBuilder spanNotQuery = new SpanNotQueryBuilder(include, exclude);
if (dist != null) {
spanNotQuery.dist(dist);
}
SpanNotQuery query;
if (pre != null && post != null) {
query = new SpanNotQuery(include, exclude, pre, post);
} else if (dist != null) {
query = new SpanNotQuery(include, exclude, dist);
} else {
query = new SpanNotQuery(include, exclude);
if (pre != null) {
spanNotQuery.pre(pre);
}
query.setBoost(boost);
if (queryName != null) {
parseContext.addNamedQuery(queryName, query);
if (post != null) {
spanNotQuery.post(post);
}
return query;
spanNotQuery.boost(boost);
spanNotQuery.queryName(queryName);
return spanNotQuery;
}
@Override

View File

@ -30,7 +30,6 @@ import org.apache.lucene.queries.ExtendedCommonTermsQuery;
import org.apache.lucene.queries.TermsQuery;
import org.apache.lucene.search.*;
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.MultiTermQuery.RewriteMethod;
import org.apache.lucene.search.join.ToParentBlockJoinQuery;
import org.apache.lucene.search.spans.*;
import org.apache.lucene.spatial.prefix.IntersectsPrefixTreeFilter;
@ -1323,7 +1322,7 @@ public class SimpleIndexQueryParserTests extends ElasticsearchSingleNodeTest {
@Test
public void testSpanNotQueryBuilder() throws IOException {
IndexQueryParserService queryParser = queryParser();
Query parsedQuery = queryParser.parse(spanNotQuery().include(spanTermQuery("age", 34)).exclude(spanTermQuery("age", 35))).query();
Query parsedQuery = queryParser.parse(spanNotQuery(spanTermQuery("age", 34), spanTermQuery("age", 35))).query();
assertThat(parsedQuery, instanceOf(SpanNotQuery.class));
SpanNotQuery spanNotQuery = (SpanNotQuery) parsedQuery;
// since age is automatically registered in data, we encode it as numeric

View File

@ -0,0 +1,230 @@
/*
* 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 org.apache.lucene.search.spans.SpanNotQuery;
import org.apache.lucene.search.spans.SpanQuery;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.junit.Test;
import java.io.IOException;
import static org.elasticsearch.index.query.QueryBuilders.spanNearQuery;
import static org.elasticsearch.index.query.QueryBuilders.spanTermQuery;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
public class SpanNotQueryBuilderTest extends BaseQueryTestCase<SpanNotQueryBuilder> {
@Override
protected Query doCreateExpectedQuery(SpanNotQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException {
SpanQuery include = (SpanQuery) testQueryBuilder.include().toQuery(context);
SpanQuery exclude = (SpanQuery) testQueryBuilder.exclude().toQuery(context);
return new SpanNotQuery(include, exclude, testQueryBuilder.pre(), testQueryBuilder.post());
}
@Override
protected SpanNotQueryBuilder doCreateTestQueryBuilder() {
SpanTermQueryBuilder includeQuery = new SpanTermQueryBuilderTest().createTestQueryBuilder();
// we need same field name and value type as bigQuery for little query
String fieldName = includeQuery.fieldName();
Object value;
switch (fieldName) {
case BOOLEAN_FIELD_NAME: value = randomBoolean(); break;
case INT_FIELD_NAME: value = randomInt(); break;
case DOUBLE_FIELD_NAME: value = randomDouble(); break;
case STRING_FIELD_NAME: value = randomAsciiOfLengthBetween(1, 10); break;
default : value = randomAsciiOfLengthBetween(1, 10);
}
SpanTermQueryBuilder excludeQuery = new SpanTermQueryBuilder(fieldName, value);
SpanNotQueryBuilder queryBuilder = new SpanNotQueryBuilder(includeQuery, excludeQuery);
if (randomBoolean()) {
// also test negative values, they should implicitly be changed to 0
queryBuilder.dist(randomIntBetween(-2, 10));
} else {
if (randomBoolean()) {
queryBuilder.pre(randomIntBetween(-2, 10));
}
if (randomBoolean()) {
queryBuilder.post(randomIntBetween(-2, 10));
}
}
return queryBuilder;
}
@Test
public void testValidate() {
int totalExpectedErrors = 0;
SpanQueryBuilder include;
if (randomBoolean()) {
include = new SpanTermQueryBuilder("", "test");
totalExpectedErrors++;
} else {
include = new SpanTermQueryBuilder("name", "value");
}
SpanQueryBuilder exclude;
if (randomBoolean()) {
exclude = new SpanTermQueryBuilder("", "test");
totalExpectedErrors++;
} else {
exclude = new SpanTermQueryBuilder("name", "value");
}
SpanNotQueryBuilder queryBuilder = new SpanNotQueryBuilder(include, exclude);
assertValidate(queryBuilder, totalExpectedErrors);
}
@Test
public void testDist() {
SpanNotQueryBuilder builder = new SpanNotQueryBuilder(new SpanTermQueryBuilder("name1", "value1"), new SpanTermQueryBuilder("name2", "value2"));
assertThat(builder.pre(), equalTo(0));
assertThat(builder.post(), equalTo(0));
builder.dist(-4);
assertThat(builder.pre(), equalTo(0));
assertThat(builder.post(), equalTo(0));
builder.dist(4);
assertThat(builder.pre(), equalTo(4));
assertThat(builder.post(), equalTo(4));
}
@Test
public void testPrePost() {
SpanNotQueryBuilder builder = new SpanNotQueryBuilder(new SpanTermQueryBuilder("name1", "value1"), new SpanTermQueryBuilder("name2", "value2"));
assertThat(builder.pre(), equalTo(0));
assertThat(builder.post(), equalTo(0));
builder.pre(-4).post(-4);
assertThat(builder.pre(), equalTo(0));
assertThat(builder.post(), equalTo(0));
builder.pre(1).post(2);
assertThat(builder.pre(), equalTo(1));
assertThat(builder.post(), equalTo(2));
}
/**
* test correct parsing of `dist` parameter, this should create builder with pre/post set to same value
*/
@Test
public void testParseDist() throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startObject();
builder.startObject(SpanNotQueryBuilder.NAME);
builder.field("exclude");
spanTermQuery("description", "jumped").toXContent(builder, null);
builder.field("include");
spanNearQuery(1).clause(QueryBuilders.spanTermQuery("description", "quick"))
.clause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
builder.field("dist", 3);
builder.endObject();
builder.endObject();
QueryParseContext context = createContext();
XContentParser parser = XContentFactory.xContent(builder.string()).createParser(builder.string());
context.reset(parser);
assertQueryHeader(parser, SpanNotQueryBuilder.NAME);
SpanNotQueryBuilder query = (SpanNotQueryBuilder) new SpanNotQueryParser().fromXContent(context);
assertThat(query.pre(), equalTo(3));
assertThat(query.post(), equalTo(3));
assertNotNull(query.include());
assertNotNull(query.exclude());
}
/**
* test exceptions for three types of broken json, missing include / exclude and both dist and pre/post specified
*/
@Test
public void testParserExceptions() throws IOException {
try {
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startObject();
builder.startObject(SpanNotQueryBuilder.NAME);
builder.field("exclude");
spanTermQuery("description", "jumped").toXContent(builder, null);
builder.field("dist", 2);
builder.endObject();
builder.endObject();
QueryParseContext context = createContext();
XContentParser parser = XContentFactory.xContent(builder.string()).createParser(builder.string());
context.reset(parser);
assertQueryHeader(parser, SpanNotQueryBuilder.NAME);
new SpanNotQueryParser().fromXContent(context);
fail("QueryParsingException should have been caught");
} catch (QueryParsingException e) {
assertThat("QueryParsingException should have been caught", e.getDetailedMessage(), containsString("spanNot must have [include]"));
}
try {
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startObject();
builder.startObject(SpanNotQueryBuilder.NAME);
builder.field("include");
spanNearQuery(1).clause(QueryBuilders.spanTermQuery("description", "quick"))
.clause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
builder.field("dist", 2);
builder.endObject();
builder.endObject();
QueryParseContext context = createContext();
XContentParser parser = XContentFactory.xContent(builder.string()).createParser(builder.string());
context.reset(parser);
assertQueryHeader(parser, SpanNotQueryBuilder.NAME);
new SpanNotQueryParser().fromXContent(context);
fail("QueryParsingException should have been caught");
} catch (QueryParsingException e) {
assertThat("QueryParsingException should have been caught", e.getDetailedMessage(), containsString("spanNot must have [exclude]"));
}
try {
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startObject();
builder.startObject(SpanNotQueryBuilder.NAME);
builder.field("include");
spanNearQuery(1).clause(QueryBuilders.spanTermQuery("description", "quick"))
.clause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
builder.field("exclude");
spanTermQuery("description", "jumped").toXContent(builder, null);
builder.field("dist", 2);
builder.field("pre", 2);
builder.endObject();
builder.endObject();
QueryParseContext context = createContext();
XContentParser parser = XContentFactory.xContent(builder.string()).createParser(builder.string());
context.reset(parser);
assertQueryHeader(parser, SpanNotQueryBuilder.NAME);
new SpanNotQueryParser().fromXContent(context);
fail("QueryParsingException should have been caught");
} catch (QueryParsingException e) {
assertThat("QueryParsingException should have been caught", e.getDetailedMessage(), containsString("spanNot can either use [dist] or [pre] & [post] (or none)"));
}
}
@Test(expected=NullPointerException.class)
public void testNullInclude() {
new SpanNotQueryBuilder(null, new SpanTermQueryBuilder("name", "value"));
}
@Test(expected=NullPointerException.class)
public void testNullExclude() {
new SpanNotQueryBuilder(new SpanTermQueryBuilder("name", "value"), null);
}
}

View File

@ -21,7 +21,6 @@ package org.elasticsearch.search.query;
import org.apache.lucene.util.English;
import org.apache.lucene.util.LuceneTestCase.Slow;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.index.IndexRequestBuilder;
@ -1623,33 +1622,22 @@ public class SearchQueryTests extends ElasticsearchIntegrationTest {
refresh();
SearchResponse searchResponse = client().prepareSearch("test")
.setQuery(spanNotQuery().include(spanNearQuery(1)
.setQuery(spanNotQuery(spanNearQuery(1)
.clause(QueryBuilders.spanTermQuery("description", "quick"))
.clause(QueryBuilders.spanTermQuery("description", "fox"))).exclude(spanTermQuery("description", "brown"))).get();
.clause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "brown"))).get();
assertHitCount(searchResponse, 1l);
searchResponse = client().prepareSearch("test")
.setQuery(spanNotQuery().include(spanNearQuery(1)
.setQuery(spanNotQuery(spanNearQuery(1)
.clause(QueryBuilders.spanTermQuery("description", "quick"))
.clause(QueryBuilders.spanTermQuery("description", "fox"))).exclude(spanTermQuery("description", "sleeping")).dist(5)).get();
.clause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "sleeping")).dist(5)).get();
assertHitCount(searchResponse, 1l);
searchResponse = client().prepareSearch("test")
.setQuery(spanNotQuery().include(spanNearQuery(1)
.setQuery(spanNotQuery(spanNearQuery(1)
.clause(QueryBuilders.spanTermQuery("description", "quick"))
.clause(QueryBuilders.spanTermQuery("description", "fox"))).exclude(spanTermQuery("description", "jumped")).pre(1).post(1)).get();
.clause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "jumped")).pre(1).post(1)).get();
assertHitCount(searchResponse, 1l);
try {
client().prepareSearch("test")
.setQuery(spanNotQuery().include(spanNearQuery(1)
.clause(QueryBuilders.spanTermQuery("description", "quick"))
.clause(QueryBuilders.spanTermQuery("description", "fox"))).exclude(spanTermQuery("description", "jumped")).dist(2).pre(2)
).get();
fail("ElasticsearchIllegalArgumentException should have been caught");
} catch (ElasticsearchException e) {
assertThat("ElasticsearchIllegalArgumentException should have been caught", e.getDetailedMessage(), containsString("spanNot can either use [dist] or [pre] & [post] (or none)"));
}
}
@Test

View File

@ -22,6 +22,11 @@ static factory methods in QueryBuilders accordingly.
Removed setter for mandatory slop parameter, needs to be set in constructor now.
Updated the static factory methods in QueryBuilders accordingly.
==== SpanNotQueryBuilder
Removed setter for mandatory include/exclude span query clause, needs to be set in constructor now.
Updated the static factory methods in QueryBuilders and tests accordingly.
==== QueryFilterBuilder
Removed the setter `queryName(String queryName)` since this field is not supported