Skip to content

Commit addfb26

Browse files
mridula-s109Copilot
authored andcommitted
Fix: Allow non-score sorts in pinned retriever sub-retrievers (elastic#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>
1 parent 8ce42b8 commit addfb26

File tree

3 files changed

+25
-13
lines changed

3 files changed

+25
-13
lines changed

docs/changelog/128323.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 128323
2+
summary: "Fix: Allow non-score secondary sorts in pinned retriever sub-retrievers"
3+
area: Relevance
4+
type: bug
5+
issues: []

x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
import org.elasticsearch.search.retriever.RetrieverBuilder;
1919
import org.elasticsearch.search.retriever.RetrieverBuilderWrapper;
2020
import org.elasticsearch.search.retriever.RetrieverParserContext;
21-
import org.elasticsearch.search.sort.FieldSortBuilder;
2221
import org.elasticsearch.search.sort.ScoreSortBuilder;
23-
import org.elasticsearch.search.sort.ShardDocSortField;
2422
import org.elasticsearch.search.sort.SortBuilder;
2523
import org.elasticsearch.xcontent.ConstructingObjectParser;
2624
import org.elasticsearch.xcontent.ParseField;
@@ -106,16 +104,9 @@ private void validateSort(SearchSourceBuilder source) {
106104
if (sorts == null || sorts.isEmpty()) {
107105
return;
108106
}
109-
for (SortBuilder<?> sort : sorts) {
110-
if (sort instanceof ScoreSortBuilder) {
111-
continue;
112-
}
113-
if (sort instanceof FieldSortBuilder) {
114-
FieldSortBuilder fieldSort = (FieldSortBuilder) sort;
115-
if (ShardDocSortField.NAME.equals(fieldSort.getFieldName())) {
116-
continue;
117-
}
118-
}
107+
108+
SortBuilder<?> sort = sorts.get(0);
109+
if (sort instanceof ScoreSortBuilder == false) {
119110
throw new IllegalArgumentException(
120111
"[" + NAME + "] retriever only supports sorting by score, invalid sort criterion: " + sort.toString()
121112
);

x-pack/plugin/search-business-rules/src/test/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilderTests.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
import org.elasticsearch.search.retriever.RetrieverBuilder;
1717
import org.elasticsearch.search.retriever.RetrieverParserContext;
1818
import org.elasticsearch.search.retriever.TestRetrieverBuilder;
19+
import org.elasticsearch.search.sort.FieldSortBuilder;
20+
import org.elasticsearch.search.sort.ScoreSortBuilder;
21+
import org.elasticsearch.search.sort.SortOrder;
1922
import org.elasticsearch.test.AbstractXContentTestCase;
2023
import org.elasticsearch.usage.SearchUsage;
2124
import org.elasticsearch.usage.SearchUsageHolder;
@@ -209,7 +212,20 @@ public void testValidateSort() {
209212
multipleSortsSource.query(dummyQuery);
210213
multipleSortsSource.sort("_score");
211214
multipleSortsSource.sort("field1");
212-
e = expectThrows(IllegalArgumentException.class, () -> builder.finalizeSourceBuilder(multipleSortsSource));
215+
builder.finalizeSourceBuilder(multipleSortsSource);
216+
217+
assertThat(multipleSortsSource.sorts().size(), equalTo(2));
218+
assertThat(multipleSortsSource.sorts().get(0), instanceOf(ScoreSortBuilder.class));
219+
assertThat(((ScoreSortBuilder) multipleSortsSource.sorts().get(0)).order(), equalTo(SortOrder.DESC));
220+
assertThat(multipleSortsSource.sorts().get(1), instanceOf(FieldSortBuilder.class));
221+
assertThat(((FieldSortBuilder) multipleSortsSource.sorts().get(1)).getFieldName(), equalTo("field1"));
222+
assertThat(((FieldSortBuilder) multipleSortsSource.sorts().get(1)).order(), equalTo(SortOrder.ASC));
223+
224+
SearchSourceBuilder fieldFirstSource = new SearchSourceBuilder();
225+
fieldFirstSource.query(dummyQuery);
226+
fieldFirstSource.sort("field1");
227+
fieldFirstSource.sort("_score");
228+
e = expectThrows(IllegalArgumentException.class, () -> builder.finalizeSourceBuilder(fieldFirstSource));
213229
assertThat(
214230
e.getMessage(),
215231
equalTo(

0 commit comments

Comments
 (0)