Skip to content

Fix: Allow non-score sorts in pinned retriever sub-retrievers #128323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
May 30, 2025
Merged

Fix: Allow non-score sorts in pinned retriever sub-retrievers #128323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3b7077c
Fix scoring and sort handling in pinned retriever
mridula-s109 May 22, 2025
d6e2c22
Remove books.es from version control and add to .gitignore
mridula-s109 May 22, 2025
5ea054c
Remove books.es from version control and add to .gitignore
mridula-s109 May 22, 2025
0c7e5a9
Remove books.es entries from .gitignore
mridula-s109 May 22, 2025
4686f62
fixed the mess
mridula-s109 May 22, 2025
d3eafef
Update x-pack/plugin/search-business-rules/src/test/java/org/elastics…
mridula-s109 May 22, 2025
0a81ba5
Update x-pack/plugin/search-business-rules/src/main/java/org/elastics…
mridula-s109 May 22, 2025
87c0673
Update docs/changelog/128323.yaml
mridula-s109 May 22, 2025
56dde7e
Update docs/changelog/128323.yaml
mridula-s109 May 23, 2025
3e19115
Update PinnedRetrieverBuilder.java
mridula-s109 May 23, 2025
3897e24
Merge branch 'main' into SEARCH-1007-pinned-retriever-errors-if-sub-r…
mridula-s109 May 28, 2025
22e9f9f
Fixed the PinnedRetrieverBuilderTests
mridula-s109 May 29, 2025
1bf9981
Spotless and checkstyle fix
mridula-s109 May 29, 2025
83ecb17
Merge branch 'main' into SEARCH-1007-pinned-retriever-errors-if-sub-r…
mridula-s109 May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/128323.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 128323
summary: "Fix: Allow non-score secondary sorts in pinned retriever sub-retrievers"
area: Relevance
type: bug
issues: []
15 changes: 3 additions & 12 deletions ...in/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@
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;
Expand Down Expand Up @@ -106,16 +104,9 @@ private void validateSort(SearchSourceBuilder source) {
if (sorts == null || sorts.isEmpty()) {
return;
}
for (SortBuilder<?> 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()
);
Expand Down
18 changes: 17 additions & 1 deletion ...va/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
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;
Expand Down Expand Up @@ -209,7 +212,20 @@ public void testValidateSort() {
multipleSortsSource.query(dummyQuery);
multipleSortsSource.sort("_score");
multipleSortsSource.sort("field1");
e = expectThrows(IllegalArgumentException.class, () -> 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(
Expand Down
Loading