-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Retrievers - Fix rank_window_size validation #128108
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.
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
Conversation
Pinging @elastic/es-search (Team:Search) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for fix! Left one non-blocking nit.
...plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(cherry picked from commit e9a8efe) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java # x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java
(cherry picked from commit e9a8efe) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java
(cherry picked from commit e9a8efe) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java # x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
* Retrievers - Fix rank_window_size validation (#128108) (cherry picked from commit e9a8efe) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java # x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java * Fix build error --------- Co-authored-by: Ioana Tagirta <[email protected]>
* Retrievers - Fix rank_window_size validation (#128108) (cherry picked from commit e9a8efe) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java * Fix build error --------- Co-authored-by: Ioana Tagirta <[email protected]>
* Retrievers - Fix rank_window_size validation (#128108) (cherry picked from commit e9a8efe) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java # x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java * Fix build error --------- Co-authored-by: Ioana Tagirta <[email protected]>
When passing a negative
rank_window_size
to a compound retriever, we return an error message that referencessize
instead ofrank_window_size
:Response:
This is coming from:
elasticsearch/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java
Lines 517 to 519 in f0d7ec4
This validation needs to happen at the compound retriever level.
Besides fixing the error message, I also moved
rankWindowSize
to the base class for compound retrievers, instead of having it duplicated in 3 inherited retrievers.