Fix Bug in RankDocRetrieverBuilder when `from` is set to Default (-1) by markjhoy · Pull Request #137637 · elastic/elasticsearch

markjhoy · 2025-11-05T16:54:36Z

Fixes a bug found in the RankDocRetrieverBuilder when the default from value for the query is set to -1 (the default).

In the RankDocRetrieverBuilder, when performing the extractToSearchSourceBuilder, if the expected size + the offset (from) is greater than the number of docs, then it sets the limit to the number of docs minus the from value. Without this fix, if the default is used (-1), then the expected number of docs could have an additional document in the result set.

elasticsearchmachine · 2025-11-05T16:55:02Z

Pinging @elastic/search-relevance (Team:Search - Relevance)

elasticsearchmachine · 2025-11-05T16:55:02Z

Hi @markjhoy, I've created a changelog YAML for you.

mridula-s109

LGTM!

markjhoy · 2025-11-05T17:16:56Z

buildkite test this

kderusso

Thanks for spinning up this fix so quickly! I have one suggestion, otherwise it LGTM.

server/src/main/java/org/elasticsearch/search/retriever/RankDocsRetrieverBuilder.java

...rc/yamlRestTest/resources/rest-api-spec/test/inference/70_text_similarity_rank_retriever.yml

pmpailis

LGTM

…_from_size

...rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java

docs/changelog/137637.yaml

...rc/yamlRestTest/resources/rest-api-spec/test/inference/70_text_similarity_rank_retriever.yml

...rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java

Mikep86

LGTM, except for the commented out code

...rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java

Mikep86 · 2025-11-06T21:44:53Z

I think we should also backport this to 8.19?

…_from_size

…elastic#137637) * correct `from` in extractToSearchSourceBuilder * Update docs/changelog/137637.yaml * fix tests; simplify from check * revert auto-formatting * fix failing test; additional tests; changelog area * remove comment * [CI] Auto commit changes from spotless * cleanup unit and YAML tests * [CI] Auto commit changes from spotless * remove old, commented code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

elasticsearchmachine · 2025-11-07T00:37:35Z

💔 Backport failed

Status	Branch	Result
✅	9.2
❌	8.19	Commit could not be cherrypicked due to conflicts
❌	9.1	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 137637

…elastic#137637) * correct `from` in extractToSearchSourceBuilder * Update docs/changelog/137637.yaml * fix tests; simplify from check * revert auto-formatting * fix failing test; additional tests; changelog area * remove comment * [CI] Auto commit changes from spotless * cleanup unit and YAML tests * [CI] Auto commit changes from spotless * remove old, commented code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> (cherry picked from commit 2e297b9) # Conflicts: # x-pack/plugin/rank-rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java

markjhoy · 2025-11-07T00:42:04Z

💚 All backports created successfully

Status	Branch	Result
✅	9.1
✅	8.19

Questions ?

Please refer to the Backport tool documentation

…#137637) (#137708) * correct `from` in extractToSearchSourceBuilder * Update docs/changelog/137637.yaml * fix tests; simplify from check * revert auto-formatting * fix failing test; additional tests; changelog area * remove comment * [CI] Auto commit changes from spotless * cleanup unit and YAML tests * [CI] Auto commit changes from spotless * remove old, commented code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

…t (-1) (#137637) (#137709) * Fix Bug in RankDocRetrieverBuilder when `from` is set to Default (-1) (#137637) * correct `from` in extractToSearchSourceBuilder * Update docs/changelog/137637.yaml * fix tests; simplify from check * revert auto-formatting * fix failing test; additional tests; changelog area * remove comment * [CI] Auto commit changes from spotless * cleanup unit and YAML tests * [CI] Auto commit changes from spotless * remove old, commented code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> (cherry picked from commit 2e297b9) # Conflicts: # x-pack/plugin/rank-rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java * Fix failing test from backport * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

…lt (-1) (#137637) (#137710) * Fix Bug in RankDocRetrieverBuilder when `from` is set to Default (-1) (#137637) * correct `from` in extractToSearchSourceBuilder * Update docs/changelog/137637.yaml * fix tests; simplify from check * revert auto-formatting * fix failing test; additional tests; changelog area * remove comment * [CI] Auto commit changes from spotless * cleanup unit and YAML tests * [CI] Auto commit changes from spotless * remove old, commented code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> (cherry picked from commit 2e297b9) # Conflicts: # x-pack/plugin/rank-rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java * Fix failing test from backport * [CI] Auto commit changes from spotless * Fix backport compilation errors --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

…elastic#137637) * correct `from` in extractToSearchSourceBuilder * Update docs/changelog/137637.yaml * fix tests; simplify from check * revert auto-formatting * fix failing test; additional tests; changelog area * remove comment * [CI] Auto commit changes from spotless * cleanup unit and YAML tests * [CI] Auto commit changes from spotless * remove old, commented code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

correct from in extractToSearchSourceBuilder

635bb9c

markjhoy requested review from a team and pmpailis November 5, 2025 16:54

markjhoy added >bug auto-backport Automatically create backport pull requests when merged :SearchOrg/Relevance Label for the Search (solution/org) Relevance team v9.3.0 v9.2.2 v9.1.8 labels Nov 5, 2025

elasticsearchmachine added the Team:Search - Relevance The Search organization Search Relevance team label Nov 5, 2025

Update docs/changelog/137637.yaml

b39ba2f

mridula-s109 approved these changes Nov 5, 2025

View reviewed changes

markjhoy requested a review from a team November 5, 2025 17:26

markjhoy mentioned this pull request Nov 5, 2025

Adds Internal mechanisms and retriever for MMR based result diversification #135880

Merged

kderusso approved these changes Nov 5, 2025

View reviewed changes

server/src/main/java/org/elasticsearch/search/retriever/RankDocsRetrieverBuilder.java Outdated Show resolved Hide resolved

fix tests; simplify from check

359ffdc

pmpailis reviewed Nov 6, 2025

View reviewed changes

...rc/yamlRestTest/resources/rest-api-spec/test/inference/70_text_similarity_rank_retriever.yml Outdated Show resolved Hide resolved

revert auto-formatting

b00fc52

pmpailis approved these changes Nov 6, 2025

View reviewed changes

Merge branch 'main' into markjhoy/fix_bug_rank_docs_retriever_builder…

7bf6e2d

…_from_size

pmpailis reviewed Nov 6, 2025

View reviewed changes

...rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java Outdated Show resolved Hide resolved

Mikep86 reviewed Nov 6, 2025

View reviewed changes

docs/changelog/137637.yaml Outdated Show resolved Hide resolved

...rc/yamlRestTest/resources/rest-api-spec/test/inference/70_text_similarity_rank_retriever.yml Outdated Show resolved Hide resolved

fix failing test; additional tests; changelog area

2491054

markjhoy requested review from Mikep86 and pmpailis November 6, 2025 18:23

markjhoy and others added 2 commits November 6, 2025 13:23

remove comment

29b1197

[CI] Auto commit changes from spotless

d027200

Mikep86 reviewed Nov 6, 2025

View reviewed changes

cleanup unit and YAML tests

12420c4

markjhoy requested a review from Mikep86 November 6, 2025 21:21

[CI] Auto commit changes from spotless

bccb08a

Mikep86 approved these changes Nov 6, 2025

View reviewed changes

...rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java Outdated Show resolved Hide resolved

remove old, commented code

e5ac4f4

markjhoy added the v8.19.8 label Nov 6, 2025

Merge branch 'main' into markjhoy/fix_bug_rank_docs_retriever_builder…

600332a

…_from_size

markjhoy merged commit 2e297b9 into elastic:main Nov 7, 2025
34 checks passed

markjhoy mentioned this pull request Nov 7, 2025

[9.2] Fix Bug in RankDocRetrieverBuilder when from is set to Default (-1) (#137637) #137708

Merged

elasticsearchmachine added the backport pending label Nov 7, 2025

markjhoy mentioned this pull request Nov 7, 2025

[9.1] Fix Bug in RankDocRetrieverBuilder when from is set to Default (-1) (#137637) #137709

Merged

markjhoy mentioned this pull request Nov 7, 2025

[8.19] Fix Bug in RankDocRetrieverBuilder when from is set to Default (-1) (#137637) #137710

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Bug in RankDocRetrieverBuilder when `from` is set to Default (-1)#137637

Fix Bug in RankDocRetrieverBuilder when `from` is set to Default (-1)#137637
markjhoy merged 12 commits intoelastic:mainfrom
markjhoy:markjhoy/fix_bug_rank_docs_retriever_builder_from_size

markjhoy commented Nov 5, 2025

elasticsearchmachine commented Nov 5, 2025

elasticsearchmachine commented Nov 5, 2025

mridula-s109 left a comment

markjhoy commented Nov 5, 2025

kderusso left a comment

Uh oh!

Uh oh!

pmpailis left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mikep86 left a comment

Uh oh!

Mikep86 commented Nov 6, 2025

Uh oh!

elasticsearchmachine commented Nov 7, 2025

markjhoy commented Nov 7, 2025

Labels

6 participants

Conversation

markjhoy commented Nov 5, 2025

elasticsearchmachine commented Nov 5, 2025

elasticsearchmachine commented Nov 5, 2025

mridula-s109 left a comment

Choose a reason for hiding this comment

markjhoy commented Nov 5, 2025

kderusso left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pmpailis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mikep86 left a comment

Choose a reason for hiding this comment

Uh oh!

Mikep86 commented Nov 6, 2025

Uh oh!

elasticsearchmachine commented Nov 7, 2025

💔 Backport failed

markjhoy commented Nov 7, 2025

💚 All backports created successfully

Questions ?

Labels

6 participants