Skip to content

Commit

Permalink
Throw a parsing exception when boost is set in span_or query (elastic…
Browse files Browse the repository at this point in the history 
…#28390) (elastic#34112)
  • Loading branch information
@cbismuth @mayya-sharipova
cbismuth authored and mayya-sharipova committed Nov 26, 2018
1 parent 0baffda commit b95a4db
Show file tree
Hide file tree
Showing 18 changed files with 474 additions and 49 deletions.
6 changes: 4 additions & 2 deletions docs/reference/migration/migrate_7_0/search.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
now include entire geohash cell, instead of just geohash center.

* Attempts to generate multi-term phrase queries against non-text fields
with a custom analyzer will now throw an exception
with a custom analyzer will now throw an exception.

* An `envelope` crossing the dateline in a `geo_shape `query is now processed
correctly when specified using REST API instead of having its left and
right corners flipped.

* Attempts to set `boost` on inner span queries will now throw a parsing exception.

[float]
==== Adaptive replica selection enabled by default

Expand Down Expand Up @@ -156,4 +158,4 @@ To deboost a specific query you can use a `boost` comprise between 0 and 1.
Negative scores in the Function Score Query are deprecated in 6.x, and are
not allowed in this version. If a negative score is produced as a result
of computation (e.g. in `script_score` or `field_value_factor` functions),
an error will be thrown.
an error will be thrown.
8 changes: 7 additions & 1 deletion docs/reference/query-dsl/span-queries.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ Span queries are low-level positional queries which provide expert control
over the order and proximity of the specified terms. These are typically used
to implement very specific queries on legal documents or patents.

It is only allowed to set boost on an outer span query. Compound span queries,
like span_near, only use the list of matching spans of inner span queries in
order to find their own spans, which they then use to produce a score. Scores
are never computed on inner span queries, which is the reason why boosts are not
allowed: they only influence the way scores are computed, not spans.

Span queries cannot be mixed with non-span queries (with the exception of the `span_multi` query).

The queries in this group are:
Expand Down Expand Up @@ -67,4 +73,4 @@ include::span-containing-query.asciidoc[]

include::span-within-query.asciidoc[]

include::span-field-masking-query.asciidoc[]
include::span-field-masking-query.asciidoc[]
4 changes: 4 additions & 0 deletions server/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import java.io.IOException;
import java.util.Objects;

import static org.elasticsearch.index.query.SpanQueryBuilder.SpanQueryBuilderUtil.checkNoBoost;

/**
* Builder for {@link org.apache.lucene.search.spans.SpanContainingQuery}.
*/
Expand Down Expand Up @@ -117,12 +119,14 @@ public static SpanContainingQueryBuilder fromXContent(XContentParser parser) thr
throw new ParsingException(parser.getTokenLocation(), "span_containing [big] must be of type span query");
}
big = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, big);
} else if (LITTLE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "span_containing [little] must be of type span query");
}
little = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, little);
} else {
throw new ParsingException(parser.getTokenLocation(),
"[span_containing] query does not support [" + currentFieldName + "]");
Expand Down
9 changes: 6 additions & 3 deletions server/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import java.io.IOException;
import java.util.Objects;

import static org.elasticsearch.index.query.SpanQueryBuilder.SpanQueryBuilderUtil.checkNoBoost;

public class SpanFirstQueryBuilder extends AbstractQueryBuilder<SpanFirstQueryBuilder> implements SpanQueryBuilder {
public static final String NAME = "span_first";

Expand Down Expand Up @@ -115,9 +117,10 @@ public static SpanFirstQueryBuilder fromXContent(XContentParser parser) throws I
if (MATCH_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "spanFirst [match] must be of type span query");
throw new ParsingException(parser.getTokenLocation(), "span_first [match] must be of type span query");
}
match = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, match);
} else {
throw new ParsingException(parser.getTokenLocation(), "[span_first] query does not support [" + currentFieldName + "]");
}
Expand All @@ -134,10 +137,10 @@ public static SpanFirstQueryBuilder fromXContent(XContentParser parser) throws I
}
}
if (match == null) {
throw new ParsingException(parser.getTokenLocation(), "spanFirst must have [match] span query clause");
throw new ParsingException(parser.getTokenLocation(), "span_first must have [match] span query clause");
}
if (end == null) {
throw new ParsingException(parser.getTokenLocation(), "spanFirst must have [end] set for it");
throw new ParsingException(parser.getTokenLocation(), "span_first must have [end] set for it");
}
SpanFirstQueryBuilder queryBuilder = new SpanFirstQueryBuilder(match, end);
queryBuilder.boost(boost).queryName(queryName);
Expand Down
8 changes: 6 additions & 2 deletions server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import java.util.List;
import java.util.Objects;

import static org.elasticsearch.index.query.SpanQueryBuilder.SpanQueryBuilderUtil.checkNoBoost;

/**
* Matches spans which are near one another. One can specify slop, the maximum number
* of intervening unmatched positions, as well as whether matches are required to be in-order.
Expand Down Expand Up @@ -166,9 +168,11 @@ public static SpanNearQueryBuilder fromXContent(XContentParser parser) throws IO
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "spanNear [clauses] must be of type span query");
throw new ParsingException(parser.getTokenLocation(), "span_near [clauses] must be of type span query");
}
clauses.add((SpanQueryBuilder) query);
final SpanQueryBuilder clause = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, clause);
clauses.add(clause);
}
} else {
throw new ParsingException(parser.getTokenLocation(), "[span_near] query does not support [" + currentFieldName + "]");
Expand Down
14 changes: 9 additions & 5 deletions server/src/main/java/org/elasticsearch/index/query/SpanNotQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import java.io.IOException;
import java.util.Objects;

import static org.elasticsearch.index.query.SpanQueryBuilder.SpanQueryBuilderUtil.checkNoBoost;

public class SpanNotQueryBuilder extends AbstractQueryBuilder<SpanNotQueryBuilder> implements SpanQueryBuilder {
public static final String NAME = "span_not";

Expand Down Expand Up @@ -181,15 +183,17 @@ public static SpanNotQueryBuilder fromXContent(XContentParser parser) throws IOE
if (INCLUDE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "spanNot [include] must be of type span query");
throw new ParsingException(parser.getTokenLocation(), "span_not [include] must be of type span query");
}
include = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, include);
} else if (EXCLUDE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "spanNot [exclude] must be of type span query");
throw new ParsingException(parser.getTokenLocation(), "span_not [exclude] must be of type span query");
}
exclude = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, exclude);
} else {
throw new ParsingException(parser.getTokenLocation(), "[span_not] query does not support [" + currentFieldName + "]");
}
Expand All @@ -210,13 +214,13 @@ public static SpanNotQueryBuilder fromXContent(XContentParser parser) throws IOE
}
}
if (include == null) {
throw new ParsingException(parser.getTokenLocation(), "spanNot must have [include] span query clause");
throw new ParsingException(parser.getTokenLocation(), "span_not must have [include] span query clause");
}
if (exclude == null) {
throw new ParsingException(parser.getTokenLocation(), "spanNot must have [exclude] span query clause");
throw new ParsingException(parser.getTokenLocation(), "span_not must have [exclude] span query clause");
}
if (dist != null && (pre != null || post != null)) {
throw new ParsingException(parser.getTokenLocation(), "spanNot can either use [dist] or [pre] & [post] (or none)");
throw new ParsingException(parser.getTokenLocation(), "span_not can either use [dist] or [pre] & [post] (or none)");
}

SpanNotQueryBuilder spanNotQuery = new SpanNotQueryBuilder(include, exclude);
Expand Down
10 changes: 7 additions & 3 deletions server/src/main/java/org/elasticsearch/index/query/SpanOrQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import java.util.List;
import java.util.Objects;

import static org.elasticsearch.index.query.SpanQueryBuilder.SpanQueryBuilderUtil.checkNoBoost;

/**
* Span query that matches the union of its clauses. Maps to {@link SpanOrQuery}.
*/
Expand Down Expand Up @@ -113,9 +115,11 @@ public static SpanOrQueryBuilder fromXContent(XContentParser parser) throws IOEx
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "spanOr [clauses] must be of type span query");
throw new ParsingException(parser.getTokenLocation(), "span_or [clauses] must be of type span query");
}
clauses.add((SpanQueryBuilder) query);
final SpanQueryBuilder clause = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, clause);
clauses.add(clause);
}
} else {
throw new ParsingException(parser.getTokenLocation(), "[span_or] query does not support [" + currentFieldName + "]");
Expand All @@ -132,7 +136,7 @@ public static SpanOrQueryBuilder fromXContent(XContentParser parser) throws IOEx
}

if (clauses.isEmpty()) {
throw new ParsingException(parser.getTokenLocation(), "spanOr must include [clauses]");
throw new ParsingException(parser.getTokenLocation(), "span_or must include [clauses]");
}

SpanOrQueryBuilder queryBuilder = new SpanOrQueryBuilder(clauses.get(0));
Expand Down
30 changes: 29 additions & 1 deletion server/src/main/java/org/elasticsearch/index/query/SpanQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,37 @@

package org.elasticsearch.index.query;

import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.xcontent.XContentParser;

/**
* Marker interface for a specific type of {@link QueryBuilder} that allows to build span queries
* Marker interface for a specific type of {@link QueryBuilder} that allows to build span queries.
*/
public interface SpanQueryBuilder extends QueryBuilder {

class SpanQueryBuilderUtil {
private SpanQueryBuilderUtil() {
// utility class
}

/**
* Checks boost value of a nested span clause is equal to {@link AbstractQueryBuilder#DEFAULT_BOOST}.
*
* @param queryName a query name
* @param fieldName a field name
* @param parser a parser
* @param clause a span query builder
* @throws ParsingException if query boost value isn't equal to {@link AbstractQueryBuilder#DEFAULT_BOOST}
*/
static void checkNoBoost(String queryName, String fieldName, XContentParser parser, SpanQueryBuilder clause) {
try {
if (clause.boost() != AbstractQueryBuilder.DEFAULT_BOOST) {
throw new ParsingException(parser.getTokenLocation(), queryName + " [" + fieldName + "] " +
"as a nested span clause can't have non-default boost value [" + clause.boost() + "]");
}
} catch (UnsupportedOperationException ignored) {
// if boost is unsupported it can't have been set
}
}
}
}
4 changes: 4 additions & 0 deletions server/src/main/java/org/elasticsearch/index/query/SpanWithinQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import java.io.IOException;
import java.util.Objects;

import static org.elasticsearch.index.query.SpanQueryBuilder.SpanQueryBuilderUtil.checkNoBoost;

/**
* Builder for {@link org.apache.lucene.search.spans.SpanWithinQuery}.
*/
Expand Down Expand Up @@ -122,12 +124,14 @@ public static SpanWithinQueryBuilder fromXContent(XContentParser parser) throws
throw new ParsingException(parser.getTokenLocation(), "span_within [big] must be of type span query");
}
big = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, big);
} else if (LITTLE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "span_within [little] must be of type span query");
}
little = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, little);
} else {
throw new ParsingException(parser.getTokenLocation(),
"[span_within] query does not support [" + currentFieldName + "]");
Expand Down
91 changes: 90 additions & 1 deletion server/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@

import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanContainingQuery;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.AbstractQueryTestCase;

import java.io.IOException;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;

public class SpanContainingQueryBuilderTests extends AbstractQueryTestCase<SpanContainingQueryBuilder> {
Expand Down Expand Up @@ -80,7 +82,7 @@ public void testFromJson() throws IOException {
" }\n" +
" }\n" +
" },\n" +
" \"boost\" : 1.0\n" +
" \"boost\" : 2.0\n" +
" }\n" +
"}";

Expand All @@ -89,5 +91,92 @@ public void testFromJson() throws IOException {

assertEquals(json, 2, ((SpanNearQueryBuilder) parsed.bigQuery()).clauses().size());
assertEquals(json, "foo", ((SpanTermQueryBuilder) parsed.littleQuery()).value());
assertEquals(json, 2.0, parsed.boost(), 0.0);
}

public void testFromJsoWithNonDefaultBoostInBigQuery() {
String json =
"{\n" +
" \"span_containing\" : {\n" +
" \"big\" : {\n" +
" \"span_near\" : {\n" +
" \"clauses\" : [ {\n" +
" \"span_term\" : {\n" +
" \"field1\" : {\n" +
" \"value\" : \"bar\",\n" +
" \"boost\" : 1.0\n" +
" }\n" +
" }\n" +
" }, {\n" +
" \"span_term\" : {\n" +
" \"field1\" : {\n" +
" \"value\" : \"baz\",\n" +
" \"boost\" : 1.0\n" +
" }\n" +
" }\n" +
" } ],\n" +
" \"slop\" : 5,\n" +
" \"in_order\" : true,\n" +
" \"boost\" : 2.0\n" +
" }\n" +
" },\n" +
" \"little\" : {\n" +
" \"span_term\" : {\n" +
" \"field1\" : {\n" +
" \"value\" : \"foo\",\n" +
" \"boost\" : 1.0\n" +
" }\n" +
" }\n" +
" },\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_containing [big] as a nested span clause can't have non-default boost value [2.0]"));
}

public void testFromJsonWithNonDefaultBoostInLittleQuery() {
String json =
"{\n" +
" \"span_containing\" : {\n" +
" \"little\" : {\n" +
" \"span_near\" : {\n" +
" \"clauses\" : [ {\n" +
" \"span_term\" : {\n" +
" \"field1\" : {\n" +
" \"value\" : \"bar\",\n" +
" \"boost\" : 1.0\n" +
" }\n" +
" }\n" +
" }, {\n" +
" \"span_term\" : {\n" +
" \"field1\" : {\n" +
" \"value\" : \"baz\",\n" +
" \"boost\" : 1.0\n" +
" }\n" +
" }\n" +
" } ],\n" +
" \"slop\" : 5,\n" +
" \"in_order\" : true,\n" +
" \"boost\" : 2.0\n" +
" }\n" +
" },\n" +
" \"big\" : {\n" +
" \"span_term\" : {\n" +
" \"field1\" : {\n" +
" \"value\" : \"foo\",\n" +
" \"boost\" : 1.0\n" +
" }\n" +
" }\n" +
" },\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_containing [little] as a nested span clause can't have non-default boost value [2.0]"));
}
}
Loading

0 comments on commit b95a4db

Please sign in to comment.