Skip to content

Commit

Permalink
Deprecate setting boost on inner span queries (#36191)
Browse files Browse the repository at this point in the history 
Convert parsingException to deprcation warning

Substitute for #35967, backport for #34112
  • Loading branch information
@mayya-sharipova
mayya-sharipova authored Dec 4, 2018
1 parent 7bd37c0 commit 89c3eaa
Show file tree
Hide file tree
Showing 15 changed files with 52 additions and 67 deletions.
5 changes: 3 additions & 2 deletions docs/reference/migration/migrate_6_6.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ 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.
==== Deprecate boosts on inner span queries.

Attempts to set `boost` on inner span queries will now throw a parsing exception.
Setting `boost` on inner span queries is deprecated. In the next major version
setting `boost` on inner span queries will throw a parsing exception.

[float]
==== Deprecate `.values` and `.getValues()` on doc values in scripts
Expand Down
6 changes: 3 additions & 3 deletions docs/reference/query-dsl/span-queries.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ 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,
Setting `boost` on inner span queries is deprecated. 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.
are never computed on inner span queries, which is the reason why their boosts
don't make sense.

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

Expand Down
4 changes: 2 additions & 2 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 @@ -119,14 +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);
checkNoBoost(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);
checkNoBoost(little);
} else {
throw new ParsingException(parser.getTokenLocation(),
"[span_containing] query does not support [" + currentFieldName + "]");
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public static SpanFirstQueryBuilder fromXContent(XContentParser parser) throws I
throw new ParsingException(parser.getTokenLocation(), "span_first [match] must be of type span query");
}
match = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, match);
checkNoBoost(match);
} else {
throw new ParsingException(parser.getTokenLocation(), "[span_first] query does not support [" + currentFieldName + "]");
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public static SpanNearQueryBuilder fromXContent(XContentParser parser) throws IO
throw new ParsingException(parser.getTokenLocation(), "span_near [clauses] must be of type span query");
}
final SpanQueryBuilder clause = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, clause);
checkNoBoost(clause);
clauses.add(clause);
}
} else {
Expand Down
4 changes: 2 additions & 2 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 @@ -186,14 +186,14 @@ public static SpanNotQueryBuilder fromXContent(XContentParser parser) throws IOE
throw new ParsingException(parser.getTokenLocation(), "span_not [include] must be of type span query");
}
include = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, include);
checkNoBoost(include);
} else if (EXCLUDE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "span_not [exclude] must be of type span query");
}
exclude = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, exclude);
checkNoBoost(exclude);
} else {
throw new ParsingException(parser.getTokenLocation(), "[span_not] query does not support [" + currentFieldName + "]");
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/query/SpanOrQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public static SpanOrQueryBuilder fromXContent(XContentParser parser) throws IOEx
throw new ParsingException(parser.getTokenLocation(), "span_or [clauses] must be of type span query");
}
final SpanQueryBuilder clause = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, clause);
checkNoBoost(clause);
clauses.add(clause);
}
} else {
Expand Down
21 changes: 10 additions & 11 deletions 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,33 +19,32 @@

package org.elasticsearch.index.query;

import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.xcontent.XContentParser;
import org.apache.logging.log4j.LogManager;
import org.elasticsearch.common.logging.DeprecationLogger;

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

class SpanQueryBuilderUtil {

private static final DeprecationLogger DEPRECATION_LOGGER =
new DeprecationLogger(LogManager.getLogger(SpanQueryBuilderUtil.class));

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
* Checks boost value of a nested span clause is equal to {@link AbstractQueryBuilder#DEFAULT_BOOST},
* and if not issues a deprecation warning
* @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) {
static void checkNoBoost(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() + "]");
DEPRECATION_LOGGER.deprecatedAndMaybeLog("span_inner_queries", "setting boost on inner span queries is deprecated!");
}
} catch (UnsupportedOperationException ignored) {
// if boost is unsupported it can't have been set
Expand Down
4 changes: 2 additions & 2 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 @@ -124,14 +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);
checkNoBoost(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);
checkNoBoost(little);
} else {
throw new ParsingException(parser.getTokenLocation(),
"[span_within] query does not support [" + currentFieldName + "]");
Expand Down
16 changes: 6 additions & 10 deletions 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,13 +21,11 @@

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 @@ -94,7 +92,7 @@ public void testFromJson() throws IOException {
assertEquals(json, 2.0, parsed.boost(), 0.0);
}

public void testFromJsoWithNonDefaultBoostInBigQuery() {
public void testFromJsoWithNonDefaultBoostInBigQuery() throws IOException {
String json =
"{\n" +
" \"span_containing\" : {\n" +
Expand Down Expand Up @@ -132,12 +130,11 @@ public void testFromJsoWithNonDefaultBoostInBigQuery() {
" }\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]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}

public void testFromJsonWithNonDefaultBoostInLittleQuery() {
public void testFromJsonWithNonDefaultBoostInLittleQuery() throws IOException {
String json =
"{\n" +
" \"span_containing\" : {\n" +
Expand Down Expand Up @@ -175,8 +172,7 @@ public void testFromJsonWithNonDefaultBoostInLittleQuery() {
" }\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]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}
}
8 changes: 3 additions & 5 deletions server/src/test/java/org/elasticsearch/index/query/SpanFirstQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.io.IOException;

import static org.elasticsearch.index.query.QueryBuilders.spanTermQuery;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;

public class SpanFirstQueryBuilderTests extends AbstractQueryTestCase<SpanFirstQueryBuilder> {
Expand Down Expand Up @@ -100,7 +99,7 @@ public void testFromJson() throws IOException {
}


public void testFromJsonWithNonDefaultBoostInMatchQuery() {
public void testFromJsonWithNonDefaultBoostInMatchQuery() throws IOException {
String json =
"{\n" +
" \"span_first\" : {\n" +
Expand All @@ -117,8 +116,7 @@ public void testFromJsonWithNonDefaultBoostInMatchQuery() {
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_first [match] as a nested span clause can't have non-default boost value [2.0]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}
}
7 changes: 3 additions & 4 deletions server/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public void testCollectPayloadsNoLongerSupported() throws Exception {
assertThat(e.getMessage(), containsString("[span_near] query does not support [collect_payloads]"));
}

public void testFromJsonWithNonDefaultBoostInInnerQuery() {
public void testFromJsonWithNonDefaultBoostInInnerQuery() throws IOException {
String json =
"{\n" +
" \"span_near\" : {\n" +
Expand Down Expand Up @@ -220,8 +220,7 @@ public void testFromJsonWithNonDefaultBoostInInnerQuery() {
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_near [clauses] as a nested span clause can't have non-default boost value [2.0]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}
}
14 changes: 6 additions & 8 deletions server/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ public void testFromJson() throws IOException {
assertEquals(json, 2.0, parsed.boost(), 0.0);
}

public void testFromJsonWithNonDefaultBoostInIncludeQuery() {
public void testFromJsonWithNonDefaultBoostInIncludeQuery() throws IOException {
String json =
"{\n" +
" \"span_not\" : {\n" +
Expand Down Expand Up @@ -255,13 +255,12 @@ public void testFromJsonWithNonDefaultBoostInIncludeQuery() {
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_not [include] as a nested span clause can't have non-default boost value [2.0]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}


public void testFromJsonWithNonDefaultBoostInExcludeQuery() {
public void testFromJsonWithNonDefaultBoostInExcludeQuery() throws IOException {
String json =
"{\n" +
" \"span_not\" : {\n" +
Expand Down Expand Up @@ -301,8 +300,7 @@ public void testFromJsonWithNonDefaultBoostInExcludeQuery() {
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_not [exclude] as a nested span clause can't have non-default boost value [2.0]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}
}
8 changes: 3 additions & 5 deletions server/src/test/java/org/elasticsearch/index/query/SpanOrQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanOrQuery;
import org.apache.lucene.search.spans.SpanQuery;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.AbstractQueryTestCase;

Expand Down Expand Up @@ -106,7 +105,7 @@ public void testFromJson() throws IOException {
assertEquals(json, 2.0, parsed.boost(), 0.0);
}

public void testFromJsonWithNonDefaultBoostInInnerQuery() {
public void testFromJsonWithNonDefaultBoostInInnerQuery() throws IOException {
String json =
"{\n" +
" \"span_or\" : {\n" +
Expand All @@ -122,8 +121,7 @@ public void testFromJsonWithNonDefaultBoostInInnerQuery() {
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_or [clauses] as a nested span clause can't have non-default boost value [2.0]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}
}
16 changes: 6 additions & 10 deletions server/src/test/java/org/elasticsearch/index/query/SpanWithinQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@

import org.apache.lucene.search.Query;
import org.apache.lucene.search.spans.SpanWithinQuery;
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 SpanWithinQueryBuilderTests extends AbstractQueryTestCase<SpanWithinQueryBuilder> {
Expand Down Expand Up @@ -94,7 +92,7 @@ public void testFromJson() throws IOException {
assertEquals(json, 2.0, parsed.boost(), 0.0);
}

public void testFromJsonWithNonDefaultBoostInBigQuery() {
public void testFromJsonWithNonDefaultBoostInBigQuery() throws IOException {
String json =
"{\n" +
" \"span_within\" : {\n" +
Expand Down Expand Up @@ -132,12 +130,11 @@ public void testFromJsonWithNonDefaultBoostInBigQuery() {
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_within [big] as a nested span clause can't have non-default boost value [2.0]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}

public void testFromJsonWithNonDefaultBoostInLittleQuery() {
public void testFromJsonWithNonDefaultBoostInLittleQuery() throws IOException {
String json =
"{\n" +
" \"span_within\" : {\n" +
Expand Down Expand Up @@ -175,8 +172,7 @@ public void testFromJsonWithNonDefaultBoostInLittleQuery() {
" }\n" +
"}";

Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("span_within [little] as a nested span clause can't have non-default boost value [2.0]"));
parseQuery(json);
assertWarnings("setting boost on inner span queries is deprecated!");
}
}

0 comments on commit 89c3eaa

Please sign in to comment.