From e79127ba2a0b47f8681bf6e41514ad3af2cc793c Mon Sep 17 00:00:00 2001 From: Panagiotis Bailis Date: Wed, 16 Oct 2024 21:56:13 +0300 Subject: [PATCH] Adding deprecation warnings for rank and sub_searches (#114854) --- docs/changelog/114854.yaml | 10 +++ ...ulkByScrollParallelizationHelperTests.java | 4 +- .../search/builder/SearchSourceBuilder.java | 6 +- .../search/AbstractSearchTestCase.java | 1 - .../vectors/KnnSearchRequestParserTests.java | 1 - .../search/RandomSearchRequestGenerator.java | 17 ------ .../xpack/rank/rrf/RRFRankBuilder.java | 7 ++- .../xpack/rank/rrf/RRFRankDoc.java | 2 +- .../xpack/rank/rrf/RRFRetrieverBuilder.java | 7 ++- .../rrf/RRFRetrieverBuilderParsingTests.java | 2 +- .../rest-api-spec/test/rrf/100_rank_rrf.yml | 25 ++++++++ .../test/rrf/150_rank_rrf_pagination.yml | 61 +++++++++++++++++++ .../test/rrf/200_rank_rrf_script.yml | 21 +++++++ .../test/rrf/550_rrf_sub_searches_explain.yml | 38 ++++++++++++ .../test/rrf/600_rrf_retriever_profile.yml | 7 +++ 15 files changed, 178 insertions(+), 31 deletions(-) create mode 100644 docs/changelog/114854.yaml diff --git a/docs/changelog/114854.yaml b/docs/changelog/114854.yaml new file mode 100644 index 000000000000..144a10ba8504 --- /dev/null +++ b/docs/changelog/114854.yaml @@ -0,0 +1,10 @@ +pr: 114854 +summary: Adding deprecation warnings for rrf using rank and `sub_searches` +area: Search +type: deprecation +issues: [] +deprecation: + title: Adding deprecation warnings for rrf using rank and `sub_searches` + area: REST API + details: Search API parameter `sub_searches` will no longer be a supported and will be removed in future releases. Similarly, `rrf` can only be used through the specified `retriever` and no longer though the `rank` parameter + impact: Requests specifying rrf through `rank` and/or `sub_searches` elements will be disallowed in a future version. Users should instead utilize the new `retriever` parameter. diff --git a/modules/reindex/src/test/java/org/elasticsearch/reindex/BulkByScrollParallelizationHelperTests.java b/modules/reindex/src/test/java/org/elasticsearch/reindex/BulkByScrollParallelizationHelperTests.java index a6e28477f858..ebb4471566fb 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/reindex/BulkByScrollParallelizationHelperTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/reindex/BulkByScrollParallelizationHelperTests.java @@ -15,8 +15,8 @@ import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.test.ESTestCase; import java.io.IOException; +import java.util.Collections; -import static java.util.Collections.emptyList; import static org.elasticsearch.reindex.BulkByScrollParallelizationHelper.sliceIntoSubRequests; import static org.elasticsearch.search.RandomSearchRequestGenerator.randomSearchRequest; import static org.elasticsearch.search.RandomSearchRequestGenerator.randomSearchSourceBuilder; @@ -24,7 +24,7 @@ import static org.elasticsearch.search.RandomSearchRequestGenerator.randomSearch public class BulkByScrollParallelizationHelperTests extends ESTestCase { public void testSliceIntoSubRequests() throws IOException { SearchRequest searchRequest = randomSearchRequest( - () -> randomSearchSourceBuilder(() -> null, () -> null, () -> null, () -> null, () -> emptyList(), () -> null, () -> null) + () -> randomSearchSourceBuilder(() -> null, () -> null, () -> null, Collections::emptyList, () -> null, () -> null) ); if (searchRequest.source() != null) { // Clear the slice builder if there is one set. We can't call sliceIntoSubRequests if it is. diff --git a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index 6d427aace51d..6ceb02f0e797 100644 --- a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.Booleans; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.core.UpdateForV10; import org.elasticsearch.features.NodeFeature; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; @@ -98,10 +99,11 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R public static final ParseField TIMEOUT_FIELD = new ParseField("timeout"); public static final ParseField TERMINATE_AFTER_FIELD = new ParseField("terminate_after"); public static final ParseField QUERY_FIELD = new ParseField("query"); - public static final ParseField SUB_SEARCHES_FIELD = new ParseField("sub_searches"); + @UpdateForV10(owner = UpdateForV10.Owner.SEARCH_RELEVANCE) // remove [sub_searches] and [rank] support in 10.0 + public static final ParseField SUB_SEARCHES_FIELD = new ParseField("sub_searches").withAllDeprecated("retriever"); + public static final ParseField RANK_FIELD = new ParseField("rank").withAllDeprecated("retriever"); public static final ParseField POST_FILTER_FIELD = new ParseField("post_filter"); public static final ParseField KNN_FIELD = new ParseField("knn"); - public static final ParseField RANK_FIELD = new ParseField("rank"); public static final ParseField MIN_SCORE_FIELD = new ParseField("min_score"); public static final ParseField VERSION_FIELD = new ParseField("version"); public static final ParseField SEQ_NO_PRIMARY_TERM_FIELD = new ParseField("seq_no_primary_term"); diff --git a/server/src/test/java/org/elasticsearch/search/AbstractSearchTestCase.java b/server/src/test/java/org/elasticsearch/search/AbstractSearchTestCase.java index b716f11b5fff..88c83df3e20f 100644 --- a/server/src/test/java/org/elasticsearch/search/AbstractSearchTestCase.java +++ b/server/src/test/java/org/elasticsearch/search/AbstractSearchTestCase.java @@ -89,7 +89,6 @@ public abstract class AbstractSearchTestCase extends ESTestCase { return RandomSearchRequestGenerator.randomSearchSourceBuilder( HighlightBuilderTests::randomHighlighterBuilder, SuggestBuilderTests::randomSuggestBuilder, - TestRankBuilder::randomRankBuilder, QueryRescorerBuilderTests::randomRescoreBuilder, randomExtBuilders, CollapseBuilderTests::randomCollapseBuilder, diff --git a/server/src/test/java/org/elasticsearch/search/vectors/KnnSearchRequestParserTests.java b/server/src/test/java/org/elasticsearch/search/vectors/KnnSearchRequestParserTests.java index d9fe421bafb4..4e4d2158a957 100644 --- a/server/src/test/java/org/elasticsearch/search/vectors/KnnSearchRequestParserTests.java +++ b/server/src/test/java/org/elasticsearch/search/vectors/KnnSearchRequestParserTests.java @@ -74,7 +74,6 @@ public class KnnSearchRequestParserTests extends ESTestCase { () -> null, () -> null, () -> null, - () -> null, Collections::emptyList, () -> null, () -> null diff --git a/test/framework/src/main/java/org/elasticsearch/search/RandomSearchRequestGenerator.java b/test/framework/src/main/java/org/elasticsearch/search/RandomSearchRequestGenerator.java index b59f1a5e5f02..363d34ca3ff8 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/RandomSearchRequestGenerator.java +++ b/test/framework/src/main/java/org/elasticsearch/search/RandomSearchRequestGenerator.java @@ -23,13 +23,11 @@ import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.builder.PointInTimeBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; -import org.elasticsearch.search.builder.SubSearchSourceBuilder; import org.elasticsearch.search.collapse.CollapseBuilder; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.FieldAndFormat; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.search.rank.RankBuilder; import org.elasticsearch.search.rescore.RescorerBuilder; import org.elasticsearch.search.searchafter.SearchAfterBuilder; import org.elasticsearch.search.slice.SliceBuilder; @@ -122,7 +120,6 @@ public class RandomSearchRequestGenerator { public static SearchSourceBuilder randomSearchSourceBuilder( Supplier randomHighlightBuilder, Supplier randomSuggestBuilder, - Supplier rankContextBuilderSupplier, Supplier> randomRescoreBuilder, Supplier> randomExtBuilders, Supplier randomCollapseBuilder, @@ -250,17 +247,6 @@ public class RandomSearchRequestGenerator { } if (randomBoolean()) { builder.query(QueryBuilders.termQuery(randomAlphaOfLengthBetween(5, 20), randomAlphaOfLengthBetween(5, 20))); - } else if (randomBoolean()) { - builder.subSearches( - List.of( - new SubSearchSourceBuilder( - QueryBuilders.termQuery(randomAlphaOfLengthBetween(5, 20), randomAlphaOfLengthBetween(5, 20)) - ), - new SubSearchSourceBuilder( - QueryBuilders.termQuery(randomAlphaOfLengthBetween(5, 20), randomAlphaOfLengthBetween(5, 20)) - ) - ) - ); } if (randomBoolean()) { builder.postFilter(QueryBuilders.termQuery(randomAlphaOfLengthBetween(5, 20), randomAlphaOfLengthBetween(5, 20))); @@ -354,9 +340,6 @@ public class RandomSearchRequestGenerator { if (randomBoolean()) { builder.suggest(randomSuggestBuilder.get()); } - if (randomBoolean()) { - builder.rankBuilder(rankContextBuilderSupplier.get()); - } if (randomBoolean()) { int numRescores = randomIntBetween(1, 5); for (int i = 0; i < numRescores; i++) { diff --git a/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRankBuilder.java b/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRankBuilder.java index 10aff2f4d68c..fb20f834937d 100644 --- a/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRankBuilder.java +++ b/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRankBuilder.java @@ -36,16 +36,17 @@ import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstr /** * The builder to support RRF. Adds user-defined parameters for window size and rank constant. + * + * @deprecated RRF support is provided through the retriever framework. Please use {@link RRFRetrieverBuilder instead} */ +@Deprecated public class RRFRankBuilder extends RankBuilder { - public static final int DEFAULT_RANK_CONSTANT = 60; - public static final ParseField RANK_CONSTANT_FIELD = new ParseField("rank_constant"); static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(RRFRankPlugin.NAME, args -> { int windowSize = args[0] == null ? DEFAULT_RANK_WINDOW_SIZE : (int) args[0]; - int rankConstant = args[1] == null ? DEFAULT_RANK_CONSTANT : (int) args[1]; + int rankConstant = args[1] == null ? RRFRetrieverBuilder.DEFAULT_RANK_CONSTANT : (int) args[1]; return new RRFRankBuilder(windowSize, rankConstant); }); diff --git a/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRankDoc.java b/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRankDoc.java index 272df248e53e..4cd10801b298 100644 --- a/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRankDoc.java +++ b/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRankDoc.java @@ -19,7 +19,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Objects; -import static org.elasticsearch.xpack.rank.rrf.RRFRankBuilder.DEFAULT_RANK_CONSTANT; +import static org.elasticsearch.xpack.rank.rrf.RRFRetrieverBuilder.DEFAULT_RANK_CONSTANT; /** * {@code RRFRankDoc} supports additional ranking information diff --git a/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java b/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java index 12c43a2f169f..c3c9f19cde6e 100644 --- a/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java +++ b/x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java @@ -12,6 +12,7 @@ import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.util.Maps; import org.elasticsearch.features.NodeFeature; import org.elasticsearch.license.LicenseUtils; +import org.elasticsearch.search.rank.RankBuilder; import org.elasticsearch.search.rank.RankDoc; import org.elasticsearch.search.retriever.CompoundRetrieverBuilder; import org.elasticsearch.search.retriever.RetrieverBuilder; @@ -31,7 +32,6 @@ import java.util.Objects; import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg; import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; -import static org.elasticsearch.xpack.rank.rrf.RRFRankPlugin.NAME; /** * An rrf retriever is used to represent an rrf rank element, but @@ -50,6 +50,7 @@ public final class RRFRetrieverBuilder extends CompoundRetrieverBuilder PARSER = new ConstructingObjectParser<>( NAME, @@ -57,8 +58,8 @@ public final class RRFRetrieverBuilder extends CompoundRetrieverBuilder { List childRetrievers = (List) args[0]; List innerRetrievers = childRetrievers.stream().map(r -> new RetrieverSource(r, null)).toList(); - int rankWindowSize = args[1] == null ? RRFRankBuilder.DEFAULT_RANK_WINDOW_SIZE : (int) args[1]; - int rankConstant = args[2] == null ? RRFRankBuilder.DEFAULT_RANK_CONSTANT : (int) args[2]; + int rankWindowSize = args[1] == null ? RankBuilder.DEFAULT_RANK_WINDOW_SIZE : (int) args[1]; + int rankConstant = args[2] == null ? DEFAULT_RANK_CONSTANT : (int) args[2]; return new RRFRetrieverBuilder(innerRetrievers, rankWindowSize, rankConstant); } ); diff --git a/x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderParsingTests.java b/x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderParsingTests.java index d324effe41c2..cae758457a2a 100644 --- a/x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderParsingTests.java +++ b/x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderParsingTests.java @@ -41,7 +41,7 @@ public class RRFRetrieverBuilderParsingTests extends AbstractXContentTestCase