From d6a2e01e4b59de533cae7113be60ecbc3be55646 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 28 May 2019 13:08:13 +0200 Subject: [PATCH 01/10] Deprecate CommonTermsQuery and cutoff_frequency Since the max_score optimization landed in Elasticsearch 7, the CommonTermsQuery is redundant and slower. Moreover the cutoff_frequency parameter for MatchQuery and MultiMatchQuery is also redundant. Relates to #27096 --- .../query-dsl/common-terms-query.asciidoc | 2 ++ docs/reference/query-dsl/match-query.asciidoc | 2 ++ .../lucene/queries/BlendedTermQuery.java | 5 ++++ .../queries/ExtendedCommonTermsQuery.java | 4 +++ .../index/query/CommonTermsQueryBuilder.java | 4 +++ .../index/query/MatchQueryBuilder.java | 24 ++++++++++++++- .../index/query/MultiMatchQueryBuilder.java | 29 ++++++++++++++++++- .../index/query/QueryBuilders.java | 3 ++ .../index/search/MatchQuery.java | 17 +++++++++++ .../query/CommonTermsQueryBuilderTests.java | 4 +++ .../query/CommonTermsQueryParserTests.java | 8 +++-- .../index/query/MatchQueryBuilderTests.java | 9 +++--- .../query/MultiMatchQueryBuilderTests.java | 11 +++++-- .../profile/query/RandomQueryGenerator.java | 4 +++ 14 files changed, 114 insertions(+), 12 deletions(-) diff --git a/docs/reference/query-dsl/common-terms-query.asciidoc b/docs/reference/query-dsl/common-terms-query.asciidoc index 87288778246a6..d33c36a616ee4 100644 --- a/docs/reference/query-dsl/common-terms-query.asciidoc +++ b/docs/reference/query-dsl/common-terms-query.asciidoc @@ -1,6 +1,8 @@ [[query-dsl-common-terms-query]] === Common Terms Query +deprecated[7.3.0,Replaced by the more performant `max_score` optimization which is applied automatically without any configuration on a <>] + The `common` terms query is a modern alternative to stopwords which improves the precision and recall of search results (by taking stopwords into account), without sacrificing performance. diff --git a/docs/reference/query-dsl/match-query.asciidoc b/docs/reference/query-dsl/match-query.asciidoc index 89a0a942b79ce..e7635aa7f7877 100644 --- a/docs/reference/query-dsl/match-query.asciidoc +++ b/docs/reference/query-dsl/match-query.asciidoc @@ -122,6 +122,8 @@ GET /_search [[query-dsl-match-query-cutoff]] ===== Cutoff frequency +deprecated[7.3.0,Replaced by the more performant `max_score` optimization which is applied automatically without any configuration] + The match query supports a `cutoff_frequency` that allows specifying an absolute or relative document frequency where high frequency terms are moved into an optional subquery and are only scored diff --git a/server/src/main/java/org/apache/lucene/queries/BlendedTermQuery.java b/server/src/main/java/org/apache/lucene/queries/BlendedTermQuery.java index c696d476bbb43..f823f3a142690 100644 --- a/server/src/main/java/org/apache/lucene/queries/BlendedTermQuery.java +++ b/server/src/main/java/org/apache/lucene/queries/BlendedTermQuery.java @@ -278,6 +278,11 @@ public int hashCode() { return Objects.hash(classHash(), Arrays.hashCode(equalsTerms())); } + /** + * @deprecated Since max_score optimization landed in 7.0, normal MultiMatchQuery + * will achieve the same result without any configuration. + */ + @Deprecated public static BlendedTermQuery commonTermsBlendedQuery(Term[] terms, final float[] boosts, final float maxTermFrequency) { return new BlendedTermQuery(terms, boosts) { @Override diff --git a/server/src/main/java/org/apache/lucene/queries/ExtendedCommonTermsQuery.java b/server/src/main/java/org/apache/lucene/queries/ExtendedCommonTermsQuery.java index 249b7fa83b50b..2d70ed8b90a05 100644 --- a/server/src/main/java/org/apache/lucene/queries/ExtendedCommonTermsQuery.java +++ b/server/src/main/java/org/apache/lucene/queries/ExtendedCommonTermsQuery.java @@ -26,7 +26,11 @@ * Extended version of {@link CommonTermsQuery} that allows to pass in a * {@code minimumNumberShouldMatch} specification that uses the actual num of high frequent terms * to calculate the minimum matching terms. + * + * @deprecated Since max_optimization optimization landed in 7.0, normal MatchQuery + * will achieve the same result without any configuration. */ +@Deprecated public class ExtendedCommonTermsQuery extends CommonTermsQuery { public ExtendedCommonTermsQuery(Occur highFreqOccur, Occur lowFreqOccur, float maxTermFrequency) { diff --git a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java index d646dc4bb4b07..15800fd8161b4 100644 --- a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java @@ -47,7 +47,11 @@ * and high-frequency terms are added to an optional boolean clause. The * optional clause is only executed if the required "low-frequency' clause * matches. + * + * @deprecated Since max_optimization optimization landed in 7.0, normal MatchQuery + * will achieve the same result without any configuration. */ +@Deprecated public class CommonTermsQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "common"; diff --git a/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java index 0d451bd86f264..d8e13817f6548 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java @@ -19,12 +19,14 @@ package org.elasticsearch.index.query; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.search.FuzzyQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -42,8 +44,20 @@ * result of the analysis. */ public class MatchQueryBuilder extends AbstractQueryBuilder { + + private static final DeprecationLogger deprecationLogger = + new DeprecationLogger(LogManager.getLogger(MatchQueryBuilder.class)); + + static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "[cutoff_frequency] has been deprecated in favor of the " + + "[max_score] optimization which is applied automatically without any configuration"; + public static final ParseField ZERO_TERMS_QUERY_FIELD = new ParseField("zero_terms_query"); - public static final ParseField CUTOFF_FREQUENCY_FIELD = new ParseField("cutoff_frequency"); + /** + * @deprecated Since max_optimization optimization landed in 7.0, normal MatchQuery + * will achieve the same result without any configuration. + */ + @Deprecated + public static final ParseField CUTOFF_FREQUENCY_FIELD = new ParseField("cutoff_frequency", "cutoff_frequency"); public static final ParseField LENIENT_FIELD = new ParseField("lenient"); public static final ParseField FUZZY_TRANSPOSITIONS_FIELD = new ParseField("fuzzy_transpositions"); public static final ParseField FUZZY_REWRITE_FIELD = new ParseField("fuzzy_rewrite"); @@ -85,6 +99,10 @@ public class MatchQueryBuilder extends AbstractQueryBuilder { private MatchQuery.ZeroTermsQuery zeroTermsQuery = MatchQuery.DEFAULT_ZERO_TERMS_QUERY; + /** + * @deprecated See {@link MatchQueryBuilder#CUTOFF_FREQUENCY_FIELD} for more details + */ + @Deprecated private Float cutoffFrequency = null; private boolean autoGenerateSynonymsPhraseQuery = true; @@ -235,8 +253,12 @@ public int maxExpansions() { * Set a cutoff value in [0..1] (or absolute number >=1) representing the * maximum threshold of a terms document frequency to be considered a low * frequency term. + * + * @deprecated see {@link MatchQueryBuilder#CUTOFF_FREQUENCY_FIELD} for more details */ + @Deprecated public MatchQueryBuilder cutoffFrequency(float cutoff) { + deprecationLogger.deprecated(CUTOFF_FREQUENCY_DEPRECATION_MSG); this.cutoffFrequency = cutoff; return this; } diff --git a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java index 7827c032ea0d7..84ae1ce0afaac 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.search.FuzzyQuery; import org.apache.lucene.search.Query; import org.elasticsearch.ElasticsearchParseException; @@ -28,6 +29,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -50,6 +52,13 @@ * Same as {@link MatchQueryBuilder} but supports multiple fields. */ public class MultiMatchQueryBuilder extends AbstractQueryBuilder { + + private static final DeprecationLogger deprecationLogger = + new DeprecationLogger(LogManager.getLogger(MultiMatchQueryBuilder.class)); + + static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "[cutoff_frequency] has been deprecated in favor of the " + + "[max_score] optimization which is applied automatically without any configuration"; + public static final String NAME = "multi_match"; public static final MultiMatchQueryBuilder.Type DEFAULT_TYPE = MultiMatchQueryBuilder.Type.BEST_FIELDS; @@ -63,7 +72,12 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder { @Override diff --git a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java index f4e737ea4b024..334bbd4a66544 100644 --- a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java @@ -22,10 +22,12 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.test.ESSingleNodeTestCase; -import java.io.IOException; - +/** + * @deprecated See {@link CommonTermsQueryBuilder} + */ +@Deprecated public class CommonTermsQueryParserTests extends ESSingleNodeTestCase { - public void testWhenParsedQueryIsNullNoNullPointerExceptionIsThrown() throws IOException { + public void testWhenParsedQueryIsNullNoNullPointerExceptionIsThrown() { final String index = "test-index"; final String type = "test-type"; client() diff --git a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java index a7aad3dbc3e42..9b2a1e195895c 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java @@ -124,10 +124,6 @@ protected MatchQueryBuilder doCreateTestQueryBuilder() { matchQuery.zeroTermsQuery(randomFrom(ZeroTermsQuery.ALL, ZeroTermsQuery.NONE)); } - if (randomBoolean()) { - matchQuery.cutoffFrequency((float) 10 / randomIntBetween(1, 100)); - } - if (randomBoolean()) { matchQuery.autoGenerateSynonymsPhraseQuery(randomBoolean()); } @@ -478,6 +474,11 @@ public void testMaxBooleanClause() { query.setAnalyzer(new MockGraphAnalyzer(createGiantGraphMultiTerms())); expectThrows(BooleanQuery.TooManyClauses.class, () -> query.parse(Type.PHRASE, STRING_FIELD_NAME, "")); } + + public void testCutoffFrequency() { + new MatchQueryBuilder("field", "value").cutoffFrequency((float) 10 / randomIntBetween(1, 100)); + assertWarnings(MatchQueryBuilder.CUTOFF_FREQUENCY_DEPRECATION_MSG); + } private static class MockGraphAnalyzer extends Analyzer { diff --git a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java index 6590a5609353a..bce3ddf6de2d2 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java @@ -134,9 +134,6 @@ protected MultiMatchQueryBuilder doCreateTestQueryBuilder() { if (randomBoolean()) { query.tieBreaker(randomFloat()); } - if (randomBoolean() && query.type() != Type.BOOL_PREFIX) { - query.cutoffFrequency((float) 10 / randomIntBetween(1, 100)); - } if (randomBoolean()) { query.zeroTermsQuery(randomFrom(MatchQuery.ZeroTermsQuery.NONE, MatchQuery.ZeroTermsQuery.ALL)); } @@ -558,6 +555,14 @@ public void testNegativeFieldBoost() { assertThat(exc.getMessage(), containsString("negative [boost]")); } + public void testCutoffFrequency() { + new MultiMatchQueryBuilder("value", "field1", "field2").cutoffFrequency(10f / randomIntBetween(1, 100)); + assertWarnings(MultiMatchQueryBuilder.CUTOFF_FREQUENCY_DEPRECATION_MSG); + new MultiMatchQueryBuilder("value", "field1", "field2").cutoffFrequency(Float.valueOf(10f / randomIntBetween(1, 100))); + assertWarnings(MultiMatchQueryBuilder.CUTOFF_FREQUENCY_DEPRECATION_MSG); + + } + private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) { Settings build = Settings.builder().put(oldIndexSettings) .put(indexSettings) diff --git a/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java b/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java index 00b859394c65f..47752d32f558d 100644 --- a/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java +++ b/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java @@ -169,6 +169,10 @@ private static QueryBuilder randomConstantScoreQuery(List stringFields, return QueryBuilders.constantScoreQuery(randomQueryBuilder(stringFields, numericFields, numDocs, depth - 1)); } + /** + * @deprecated See {@link CommonTermsQueryBuilder} + */ + @Deprecated private static QueryBuilder randomCommonTermsQuery(List fields, int numDocs) { int numTerms = randomInt(numDocs); From 54451426c1ce2054d5ececafb6a3f175dde8f9e0 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 28 May 2019 14:20:12 +0200 Subject: [PATCH 02/10] Fix tests --- .../query-dsl/common-terms-query.asciidoc | 9 +++++++ docs/reference/query-dsl/match-query.asciidoc | 1 + .../search.query/50_queries_with_synonyms.yml | 27 +++++++++++++++++++ .../index/query/CommonTermsQueryBuilder.java | 10 +++++++ .../query/CommonTermsQueryBuilderTests.java | 10 +++++++ .../query/CommonTermsQueryParserTests.java | 3 +++ .../query/MultiMatchQueryBuilderTests.java | 1 - 7 files changed, 60 insertions(+), 1 deletion(-) diff --git a/docs/reference/query-dsl/common-terms-query.asciidoc b/docs/reference/query-dsl/common-terms-query.asciidoc index d33c36a616ee4..fc82f7c82d366 100644 --- a/docs/reference/query-dsl/common-terms-query.asciidoc +++ b/docs/reference/query-dsl/common-terms-query.asciidoc @@ -85,6 +85,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] The number of terms which should match can be controlled with the <> @@ -110,6 +111,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] which is roughly equivalent to: @@ -134,6 +136,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] Alternatively use <> @@ -156,6 +159,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] which is roughly equivalent to: @@ -185,6 +189,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] A different <> @@ -211,6 +216,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] which is roughly equivalent to: @@ -246,6 +252,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] In this case it means the high frequency terms have only an impact on relevance when there are at least three of them. But the most @@ -272,6 +279,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] which is roughly equivalent to: @@ -293,6 +301,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] The high frequency generated query is then slightly less restrictive than with an `AND`. diff --git a/docs/reference/query-dsl/match-query.asciidoc b/docs/reference/query-dsl/match-query.asciidoc index e7635aa7f7877..430344d4b0740 100644 --- a/docs/reference/query-dsl/match-query.asciidoc +++ b/docs/reference/query-dsl/match-query.asciidoc @@ -160,6 +160,7 @@ GET /_search } -------------------------------------------------- // CONSOLE +// TEST[warning:[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration] IMPORTANT: The `cutoff_frequency` option operates on a per-shard-level. This means that when trying it out on test indexes with low document numbers you diff --git a/modules/analysis-common/src/test/resources/rest-api-spec/test/search.query/50_queries_with_synonyms.yml b/modules/analysis-common/src/test/resources/rest-api-spec/test/search.query/50_queries_with_synonyms.yml index 784ffd9dd123a..362e55569a49c 100644 --- a/modules/analysis-common/src/test/resources/rest-api-spec/test/search.query/50_queries_with_synonyms.yml +++ b/modules/analysis-common/src/test/resources/rest-api-spec/test/search.query/50_queries_with_synonyms.yml @@ -1,5 +1,8 @@ --- "Test common terms query with stacked tokens": + - skip: + features: "warnings" + - do: indices.create: index: test @@ -47,6 +50,8 @@ refresh: true - do: + warnings: + - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -62,6 +67,8 @@ - match: { hits.hits.2._id: "3" } - do: + warnings: + - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -76,6 +83,8 @@ - match: { hits.hits.1._id: "2" } - do: + warnings: + - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -90,6 +99,8 @@ - match: { hits.hits.2._id: "3" } - do: + warnings: + - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -103,6 +114,8 @@ - match: { hits.hits.0._id: "2" } - do: + warnings: + - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -118,6 +131,8 @@ - match: { hits.hits.1._id: "1" } - do: + warnings: + - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -132,6 +147,8 @@ - match: { hits.hits.0._id: "2" } - do: + warnings: + - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -144,6 +161,8 @@ - match: { hits.hits.0._id: "2" } - do: + warnings: + - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -158,6 +177,8 @@ - match: { hits.hits.2._id: "3" } - do: + warnings: + - '[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -172,6 +193,8 @@ - match: { hits.hits.1._id: "2" } - do: + warnings: + - '[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -187,6 +210,8 @@ - match: { hits.hits.2._id: "3" } - do: + warnings: + - '[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: @@ -201,6 +226,8 @@ - match: { hits.hits.1._id: "2" } - do: + warnings: + - '[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration' search: rest_total_hits_as_int: true body: diff --git a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java index 15800fd8161b4..84fecacc39c17 100644 --- a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.TokenStream; import org.apache.lucene.analysis.tokenattributes.CharTermAttribute; @@ -32,6 +33,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.MappedFieldType; @@ -54,6 +56,12 @@ @Deprecated public class CommonTermsQueryBuilder extends AbstractQueryBuilder { + private static final DeprecationLogger deprecationLogger = + new DeprecationLogger(LogManager.getLogger(CommonTermsQueryBuilder.class)); + + static final String COMMON_TERMS_QUERY_DEPRECATION_MSG = "[Common Terms Query] has been deprecated in favor of the " + + "MatchQuery [max_score] optimization which is applied automatically without any configuration"; + public static final String NAME = "common"; public static final float DEFAULT_CUTOFF_FREQ = 0.01f; @@ -91,6 +99,7 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder { + @After + public void validateDeprecationMessage() { + if (getTestName().equals("testParseFailsWithMultipleFields") == false) { + assertWarnings(CommonTermsQueryBuilder.COMMON_TERMS_QUERY_DEPRECATION_MSG); + } + } + @Override protected CommonTermsQueryBuilder doCreateTestQueryBuilder() { int numberOfTerms = randomIntBetween(0, 10); diff --git a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java index 334bbd4a66544..c1f631ce682f5 100644 --- a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.test.ESSingleNodeTestCase; +import org.junit.After; /** * @deprecated See {@link CommonTermsQueryBuilder} @@ -48,5 +49,7 @@ public void testWhenParsedQueryIsNullNoNullPointerExceptionIsThrown() { assertNotNull(response); assertEquals(response.getHits().getHits().length, 0); + + assertWarnings(CommonTermsQueryBuilder.COMMON_TERMS_QUERY_DEPRECATION_MSG); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java index bce3ddf6de2d2..2edb0088b99fe 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java @@ -560,7 +560,6 @@ public void testCutoffFrequency() { assertWarnings(MultiMatchQueryBuilder.CUTOFF_FREQUENCY_DEPRECATION_MSG); new MultiMatchQueryBuilder("value", "field1", "field2").cutoffFrequency(Float.valueOf(10f / randomIntBetween(1, 100))); assertWarnings(MultiMatchQueryBuilder.CUTOFF_FREQUENCY_DEPRECATION_MSG); - } private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) { From 8abbce8d4eff2d856768d1d4074b2fe367f625a2 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 28 May 2019 14:30:32 +0200 Subject: [PATCH 03/10] remove unused import --- .../elasticsearch/index/query/CommonTermsQueryParserTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java index c1f631ce682f5..6bd0e18af4c30 100644 --- a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java @@ -21,7 +21,6 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.test.ESSingleNodeTestCase; -import org.junit.After; /** * @deprecated See {@link CommonTermsQueryBuilder} From 5d63b69b34b6dd5134fed6e13d8ffe7128f66aa1 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 28 May 2019 14:54:26 +0200 Subject: [PATCH 04/10] Fix tests --- docs/reference/query-dsl/common-terms-query.asciidoc | 4 ---- .../index/query/CommonTermsQueryBuilderTests.java | 2 -- 2 files changed, 6 deletions(-) diff --git a/docs/reference/query-dsl/common-terms-query.asciidoc b/docs/reference/query-dsl/common-terms-query.asciidoc index fc82f7c82d366..6ad18d1461912 100644 --- a/docs/reference/query-dsl/common-terms-query.asciidoc +++ b/docs/reference/query-dsl/common-terms-query.asciidoc @@ -136,7 +136,6 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] Alternatively use <> @@ -189,7 +188,6 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] A different <> @@ -252,7 +250,6 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] In this case it means the high frequency terms have only an impact on relevance when there are at least three of them. But the most @@ -301,7 +298,6 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] The high frequency generated query is then slightly less restrictive than with an `AND`. diff --git a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryBuilderTests.java index d214e03718a6c..ff395e31ba0e8 100644 --- a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryBuilderTests.java @@ -26,8 +26,6 @@ import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; import org.junit.After; -import org.junit.Rule; -import org.junit.rules.TestName; import java.io.IOException; import java.util.HashMap; From fc0335f14f80571c5b2f30ce24faef7d21cc3e47 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 28 May 2019 15:44:40 +0200 Subject: [PATCH 05/10] fix tests --- .../client/documentation/QueryDSLDocumentationTests.java | 2 ++ .../org/elasticsearch/index/query/CommonTermsQueryBuilder.java | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java index 51670b29de1b6..60b13d9919197 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.geo.builders.CoordinatesBuilder; import org.elasticsearch.common.geo.builders.MultiPointBuilder; import org.elasticsearch.common.unit.DistanceUnit; +import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.GeoShapeQueryBuilder; import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder; import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder.FilterFunctionBuilder; @@ -111,6 +112,7 @@ public void testCommonTerms() { commonTermsQuery("name", // <1> "kimchy"); // <2> // end::common_terms + assertWarnings(CommonTermsQueryBuilder.COMMON_TERMS_QUERY_DEPRECATION_MSG); } public void testConstantScore() { diff --git a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java index 84fecacc39c17..0ce5ae2e927e6 100644 --- a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java @@ -59,7 +59,7 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder Date: Tue, 28 May 2019 16:12:31 +0200 Subject: [PATCH 06/10] Address comments --- .../index/query/MatchQueryBuilder.java | 4 --- .../index/query/MultiMatchQueryBuilder.java | 16 +++------ .../index/search/MatchQuery.java | 12 ------- .../query/CommonTermsQueryBuilderTests.java | 4 --- .../query/CommonTermsQueryParserTests.java | 4 --- .../profile/query/RandomQueryGenerator.java | 35 ++----------------- 6 files changed, 7 insertions(+), 68 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java index d8e13817f6548..b1004a5def040 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java @@ -99,10 +99,6 @@ public class MatchQueryBuilder extends AbstractQueryBuilder { private MatchQuery.ZeroTermsQuery zeroTermsQuery = MatchQuery.DEFAULT_ZERO_TERMS_QUERY; - /** - * @deprecated See {@link MatchQueryBuilder#CUTOFF_FREQUENCY_FIELD} for more details - */ - @Deprecated private Float cutoffFrequency = null; private boolean autoGenerateSynonymsPhraseQuery = true; diff --git a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java index 84ae1ce0afaac..c8ddb1cd5b138 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java @@ -72,11 +72,6 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder { @After diff --git a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java index 6bd0e18af4c30..19c418b4d6da5 100644 --- a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java @@ -22,10 +22,6 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.test.ESSingleNodeTestCase; -/** - * @deprecated See {@link CommonTermsQueryBuilder} - */ -@Deprecated public class CommonTermsQueryParserTests extends ESSingleNodeTestCase { public void testWhenParsedQueryIsNullNoNullPointerExceptionIsThrown() { final String index = "test-index"; diff --git a/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java b/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java index 47752d32f558d..97eab8282af5b 100644 --- a/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java +++ b/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java @@ -72,7 +72,7 @@ public static QueryBuilder randomQueryBuilder(List stringFields, List stringFields, List numericFields, int numDocs) { - switch (randomIntBetween(0,6)) { + switch (randomIntBetween(0,5)) { case 0: return randomTermQuery(stringFields, numDocs); case 1: @@ -82,10 +82,8 @@ private static QueryBuilder randomTerminalQuery(List stringFields, List< case 3: return QueryBuilders.matchAllQuery(); case 4: - return randomCommonTermsQuery(stringFields, numDocs); - case 5: return randomFuzzyQuery(stringFields); - case 6: + case 5: return randomIDsQuery(); default: return randomTermQuery(stringFields, numDocs); @@ -169,35 +167,6 @@ private static QueryBuilder randomConstantScoreQuery(List stringFields, return QueryBuilders.constantScoreQuery(randomQueryBuilder(stringFields, numericFields, numDocs, depth - 1)); } - /** - * @deprecated See {@link CommonTermsQueryBuilder} - */ - @Deprecated - private static QueryBuilder randomCommonTermsQuery(List fields, int numDocs) { - int numTerms = randomInt(numDocs); - - QueryBuilder q = QueryBuilders.commonTermsQuery(randomField(fields), randomQueryString(numTerms)); - if (randomBoolean()) { - ((CommonTermsQueryBuilder)q).boost(randomFloat()); - } - - if (randomBoolean()) { - ((CommonTermsQueryBuilder)q).cutoffFrequency(randomFloat()); - } - - if (randomBoolean()) { - ((CommonTermsQueryBuilder)q).highFreqMinimumShouldMatch(Integer.toString(randomInt(numTerms))) - .highFreqOperator(randomBoolean() ? Operator.AND : Operator.OR); - } - - if (randomBoolean()) { - ((CommonTermsQueryBuilder)q).lowFreqMinimumShouldMatch(Integer.toString(randomInt(numTerms))) - .lowFreqOperator(randomBoolean() ? Operator.AND : Operator.OR); - } - - return q; - } - private static QueryBuilder randomFuzzyQuery(List fields) { QueryBuilder q = QueryBuilders.fuzzyQuery(randomField(fields), randomQueryString(1)); From 962f2bc866ef2fa200f70fb55a165f1b2a7803ab Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Tue, 28 May 2019 16:27:57 +0200 Subject: [PATCH 07/10] remove unused imports --- .../main/java/org/elasticsearch/index/search/MatchQuery.java | 1 - .../search/profile/query/RandomQueryGenerator.java | 2 -- 2 files changed, 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java b/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java index bec155c3dc49c..ec36b1dceaf97 100644 --- a/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java +++ b/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java @@ -55,7 +55,6 @@ import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.TextFieldMapper; -import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.support.QueryParsers; diff --git a/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java b/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java index 97eab8282af5b..ea9ef964153b7 100644 --- a/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java +++ b/server/src/test/java/org/elasticsearch/search/profile/query/RandomQueryGenerator.java @@ -22,11 +22,9 @@ import org.apache.lucene.util.English; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.index.query.BoolQueryBuilder; -import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.DisMaxQueryBuilder; import org.elasticsearch.index.query.FuzzyQueryBuilder; import org.elasticsearch.index.query.IdsQueryBuilder; -import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.RangeQueryBuilder; From d6b087f1ec7d8bfb75881cee7edada125e45dc9f Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Wed, 29 May 2019 18:02:52 +0200 Subject: [PATCH 08/10] Address comments Remove implicit usage of the deprecation logger. --- .../query-dsl/common-terms-query.asciidoc | 12 ++--- docs/reference/query-dsl/match-query.asciidoc | 4 +- .../search.query/50_queries_with_synonyms.yml | 24 +++++----- .../index/query/CommonTermsQueryBuilder.java | 15 +++--- .../index/query/MatchQueryBuilder.java | 13 ++--- .../index/query/MultiMatchQueryBuilder.java | 16 ++----- .../elasticsearch/search/SearchModule.java | 7 ++- .../query/CommonTermsQueryBuilderTests.java | 48 +++++++++++++++---- .../query/CommonTermsQueryParserTests.java | 2 - .../index/query/MatchQueryBuilderTests.java | 5 -- .../query/MultiMatchQueryBuilderTests.java | 7 --- .../search/SearchModuleTests.java | 6 +-- .../test/AbstractQueryTestCase.java | 3 +- 13 files changed, 83 insertions(+), 79 deletions(-) diff --git a/docs/reference/query-dsl/common-terms-query.asciidoc b/docs/reference/query-dsl/common-terms-query.asciidoc index 6ad18d1461912..5fb6145f40dbb 100644 --- a/docs/reference/query-dsl/common-terms-query.asciidoc +++ b/docs/reference/query-dsl/common-terms-query.asciidoc @@ -1,7 +1,7 @@ [[query-dsl-common-terms-query]] === Common Terms Query -deprecated[7.3.0,Replaced by the more performant `max_score` optimization which is applied automatically without any configuration on a <>] +deprecated[7.3.0,`<>` which can efficiently skip blocks of documents if the total number of hits is not tracked] The `common` terms query is a modern alternative to stopwords which improves the precision and recall of search results (by taking stopwords @@ -85,7 +85,7 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] +// TEST[warning:Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]] The number of terms which should match can be controlled with the <> @@ -111,7 +111,7 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] +// TEST[warning:Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]] which is roughly equivalent to: @@ -158,7 +158,7 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] +// TEST[warning:Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]] which is roughly equivalent to: @@ -214,7 +214,7 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] +// TEST[warning:Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]] which is roughly equivalent to: @@ -276,7 +276,7 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration] +// TEST[warning:Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]] which is roughly equivalent to: diff --git a/docs/reference/query-dsl/match-query.asciidoc b/docs/reference/query-dsl/match-query.asciidoc index 430344d4b0740..14fc155cfccae 100644 --- a/docs/reference/query-dsl/match-query.asciidoc +++ b/docs/reference/query-dsl/match-query.asciidoc @@ -122,7 +122,7 @@ GET /_search [[query-dsl-match-query-cutoff]] ===== Cutoff frequency -deprecated[7.3.0,Replaced by the more performant `max_score` optimization which is applied automatically without any configuration] +deprecated[7.3.0,"This option can be omitted as the <> can skip block of documents efficiently, without any configuration, provided that the total number of hits is not tracked."] The match query supports a `cutoff_frequency` that allows specifying an absolute or relative document frequency where high @@ -160,7 +160,7 @@ GET /_search } -------------------------------------------------- // CONSOLE -// TEST[warning:[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration] +// TEST[warning:Deprecated field [cutoff_frequency] used, replaced by [you can omit this option, the [match] query can skip block of documents efficiently if the total number of hits is not tracked]] IMPORTANT: The `cutoff_frequency` option operates on a per-shard-level. This means that when trying it out on test indexes with low document numbers you diff --git a/modules/analysis-common/src/test/resources/rest-api-spec/test/search.query/50_queries_with_synonyms.yml b/modules/analysis-common/src/test/resources/rest-api-spec/test/search.query/50_queries_with_synonyms.yml index 362e55569a49c..ce9cc74955729 100644 --- a/modules/analysis-common/src/test/resources/rest-api-spec/test/search.query/50_queries_with_synonyms.yml +++ b/modules/analysis-common/src/test/resources/rest-api-spec/test/search.query/50_queries_with_synonyms.yml @@ -51,7 +51,7 @@ - do: warnings: - - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -68,7 +68,7 @@ - do: warnings: - - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -84,7 +84,7 @@ - do: warnings: - - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -100,7 +100,7 @@ - do: warnings: - - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -115,7 +115,7 @@ - do: warnings: - - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -132,7 +132,7 @@ - do: warnings: - - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -148,7 +148,7 @@ - do: warnings: - - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -162,7 +162,7 @@ - do: warnings: - - '[Common Terms Query] has been deprecated in favor of the MatchQuery [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [common] used, replaced by [[match] query which can efficiently skip blocks of documents if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -178,7 +178,7 @@ - do: warnings: - - '[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [cutoff_frequency] used, replaced by [you can omit this option, the [match] query can skip block of documents efficiently if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -194,7 +194,7 @@ - do: warnings: - - '[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [cutoff_frequency] used, replaced by [you can omit this option, the [match] query can skip block of documents efficiently if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -211,7 +211,7 @@ - do: warnings: - - '[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [cutoff_frequency] used, replaced by [you can omit this option, the [match] query can skip block of documents efficiently if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: @@ -227,7 +227,7 @@ - do: warnings: - - '[cutoff_frequency] has been deprecated in favor of the [max_score] optimization which is applied automatically without any configuration' + - 'Deprecated field [cutoff_frequency] used, replaced by [you can omit this option, the [multi_match] query can skip block of documents efficiently if the total number of hits is not tracked]' search: rest_total_hits_as_int: true body: diff --git a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java index 0ce5ae2e927e6..5b2853ac359c2 100644 --- a/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.query; -import org.apache.logging.log4j.LogManager; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.TokenStream; import org.apache.lucene.analysis.tokenattributes.CharTermAttribute; @@ -33,7 +32,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.MappedFieldType; @@ -56,11 +54,8 @@ @Deprecated public class CommonTermsQueryBuilder extends AbstractQueryBuilder { - private static final DeprecationLogger deprecationLogger = - new DeprecationLogger(LogManager.getLogger(CommonTermsQueryBuilder.class)); - - public static final String COMMON_TERMS_QUERY_DEPRECATION_MSG = "[Common Terms Query] has been deprecated in favor of the " + - "MatchQuery [max_score] optimization which is applied automatically without any configuration"; + public static final String COMMON_TERMS_QUERY_DEPRECATION_MSG = "[match] query which can efficiently " + + "skip blocks of documents if the total number of hits is not tracked"; public static final String NAME = "common"; @@ -97,9 +92,10 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder { - private static final DeprecationLogger deprecationLogger = - new DeprecationLogger(LogManager.getLogger(MatchQueryBuilder.class)); - - static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "[cutoff_frequency] has been deprecated in favor of the " + - "[max_score] optimization which is applied automatically without any configuration"; + private static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "you can omit this option, " + + "the [match] query can skip block of documents efficiently if the total number of hits is not tracked"; public static final ParseField ZERO_TERMS_QUERY_FIELD = new ParseField("zero_terms_query"); /** @@ -57,7 +52,8 @@ public class MatchQueryBuilder extends AbstractQueryBuilder { * will achieve the same result without any configuration. */ @Deprecated - public static final ParseField CUTOFF_FREQUENCY_FIELD = new ParseField("cutoff_frequency", "cutoff_frequency"); + public static final ParseField CUTOFF_FREQUENCY_FIELD = + new ParseField("cutoff_frequency").withAllDeprecated(CUTOFF_FREQUENCY_DEPRECATION_MSG); public static final ParseField LENIENT_FIELD = new ParseField("lenient"); public static final ParseField FUZZY_TRANSPOSITIONS_FIELD = new ParseField("fuzzy_transpositions"); public static final ParseField FUZZY_REWRITE_FIELD = new ParseField("fuzzy_rewrite"); @@ -254,7 +250,6 @@ public int maxExpansions() { */ @Deprecated public MatchQueryBuilder cutoffFrequency(float cutoff) { - deprecationLogger.deprecated(CUTOFF_FREQUENCY_DEPRECATION_MSG); this.cutoffFrequency = cutoff; return this; } diff --git a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java index c8ddb1cd5b138..fb400a9d3fc75 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.query; -import org.apache.logging.log4j.LogManager; import org.apache.lucene.search.FuzzyQuery; import org.apache.lucene.search.Query; import org.elasticsearch.ElasticsearchParseException; @@ -29,7 +28,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -53,11 +51,8 @@ */ public class MultiMatchQueryBuilder extends AbstractQueryBuilder { - private static final DeprecationLogger deprecationLogger = - new DeprecationLogger(LogManager.getLogger(MultiMatchQueryBuilder.class)); - - static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "[cutoff_frequency] has been deprecated in favor of the " + - "[max_score] optimization which is applied automatically without any configuration"; + private static final String CUTOFF_FREQUENCY_DEPRECATION_MSG = "you can omit this option, " + + "the [multi_match] query can skip block of documents efficiently if the total number of hits is not tracked"; public static final String NAME = "multi_match"; @@ -72,7 +67,8 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder plugins) { registerQuery(new QuerySpec<>(MoreLikeThisQueryBuilder.NAME, MoreLikeThisQueryBuilder::new, MoreLikeThisQueryBuilder::fromXContent)); registerQuery(new QuerySpec<>(WrapperQueryBuilder.NAME, WrapperQueryBuilder::new, WrapperQueryBuilder::fromXContent)); - registerQuery(new QuerySpec<>(CommonTermsQueryBuilder.NAME, CommonTermsQueryBuilder::new, CommonTermsQueryBuilder::fromXContent)); + registerQuery(new QuerySpec<>(new ParseField(CommonTermsQueryBuilder.NAME).withAllDeprecated(COMMON_TERMS_QUERY_DEPRECATION_MSG), + CommonTermsQueryBuilder::new, CommonTermsQueryBuilder::fromXContent)); registerQuery( new QuerySpec<>(SpanMultiTermQueryBuilder.NAME, SpanMultiTermQueryBuilder::new, SpanMultiTermQueryBuilder::fromXContent)); registerQuery(new QuerySpec<>(FunctionScoreQueryBuilder.NAME, FunctionScoreQueryBuilder::new, diff --git a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryBuilderTests.java index 504a85ab947fc..d02b60c52d531 100644 --- a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryBuilderTests.java @@ -25,7 +25,6 @@ import org.elasticsearch.common.ParsingException; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; -import org.junit.After; import java.io.IOException; import java.util.HashMap; @@ -40,13 +39,6 @@ public class CommonTermsQueryBuilderTests extends AbstractQueryTestCase { - @After - public void validateDeprecationMessage() { - if (getTestName().equals("testParseFailsWithMultipleFields") == false) { - assertWarnings(CommonTermsQueryBuilder.COMMON_TERMS_QUERY_DEPRECATION_MSG); - } - } - @Override protected CommonTermsQueryBuilder doCreateTestQueryBuilder() { int numberOfTerms = randomIntBetween(0, 10); @@ -119,6 +111,30 @@ protected void doAssertLuceneQuery(CommonTermsQueryBuilder queryBuilder, Query q assertThat(extendedCommonTermsQuery.getLowFreqMinimumNumberShouldMatchSpec(), equalTo(queryBuilder.lowFreqMinimumShouldMatch())); } + @Override + public void testUnknownField() throws IOException { + super.testUnknownField(); + assertDeprecationWarning(); + } + + @Override + public void testUnknownObjectException() throws IOException { + super.testUnknownObjectException(); + assertDeprecationWarning(); + } + + @Override + public void testFromXContent() throws IOException { + super.testFromXContent(); + assertDeprecationWarning(); + } + + @Override + public void testValidOutput() throws IOException { + super.testValidOutput(); + assertDeprecationWarning(); + } + public void testIllegalArguments() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new CommonTermsQueryBuilder(null, "text")); assertEquals("field name is null or empty", e.getMessage()); @@ -154,6 +170,8 @@ public void testFromJson() throws IOException { assertEquals(query, Operator.OR, queryBuilder.lowFreqOperator()); assertEquals(query, Operator.AND, queryBuilder.highFreqOperator()); assertEquals(query, "nelly the elephant not as a cartoon", queryBuilder.value()); + + assertDeprecationWarning(); } public void testCommonTermsQuery1() throws IOException { @@ -163,6 +181,8 @@ public void testCommonTermsQuery1() throws IOException { ExtendedCommonTermsQuery ectQuery = (ExtendedCommonTermsQuery) parsedQuery; assertThat(ectQuery.getHighFreqMinimumNumberShouldMatchSpec(), nullValue()); assertThat(ectQuery.getLowFreqMinimumNumberShouldMatchSpec(), equalTo("2")); + + assertDeprecationWarning(); } public void testCommonTermsQuery2() throws IOException { @@ -172,6 +192,8 @@ public void testCommonTermsQuery2() throws IOException { ExtendedCommonTermsQuery ectQuery = (ExtendedCommonTermsQuery) parsedQuery; assertThat(ectQuery.getHighFreqMinimumNumberShouldMatchSpec(), equalTo("50%")); assertThat(ectQuery.getLowFreqMinimumNumberShouldMatchSpec(), equalTo("5<20%")); + + assertDeprecationWarning(); } public void testCommonTermsQuery3() throws IOException { @@ -181,12 +203,16 @@ public void testCommonTermsQuery3() throws IOException { ExtendedCommonTermsQuery ectQuery = (ExtendedCommonTermsQuery) parsedQuery; assertThat(ectQuery.getHighFreqMinimumNumberShouldMatchSpec(), nullValue()); assertThat(ectQuery.getLowFreqMinimumNumberShouldMatchSpec(), equalTo("2")); + + assertDeprecationWarning(); } // see #11730 public void testCommonTermsQuery4() throws IOException { Query parsedQuery = parseQuery(commonTermsQuery("field", "text")).toQuery(createShardContext()); assertThat(parsedQuery, instanceOf(ExtendedCommonTermsQuery.class)); + + assertDeprecationWarning(); } public void testParseFailsWithMultipleFields() throws IOException { @@ -212,5 +238,11 @@ public void testParseFailsWithMultipleFields() throws IOException { "}"; e = expectThrows(ParsingException.class, () -> parseQuery(shortJson)); assertEquals("[common] query doesn't support multiple fields, found [message1] and [message2]", e.getMessage()); + + assertDeprecationWarning(); + } + + private void assertDeprecationWarning() { + assertWarnings("Deprecated field [common] used, replaced by [" + CommonTermsQueryBuilder.COMMON_TERMS_QUERY_DEPRECATION_MSG + "]"); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java index 19c418b4d6da5..761520de0395f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/CommonTermsQueryParserTests.java @@ -44,7 +44,5 @@ public void testWhenParsedQueryIsNullNoNullPointerExceptionIsThrown() { assertNotNull(response); assertEquals(response.getHits().getHits().length, 0); - - assertWarnings(CommonTermsQueryBuilder.COMMON_TERMS_QUERY_DEPRECATION_MSG); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java index 9b2a1e195895c..f79bbb86242d9 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java @@ -475,11 +475,6 @@ public void testMaxBooleanClause() { expectThrows(BooleanQuery.TooManyClauses.class, () -> query.parse(Type.PHRASE, STRING_FIELD_NAME, "")); } - public void testCutoffFrequency() { - new MatchQueryBuilder("field", "value").cutoffFrequency((float) 10 / randomIntBetween(1, 100)); - assertWarnings(MatchQueryBuilder.CUTOFF_FREQUENCY_DEPRECATION_MSG); - } - private static class MockGraphAnalyzer extends Analyzer { CannedBinaryTokenStream tokenStream; diff --git a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java index 2edb0088b99fe..970a4c3a37ecb 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java @@ -555,13 +555,6 @@ public void testNegativeFieldBoost() { assertThat(exc.getMessage(), containsString("negative [boost]")); } - public void testCutoffFrequency() { - new MultiMatchQueryBuilder("value", "field1", "field2").cutoffFrequency(10f / randomIntBetween(1, 100)); - assertWarnings(MultiMatchQueryBuilder.CUTOFF_FREQUENCY_DEPRECATION_MSG); - new MultiMatchQueryBuilder("value", "field1", "field2").cutoffFrequency(Float.valueOf(10f / randomIntBetween(1, 100))); - assertWarnings(MultiMatchQueryBuilder.CUTOFF_FREQUENCY_DEPRECATION_MSG); - } - private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) { Settings build = Settings.builder().put(oldIndexSettings) .put(indexSettings) diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index b7755dd321416..2e019d1e2c432 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -236,7 +236,7 @@ public Map getHighlighters() { assertSame(highlighters.get("custom"), customHighlighter); } - public void testRegisteredQueries() throws IOException { + public void testRegisteredQueries() { List allSupportedQueries = new ArrayList<>(); Collections.addAll(allSupportedQueries, NON_DEPRECATED_QUERIES); Collections.addAll(allSupportedQueries, DEPRECATED_QUERIES); @@ -244,6 +244,7 @@ public void testRegisteredQueries() throws IOException { Set registeredNonDeprecated = module.getNamedXContents().stream() .filter(e -> e.categoryClass.equals(QueryBuilder.class)) + .filter(e -> e.name.getDeprecatedNames().length == 0) .map(e -> e.name.getPreferredName()) .collect(toSet()); Set registeredAll = module.getNamedXContents().stream() @@ -306,7 +307,6 @@ public List> getRescorers() { private static final String[] NON_DEPRECATED_QUERIES = new String[] { "bool", "boosting", - "common", "constant_score", "dis_max", "exists", @@ -354,7 +354,7 @@ public List> getRescorers() { }; //add here deprecated queries to make sure we log a deprecation warnings when they are used - private static final String[] DEPRECATED_QUERIES = new String[] {}; + private static final String[] DEPRECATED_QUERIES = new String[] {"common"}; /** * Dummy test {@link AggregationBuilder} used to test registering aggregation builders. diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 54f6df6ac0cca..847fb58eca3e7 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -20,7 +20,6 @@ package org.elasticsearch.test; import com.fasterxml.jackson.core.io.JsonStringEncoder; - import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; @@ -165,7 +164,7 @@ public void testUnknownField() throws IOException { * parse exception. Some specific objects do not cause any exception as they can hold arbitrary content; they can be * declared by overriding {@link #getObjectsHoldingArbitraryContent()}. */ - public final void testUnknownObjectException() throws IOException { + public void testUnknownObjectException() throws IOException { Set candidates = new HashSet<>(); // Adds the valid query to the list of queries to modify and test candidates.add(createTestQueryBuilder().toString()); From 9df9deb3a71431f58906adc108798b8b75146bb2 Mon Sep 17 00:00:00 2001 From: lcawl Date: Wed, 29 May 2019 09:45:39 -0700 Subject: [PATCH 09/10] [DOCS] Fixes deprecation link in Common Terms Query --- docs/reference/query-dsl/common-terms-query.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/query-dsl/common-terms-query.asciidoc b/docs/reference/query-dsl/common-terms-query.asciidoc index 5fb6145f40dbb..99e2e5aaf3ace 100644 --- a/docs/reference/query-dsl/common-terms-query.asciidoc +++ b/docs/reference/query-dsl/common-terms-query.asciidoc @@ -1,7 +1,7 @@ [[query-dsl-common-terms-query]] === Common Terms Query -deprecated[7.3.0,`<>` which can efficiently skip blocks of documents if the total number of hits is not tracked] +deprecated[7.3.0,"Use <>, which can efficiently skip blocks of documents if the total number of hits is not tracked."] The `common` terms query is a modern alternative to stopwords which improves the precision and recall of search results (by taking stopwords From 4eb1045cc980bae0e8465f2f695e7141472de3b7 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Wed, 29 May 2019 18:54:35 +0200 Subject: [PATCH 10/10] Fix docs & tests --- .../client/documentation/QueryDSLDocumentationTests.java | 2 -- docs/reference/query-dsl/common-terms-query.asciidoc | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java index 60b13d9919197..51670b29de1b6 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java @@ -25,7 +25,6 @@ import org.elasticsearch.common.geo.builders.CoordinatesBuilder; import org.elasticsearch.common.geo.builders.MultiPointBuilder; import org.elasticsearch.common.unit.DistanceUnit; -import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.GeoShapeQueryBuilder; import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder; import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder.FilterFunctionBuilder; @@ -112,7 +111,6 @@ public void testCommonTerms() { commonTermsQuery("name", // <1> "kimchy"); // <2> // end::common_terms - assertWarnings(CommonTermsQueryBuilder.COMMON_TERMS_QUERY_DEPRECATION_MSG); } public void testConstantScore() { diff --git a/docs/reference/query-dsl/common-terms-query.asciidoc b/docs/reference/query-dsl/common-terms-query.asciidoc index 99e2e5aaf3ace..f2d784eb0c4c9 100644 --- a/docs/reference/query-dsl/common-terms-query.asciidoc +++ b/docs/reference/query-dsl/common-terms-query.asciidoc @@ -1,7 +1,7 @@ [[query-dsl-common-terms-query]] === Common Terms Query -deprecated[7.3.0,"Use <>, which can efficiently skip blocks of documents if the total number of hits is not tracked."] +deprecated[7.3.0,"Use <> instead, which skips blocks of documents efficiently, without any configuration, provided that the total number of hits is not tracked."] The `common` terms query is a modern alternative to stopwords which improves the precision and recall of search results (by taking stopwords