Fix for propagating filters from compound to inner retrievers #117914
Conversation
pmpailis
added
>bug
:Search Relevance/Ranking
|
Hi @pmpailis, I've created a changelog YAML for you. |
| this.rankWindowSize = rankWindowSize; | ||
| this.rankDocs = rankDocs; | ||
| if (sources == null || sources.isEmpty()) { | ||
| throw new IllegalArgumentException("sources must not be null or empty"); | ||
| } | ||
| this.sources = sources; | ||
| this.preFilterQueryBuilders = preFilterQueryBuilders; |
There was a problem hiding this comment.
The prefilters were not actually used anywhere for the RankDocsRetrieverBuilder as they had already been accounted for when computing the parent results.
elasticsearchmachine
added
the
Team:Search Relevance
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
|
run elasticsearch-ci/part-3 |
There was a problem hiding this comment.
Could you add a test that exposes the bug? My understanding was that it appeared when using a top level filter with a query_vector_builder in kNN. Lets add a test for that somewhere. It can be a simple single node test (not yaml).
To make the query vector builder stuff easier to test, you can maybe add a "test query vector builder" that simply returns a constant vector.
server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java
Show resolved
Hide resolved
That's a good idea. The |
...rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java
Outdated
Show resolved
Hide resolved
...rrf/src/internalClusterTest/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderIT.java
Outdated
Show resolved
Hide resolved
💔 Backport failed
You can use sqren/backport to manually backport by running |
…117914) (#118047) * Fix for propagating filters from compound to inner retrievers * fix for lucene 9 * Update CompoundRetrieverBuilder.java * Update CompoundRetrieverBuilder.java * Update CompoundRetrieverBuilder.java --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
|
@benwtrent what's the procedure for flagging bugs in affected versions? Or do we just rely on the bug fix notice in the fixed version to communicate this? |
|
@leemthompo if it deserves a "known issue" call out in the release versions, then that needs to be added manually. Otherwise, the release notes already automatically contains the bug fix. |
This PR aims to fix an issue where the inner filters are not properly added to the children of compound retrievers if they require different number of rewrite rounds.