Skip to content

Commit

Permalink
Disallow boosts on inner span queries (#35967)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mayya-sharipova
mayya-sharipova authored Nov 29, 2018
1 parent 6595ded commit ef0180a
Show file tree
Hide file tree
Showing 18 changed files with 475 additions and 47 deletions.
5 changes: 5 additions & 0 deletions docs/reference/migration/migrate_6_6.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ rest of Elasticsearc's APIs and Elasticsearch will raise a deprecation warning
if those are used on any APIs. We plan to drop support for `_source_exclude` and
`_source_include` in 7.0.

[float]
==== Boosts on inner span queries are not allowed.

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

[float]
==== Deprecate `.values` and `.getValues()` on doc values in scripts

Expand Down
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 ef0180a

Please sign in to comment.