From cc461afa0a1130d474ff23ac42c4c8030f90f09d Mon Sep 17 00:00:00 2001 From: Mridula Date: Fri, 30 May 2025 08:01:01 +0100 Subject: [PATCH] Fix: Allow non-score sorts in pinned retriever sub-retrievers (#128323) * Fix scoring and sort handling in pinned retriever * Remove books.es from version control and add to .gitignore * Remove books.es from version control and add to .gitignore * Remove books.es entries from .gitignore * fixed the mess * Update x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilderTests.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- docs/changelog/128323.yaml | 5 +++++ .../retriever/PinnedRetrieverBuilder.java | 15 +++------------ .../retriever/PinnedRetrieverBuilderTests.java | 18 +++++++++++++++++- 3 files changed, 25 insertions(+), 13 deletions(-) create mode 100644 docs/changelog/128323.yaml diff --git a/docs/changelog/128323.yaml b/docs/changelog/128323.yaml new file mode 100644 index 000000000000..b6114c26ddc6 --- /dev/null +++ b/docs/changelog/128323.yaml @@ -0,0 +1,5 @@ +pr: 128323 +summary: "Fix: Allow non-score secondary sorts in pinned retriever sub-retrievers" +area: Relevance +type: bug +issues: [] diff --git a/x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java b/x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java index 60ac7019d6a9..0e254b4fcd5b 100644 --- a/x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java +++ b/x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java @@ -18,9 +18,7 @@ import org.elasticsearch.search.retriever.CompoundRetrieverBuilder; import org.elasticsearch.search.retriever.RetrieverBuilder; import org.elasticsearch.search.retriever.RetrieverBuilderWrapper; import org.elasticsearch.search.retriever.RetrieverParserContext; -import org.elasticsearch.search.sort.FieldSortBuilder; import org.elasticsearch.search.sort.ScoreSortBuilder; -import org.elasticsearch.search.sort.ShardDocSortField; import org.elasticsearch.search.sort.SortBuilder; import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.ParseField; @@ -106,16 +104,9 @@ public final class PinnedRetrieverBuilder extends CompoundRetrieverBuilder sort : sorts) { - if (sort instanceof ScoreSortBuilder) { - continue; - } - if (sort instanceof FieldSortBuilder) { - FieldSortBuilder fieldSort = (FieldSortBuilder) sort; - if (ShardDocSortField.NAME.equals(fieldSort.getFieldName())) { - continue; - } - } + + SortBuilder sort = sorts.get(0); + if (sort instanceof ScoreSortBuilder == false) { throw new IllegalArgumentException( "[" + NAME + "] retriever only supports sorting by score, invalid sort criterion: " + sort.toString() ); diff --git a/x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilderTests.java b/x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilderTests.java index 38a2299ca26f..0228a54320d8 100644 --- a/x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilderTests.java +++ b/x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilderTests.java @@ -16,6 +16,9 @@ import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.retriever.RetrieverBuilder; import org.elasticsearch.search.retriever.RetrieverParserContext; import org.elasticsearch.search.retriever.TestRetrieverBuilder; +import org.elasticsearch.search.sort.FieldSortBuilder; +import org.elasticsearch.search.sort.ScoreSortBuilder; +import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.AbstractXContentTestCase; import org.elasticsearch.usage.SearchUsage; import org.elasticsearch.usage.SearchUsageHolder; @@ -209,7 +212,20 @@ public class PinnedRetrieverBuilderTests extends AbstractXContentTestCase builder.finalizeSourceBuilder(multipleSortsSource)); + builder.finalizeSourceBuilder(multipleSortsSource); + + assertThat(multipleSortsSource.sorts().size(), equalTo(2)); + assertThat(multipleSortsSource.sorts().get(0), instanceOf(ScoreSortBuilder.class)); + assertThat(((ScoreSortBuilder) multipleSortsSource.sorts().get(0)).order(), equalTo(SortOrder.DESC)); + assertThat(multipleSortsSource.sorts().get(1), instanceOf(FieldSortBuilder.class)); + assertThat(((FieldSortBuilder) multipleSortsSource.sorts().get(1)).getFieldName(), equalTo("field1")); + assertThat(((FieldSortBuilder) multipleSortsSource.sorts().get(1)).order(), equalTo(SortOrder.ASC)); + + SearchSourceBuilder fieldFirstSource = new SearchSourceBuilder(); + fieldFirstSource.query(dummyQuery); + fieldFirstSource.sort("field1"); + fieldFirstSource.sort("_score"); + e = expectThrows(IllegalArgumentException.class, () -> builder.finalizeSourceBuilder(fieldFirstSource)); assertThat( e.getMessage(), equalTo(