Query refactoring: BoostingQueryBuilder and Parser
As part of the refactoring of queries this PR splits the parsers parse() method into a parsing and a query building part, adding serialization, hashCode() and equals() to the builder. Relates to #10217 Closes #11621
This commit is contained in:
parent
d25acdfad2
commit
620211800e
|
@ -19,9 +19,14 @@
|
|||
|
||||
package org.elasticsearch.index.query;
|
||||
|
||||
import org.apache.lucene.search.Query;
|
||||
import org.apache.lucene.queries.BoostingQuery;
|
||||
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;
|
||||
|
||||
/**
|
||||
* The BoostingQuery class can be used to effectively demote results that match a given query.
|
||||
|
@ -35,7 +40,7 @@ import java.io.IOException;
|
|||
* multiplied by the supplied "boost" parameter, so this should be less than 1 to achieve a
|
||||
* demoting effect
|
||||
*/
|
||||
public class BoostingQueryBuilder extends QueryBuilder implements BoostableQueryBuilder<BoostingQueryBuilder> {
|
||||
public class BoostingQueryBuilder extends QueryBuilder<BoostingQueryBuilder> implements BoostableQueryBuilder<BoostingQueryBuilder> {
|
||||
|
||||
public static final String NAME = "boosting";
|
||||
|
||||
|
@ -45,34 +50,74 @@ public class BoostingQueryBuilder extends QueryBuilder implements BoostableQuery
|
|||
|
||||
private float negativeBoost = -1;
|
||||
|
||||
private float boost = -1;
|
||||
private float boost = 1.0f;
|
||||
|
||||
static final BoostingQueryBuilder PROTOTYPE = new BoostingQueryBuilder();
|
||||
|
||||
public BoostingQueryBuilder() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Add the positive query for this boosting query.
|
||||
*/
|
||||
public BoostingQueryBuilder positive(QueryBuilder positiveQuery) {
|
||||
this.positiveQuery = positiveQuery;
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the positive query for this boosting query.
|
||||
*/
|
||||
public QueryBuilder positive() {
|
||||
return this.positiveQuery;
|
||||
}
|
||||
|
||||
/**
|
||||
* Add the negative query for this boosting query.
|
||||
*/
|
||||
public BoostingQueryBuilder negative(QueryBuilder negativeQuery) {
|
||||
this.negativeQuery = negativeQuery;
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the negative query for this boosting query.
|
||||
*/
|
||||
public QueryBuilder negative() {
|
||||
return this.negativeQuery;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the negative boost factor.
|
||||
*/
|
||||
public BoostingQueryBuilder negativeBoost(float negativeBoost) {
|
||||
this.negativeBoost = negativeBoost;
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the negative boost factor.
|
||||
*/
|
||||
public float negativeBoost() {
|
||||
return this.negativeBoost;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the boost factor.
|
||||
*/
|
||||
@Override
|
||||
public BoostingQueryBuilder boost(float boost) {
|
||||
this.boost = boost;
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the boost factor.
|
||||
*/
|
||||
public float boost() {
|
||||
return this.boost;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
|
||||
if (positiveQuery == null) {
|
||||
|
@ -81,25 +126,87 @@ public class BoostingQueryBuilder extends QueryBuilder implements BoostableQuery
|
|||
if (negativeQuery == null) {
|
||||
throw new IllegalArgumentException("boosting query requires negative query to be set");
|
||||
}
|
||||
if (negativeBoost == -1) {
|
||||
throw new IllegalArgumentException("boosting query requires negativeBoost to be set");
|
||||
}
|
||||
builder.startObject(NAME);
|
||||
builder.field("positive");
|
||||
positiveQuery.toXContent(builder, params);
|
||||
builder.field("negative");
|
||||
negativeQuery.toXContent(builder, params);
|
||||
|
||||
builder.field("negative_boost", negativeBoost);
|
||||
|
||||
if (boost != -1) {
|
||||
builder.field("boost", boost);
|
||||
}
|
||||
builder.endObject();
|
||||
}
|
||||
|
||||
@Override
|
||||
public QueryValidationException validate() {
|
||||
QueryValidationException validationException = null;
|
||||
if (negativeBoost < 0) {
|
||||
validationException = QueryValidationException
|
||||
.addValidationError("[boosting] query requires negativeBoost to be set to positive value", validationException);
|
||||
}
|
||||
return validationException;
|
||||
};
|
||||
|
||||
@Override
|
||||
public String queryId() {
|
||||
return NAME;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Query toQuery(QueryParseContext parseContext) throws QueryParsingException, IOException {
|
||||
|
||||
// make upstream queries ignore this query by returning `null`
|
||||
// if either inner query builder is null or returns null-Query
|
||||
if (positiveQuery == null || negativeQuery == null) {
|
||||
return null;
|
||||
}
|
||||
Query positive = positiveQuery.toQuery(parseContext);
|
||||
Query negative = negativeQuery.toQuery(parseContext);
|
||||
if (positive == null || negative == null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
BoostingQuery boostingQuery = new BoostingQuery(positive, negative, negativeBoost);
|
||||
boostingQuery.setBoost(boost);
|
||||
return boostingQuery;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int hashCode() {
|
||||
return Objects.hash(this.boost, this.negativeBoost, this.positiveQuery, this.negativeQuery);
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean equals(Object obj) {
|
||||
if (this == obj) {
|
||||
return true;
|
||||
}
|
||||
if (obj == null || getClass() != obj.getClass()) {
|
||||
return false;
|
||||
}
|
||||
BoostingQueryBuilder other = (BoostingQueryBuilder) obj;
|
||||
return Objects.equals(this.boost, other.boost) &&
|
||||
Objects.equals(this.negativeBoost, other.negativeBoost) &&
|
||||
Objects.equals(this.positiveQuery, other.positiveQuery) &&
|
||||
Objects.equals(this.negativeQuery, other.negativeQuery);
|
||||
}
|
||||
|
||||
@Override
|
||||
public BoostingQueryBuilder readFrom(StreamInput in) throws IOException {
|
||||
QueryBuilder positiveQuery = in.readNamedWriteable();
|
||||
QueryBuilder negativeQuery = in.readNamedWriteable();
|
||||
BoostingQueryBuilder boostingQuery = new BoostingQueryBuilder();
|
||||
boostingQuery.positive(positiveQuery);
|
||||
boostingQuery.negative(negativeQuery);
|
||||
boostingQuery.boost = in.readFloat();
|
||||
boostingQuery.negativeBoost = in.readFloat();
|
||||
return boostingQuery;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void writeTo(StreamOutput out) throws IOException {
|
||||
out.writeNamedWriteable(this.positiveQuery);
|
||||
out.writeNamedWriteable(this.negativeQuery);
|
||||
out.writeFloat(this.boost);
|
||||
out.writeFloat(this.negativeBoost);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -19,8 +19,6 @@
|
|||
|
||||
package org.elasticsearch.index.query;
|
||||
|
||||
import org.apache.lucene.queries.BoostingQuery;
|
||||
import org.apache.lucene.search.Query;
|
||||
import org.elasticsearch.common.inject.Inject;
|
||||
import org.elasticsearch.common.xcontent.XContentParser;
|
||||
|
||||
|
@ -29,7 +27,7 @@ import java.io.IOException;
|
|||
/**
|
||||
*
|
||||
*/
|
||||
public class BoostingQueryParser extends BaseQueryParserTemp {
|
||||
public class BoostingQueryParser extends BaseQueryParser {
|
||||
|
||||
@Inject
|
||||
public BoostingQueryParser() {
|
||||
|
@ -41,14 +39,14 @@ public class BoostingQueryParser extends BaseQueryParserTemp {
|
|||
}
|
||||
|
||||
@Override
|
||||
public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
|
||||
public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
|
||||
XContentParser parser = parseContext.parser();
|
||||
|
||||
Query positiveQuery = null;
|
||||
QueryBuilder positiveQuery = null;
|
||||
boolean positiveQueryFound = false;
|
||||
Query negativeQuery = null;
|
||||
QueryBuilder negativeQuery = null;
|
||||
boolean negativeQueryFound = false;
|
||||
float boost = -1;
|
||||
float boost = 1.0f;
|
||||
float negativeBoost = -1;
|
||||
|
||||
String currentFieldName = null;
|
||||
|
@ -58,10 +56,10 @@ public class BoostingQueryParser extends BaseQueryParserTemp {
|
|||
currentFieldName = parser.currentName();
|
||||
} else if (token == XContentParser.Token.START_OBJECT) {
|
||||
if ("positive".equals(currentFieldName)) {
|
||||
positiveQuery = parseContext.parseInnerQuery();
|
||||
positiveQuery = parseContext.parseInnerQueryBuilder();
|
||||
positiveQueryFound = true;
|
||||
} else if ("negative".equals(currentFieldName)) {
|
||||
negativeQuery = parseContext.parseInnerQuery();
|
||||
negativeQuery = parseContext.parseInnerQueryBuilder();
|
||||
negativeQueryFound = true;
|
||||
} else {
|
||||
throw new QueryParsingException(parseContext, "[boosting] query does not support [" + currentFieldName + "]");
|
||||
|
@ -83,19 +81,16 @@ public class BoostingQueryParser extends BaseQueryParserTemp {
|
|||
if (negativeQuery == null && !negativeQueryFound) {
|
||||
throw new QueryParsingException(parseContext, "[boosting] query requires 'negative' query to be set'");
|
||||
}
|
||||
if (negativeBoost == -1) {
|
||||
throw new QueryParsingException(parseContext, "[boosting] query requires 'negative_boost' to be set'");
|
||||
if (negativeBoost < 0) {
|
||||
throw new QueryParsingException(parseContext, "[boosting] query requires 'negative_boost' to be set to be a positive value'");
|
||||
}
|
||||
|
||||
// parsers returned null
|
||||
if (positiveQuery == null || negativeQuery == null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
BoostingQuery boostingQuery = new BoostingQuery(positiveQuery, negativeQuery, negativeBoost);
|
||||
if (boost != -1) {
|
||||
boostingQuery.setBoost(boost);
|
||||
}
|
||||
BoostingQueryBuilder boostingQuery = new BoostingQueryBuilder();
|
||||
boostingQuery.positive(positiveQuery);
|
||||
boostingQuery.negative(negativeQuery);
|
||||
boostingQuery.negativeBoost(negativeBoost);
|
||||
boostingQuery.boost(boost);
|
||||
boostingQuery.validate();
|
||||
return boostingQuery;
|
||||
}
|
||||
|
||||
|
|
|
@ -60,9 +60,12 @@ public abstract class QueryBuilder<QB extends QueryBuilder> extends ToXContentTo
|
|||
}
|
||||
|
||||
/**
|
||||
* Converts this QueryBuilder to a lucene {@link Query}
|
||||
* Converts this QueryBuilder to a lucene {@link Query}.
|
||||
* Returns <tt>null</tt> if this query should be ignored in the context of
|
||||
* parent queries.
|
||||
*
|
||||
* @param parseContext additional information needed to construct the queries
|
||||
* @return the {@link Query}
|
||||
* @return the {@link Query} or <tt>null</tt> if this query should be ignored upstream
|
||||
* @throws QueryParsingException
|
||||
* @throws IOException
|
||||
*/
|
||||
|
|
|
@ -0,0 +1,128 @@
|
|||
/*
|
||||
* 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.queries.BoostingQuery;
|
||||
import org.apache.lucene.search.Query;
|
||||
import org.elasticsearch.common.xcontent.ToXContent;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.common.xcontent.XContentFactory;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.io.IOException;
|
||||
|
||||
import static org.hamcrest.Matchers.is;
|
||||
|
||||
public class BoostingQueryBuilderTest extends BaseQueryTestCase<BoostingQueryBuilder> {
|
||||
|
||||
@Override
|
||||
protected BoostingQueryBuilder createTestQueryBuilder() {
|
||||
BoostingQueryBuilder query = new BoostingQueryBuilder();
|
||||
query.positive(RandomQueryBuilder.create(random()));
|
||||
query.negative(RandomQueryBuilder.create(random()));
|
||||
query.negativeBoost(2.0f / randomIntBetween(1, 20));
|
||||
if (randomBoolean()) {
|
||||
query.boost(2.0f / randomIntBetween(1, 20));
|
||||
}
|
||||
return query;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Query createExpectedQuery(BoostingQueryBuilder queryBuilder, QueryParseContext context) throws IOException {
|
||||
Query positive = queryBuilder.positive().toQuery(context);
|
||||
Query negative = queryBuilder.negative().toQuery(context);
|
||||
BoostingQuery boostingQuery = new BoostingQuery(positive, negative, queryBuilder.negativeBoost());
|
||||
if (queryBuilder.boost() != 1.0f) {
|
||||
boostingQuery.setBoost(queryBuilder.boost());
|
||||
}
|
||||
return boostingQuery;
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testToXContentIllegalArgumentExceptions() throws IOException {
|
||||
BoostingQueryBuilder boostingQueryBuilder = new BoostingQueryBuilder();
|
||||
boostingQueryBuilder.positive(new MatchAllQueryBuilder());
|
||||
try {
|
||||
boostingQueryBuilder.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS);
|
||||
fail("Expected IllegalArgumentException because of missing negative query.");
|
||||
} catch (IllegalArgumentException e) {
|
||||
// expected
|
||||
}
|
||||
|
||||
boostingQueryBuilder = new BoostingQueryBuilder();
|
||||
boostingQueryBuilder.negative(new MatchAllQueryBuilder());
|
||||
try {
|
||||
boostingQueryBuilder.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS);
|
||||
fail("Expected IllegalArgumentException because of missing positive query.");
|
||||
} catch (IllegalArgumentException e) {
|
||||
// expected
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* tests that we signal upstream queries to ignore this query by returning <tt>null</tt>
|
||||
* if any of the inner query builder is not set
|
||||
*/
|
||||
@Test
|
||||
public void testInnerQueryBuilderNull() throws IOException {
|
||||
BoostingQueryBuilder boostingQueryBuilder = new BoostingQueryBuilder();
|
||||
if (randomBoolean()) {
|
||||
boostingQueryBuilder.positive(new MatchAllQueryBuilder()).negative(null);
|
||||
} else {
|
||||
boostingQueryBuilder.positive(null).negative(new MatchAllQueryBuilder());
|
||||
}
|
||||
assertNull(boostingQueryBuilder.toQuery(createContext()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInnerQueryBuilderReturnsNull() throws IOException {
|
||||
QueryBuilder noOpBuilder = new QueryBuilder<QueryBuilder>() {
|
||||
|
||||
@Override
|
||||
public String queryId() {
|
||||
return "dummy";
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
|
||||
}
|
||||
|
||||
@Override
|
||||
public Query toQuery(QueryParseContext context) {
|
||||
return null;
|
||||
}
|
||||
};
|
||||
BoostingQueryBuilder boostingQueryBuilder = null;
|
||||
if (randomBoolean()) {
|
||||
boostingQueryBuilder = new BoostingQueryBuilder().positive(new MatchAllQueryBuilder()).negative(noOpBuilder);
|
||||
} else {
|
||||
boostingQueryBuilder = new BoostingQueryBuilder().positive(noOpBuilder).negative(new MatchAllQueryBuilder());
|
||||
}
|
||||
assertNull(boostingQueryBuilder.toQuery(createContext()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testValidate() {
|
||||
BoostingQueryBuilder boostingQueryBuilder = new BoostingQueryBuilder();
|
||||
// check for error with negative `negative boost` factor
|
||||
boostingQueryBuilder.negativeBoost(-0.5f);
|
||||
assertThat(boostingQueryBuilder.validate().validationErrors().size(), is(1));
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue