From 89c3eaa329655b6233f3327834a42bd4033e00e7 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Tue, 4 Dec 2018 10:30:06 -0500 Subject: [PATCH] Deprecate setting boost on inner span queries (#36191) Convert parsingException to deprcation warning Substitute for #35967, backport for #34112 --- docs/reference/migration/migrate_6_6.asciidoc | 5 +++-- .../reference/query-dsl/span-queries.asciidoc | 6 +++--- .../query/SpanContainingQueryBuilder.java | 4 ++-- .../index/query/SpanFirstQueryBuilder.java | 2 +- .../index/query/SpanNearQueryBuilder.java | 2 +- .../index/query/SpanNotQueryBuilder.java | 4 ++-- .../index/query/SpanOrQueryBuilder.java | 2 +- .../index/query/SpanQueryBuilder.java | 21 +++++++++---------- .../index/query/SpanWithinQueryBuilder.java | 4 ++-- .../SpanContainingQueryBuilderTests.java | 16 ++++++-------- .../query/SpanFirstQueryBuilderTests.java | 8 +++---- .../query/SpanNearQueryBuilderTests.java | 7 +++---- .../index/query/SpanNotQueryBuilderTests.java | 14 ++++++------- .../index/query/SpanOrQueryBuilderTests.java | 8 +++---- .../query/SpanWithinQueryBuilderTests.java | 16 ++++++-------- 15 files changed, 52 insertions(+), 67 deletions(-) diff --git a/docs/reference/migration/migrate_6_6.asciidoc b/docs/reference/migration/migrate_6_6.asciidoc index 2cfba69c249d9..b083ab200965c 100644 --- a/docs/reference/migration/migrate_6_6.asciidoc +++ b/docs/reference/migration/migrate_6_6.asciidoc @@ -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 diff --git a/docs/reference/query-dsl/span-queries.asciidoc b/docs/reference/query-dsl/span-queries.asciidoc index 7dc65433432ec..55acd39f1f8f3 100644 --- a/docs/reference/query-dsl/span-queries.asciidoc +++ b/docs/reference/query-dsl/span-queries.asciidoc @@ -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). diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java index 164a5809f6e39..4412cae1aa5ba 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanContainingQueryBuilder.java @@ -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 + "]"); diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java index dfd13f9f9fe2b..16fa6a430364a 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java @@ -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 + "]"); } diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java index d43c8120fe0c5..f1c3ce06bbbb8 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanNearQueryBuilder.java @@ -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 { diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanNotQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanNotQueryBuilder.java index 41e632b68f40c..d7f0e8d513622 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanNotQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanNotQueryBuilder.java @@ -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 + "]"); } diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanOrQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanOrQueryBuilder.java index d9b2d9cf4be47..a3efb023d5eeb 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanOrQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanOrQueryBuilder.java @@ -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 { diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanQueryBuilder.java index f7bf784d6cf99..311004536db96 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanQueryBuilder.java @@ -19,8 +19,8 @@ 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. @@ -28,24 +28,23 @@ 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 diff --git a/server/src/main/java/org/elasticsearch/index/query/SpanWithinQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/SpanWithinQueryBuilder.java index 8f970fc25c165..c978fffae1d8b 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SpanWithinQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/SpanWithinQueryBuilder.java @@ -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 + "]"); diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTests.java index e6e62d2909a2a..9c56234e5bb60 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SpanContainingQueryBuilderTests.java @@ -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 { @@ -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" + @@ -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" + @@ -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!"); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanFirstQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanFirstQueryBuilderTests.java index 2ac3610f2d670..3e9b5bb29cf67 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SpanFirstQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SpanFirstQueryBuilderTests.java @@ -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 { @@ -100,7 +99,7 @@ public void testFromJson() throws IOException { } - public void testFromJsonWithNonDefaultBoostInMatchQuery() { + public void testFromJsonWithNonDefaultBoostInMatchQuery() throws IOException { String json = "{\n" + " \"span_first\" : {\n" + @@ -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!"); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTests.java index cde83fb6f7424..64927089272c1 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SpanNearQueryBuilderTests.java @@ -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" + @@ -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!"); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTests.java index 7df58553e2768..f82a9663a2dfa 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SpanNotQueryBuilderTests.java @@ -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" + @@ -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" + @@ -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!"); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanOrQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanOrQueryBuilderTests.java index 9497cebc4ce2f..27f424e9aba9f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SpanOrQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SpanOrQueryBuilderTests.java @@ -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; @@ -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" + @@ -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!"); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/SpanWithinQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SpanWithinQueryBuilderTests.java index a288e2430235a..820a605a47763 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SpanWithinQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SpanWithinQueryBuilderTests.java @@ -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 { @@ -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" + @@ -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" + @@ -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!"); } }