Simplified RRF Retriever #129659

Mikep86 · 2025-06-18T19:41:54Z

Adds a simplified syntax for the rrf retriever:

GET my-index/_search
{
  "retriever": {
    "rrf": {
      "fields": ["field_1", "field_2"],
      "query": "my awesome query"
    }
  }
}

fields is optional. If it is not provided, we query the fields defined by the index.query.default_field index setting (which is * by default).

This syntax automatically handles querying a mix of lexical fields (i.e. fields that support lexical search via match) and semantic_text fields. The fields are divided into lexical and semantic groups to create a 50/50 weight distribution between the two in the final ranking. This is achieved by creating a retriever tree that looks like:

rrf
   multi_match on lexical fields
   rrf
     match on semantic_text field A
     match on semantic_text field B
     match on semantic_text field C

This is a sibling of the simplified linear retriever, which was added in #129200.

elasticsearchmachine · 2025-06-18T19:42:20Z

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

elasticsearchmachine · 2025-06-18T19:42:20Z

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

elasticsearchmachine · 2025-06-18T19:42:20Z

Pinging @elastic/search-eng (Team:SearchOrg)

elasticsearchmachine · 2025-06-18T19:42:20Z

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

Mikep86 · 2025-06-18T19:47:57Z

x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java

+            // TODO: Refactor duplicate code
+            // Using the multi-fields query format
+            var localIndicesMetadata = resolvedIndices.getConcreteLocalIndicesMetadata();
+            if (localIndicesMetadata.size() > 1) {
+                throw new IllegalArgumentException(
+                    "[" + NAME + "] cannot specify [" + QUERY_FIELD.getPreferredName() + "] when querying multiple indices"
+                );
+            } else if (resolvedIndices.getRemoteClusterIndices().isEmpty() == false) {
+                throw new IllegalArgumentException(
+                    "[" + NAME + "] cannot specify [" + QUERY_FIELD.getPreferredName() + "] when querying remote indices"
+                );
+            }


@kderusso I know you requested that we refactor this common code, can we handle that in a follow up along with refactoring the common test code?

Yes, given the timing I'm fine with that happening in a followup

kderusso

Nice work! I am OK with the cleanup/consolidation being a followup, a few questions on tests plus we should update the docs. Approving to not block.

kderusso · 2025-06-18T20:06:51Z

.../rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/rrf/310_rrf_retriever_simplified.yml

+  - match: { error.root_cause.0.reason: "[rrf] does not support per-field weights in [fields]" }
+
+---
+"Can query keyword fields":


Can we add explicit field calls for the text and semantic_text fields too?

kderusso · 2025-06-18T20:09:20Z

x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java

+            // TODO: Refactor duplicate code
+            // Using the multi-fields query format
+            var localIndicesMetadata = resolvedIndices.getConcreteLocalIndicesMetadata();
+            if (localIndicesMetadata.size() > 1) {
+                throw new IllegalArgumentException(
+                    "[" + NAME + "] cannot specify [" + QUERY_FIELD.getPreferredName() + "] when querying multiple indices"
+                );
+            } else if (resolvedIndices.getRemoteClusterIndices().isEmpty() == false) {
+                throw new IllegalArgumentException(
+                    "[" + NAME + "] cannot specify [" + QUERY_FIELD.getPreferredName() + "] when querying remote indices"
+                );
+            }


Yes, given the timing I'm fine with that happening in a followup

kderusso · 2025-06-18T20:10:26Z

...rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderParsingTests.java

-            + "    }"
-            + "  }"
-            + "}";
+        String restContent = """


Shouldn't we parse this twice, once with retrievers and once with field/query?

This test is only for XContent parsing purposes, the resulting retriever does not need to pass SearchRequest validation

kderusso · 2025-06-18T20:12:18Z

...plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderTests.java

+            "foo"
+        );
+
+        // Non-default rank window size and rank constant


I don't quite understand that this is testing anything with rank window size or rank constant?

It's testing that the rank window size and rank constant are propagated to the rewritten retrievers

Samiul-TheSoccerFan

Nice work! I liked the tests in 310_rrf_retriever_simplified.yml. It made much easier to understand the required changes.

...-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/20_linear_retriever_simplified.yml

x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java

...rrf/src/yamlRestTest/java/org/elasticsearch/xpack/rank/rrf/RRFRankClientYamlTestSuiteIT.java

pmpailis · 2025-06-19T11:47:13Z

.../rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/rrf/310_rrf_retriever_simplified.yml

+  - match: { hits.hits.0._id: "1" }
+
+---
+"Can query date fields":


Can we also add a test where the simplified and fully-expanded format yield the same results?

pmpailis

LGTM

Mikep86 added 10 commits June 18, 2025 13:57

Apply RRF retriever changes from elastic#128633

34040c9

Use RetrieverSource.from

041e4f3

Fixed references to SimplifiedInnerRetrieverUtils

488ecf5

Removed references to simplified query format from error messages

d2c439f

Copy pre-filters during RRF retriever rewrite

337e17d

Added cluster feature

ff29ab8

Fix some remaining references to simplified format

d2138e9

Update RRF YAML tests to use default distribution

88d13da

Added YAML tests

55bbdf8

Added missing headers specification

79afadf

Mikep86 requested review from pmpailis, jimczi and kderusso June 18, 2025 19:41

Mikep86 added >enhancement auto-backport Automatically create backport pull requests when merged :SearchOrg/Relevance Label for the Search (solution/org) Relevance team :Search Relevance/Search Catch all for Search Relevance v8.19.0 v9.1.0 labels Jun 18, 2025

elasticsearchmachine added Team:SearchOrg Meta label for the Search Org (Enterprise Search) Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch Team:Search - Relevance The Search organization Search Relevance team labels Jun 18, 2025

Update docs/changelog/129659.yaml

2611ff5

github-actions bot deployed to docs-preview June 18, 2025 19:43 View deployment

Fix changelog

09aab96

github-actions bot deployed to docs-preview June 18, 2025 19:44 View deployment

Mikep86 commented Jun 18, 2025

View reviewed changes

kderusso approved these changes Jun 18, 2025

View reviewed changes

Samiul-TheSoccerFan approved these changes Jun 18, 2025

View reviewed changes

...-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/20_linear_retriever_simplified.yml Show resolved Hide resolved

pmpailis reviewed Jun 19, 2025

View reviewed changes

x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java Show resolved Hide resolved

pmpailis reviewed Jun 19, 2025

View reviewed changes

...rrf/src/yamlRestTest/java/org/elasticsearch/xpack/rank/rrf/RRFRankClientYamlTestSuiteIT.java Show resolved Hide resolved

pmpailis reviewed Jun 19, 2025

View reviewed changes

pmpailis approved these changes Jun 19, 2025

View reviewed changes

Simplified RRF Retriever #129659

Are you sure you want to change the base?

Simplified RRF Retriever #129659

Uh oh!

Conversation

Mikep86 commented Jun 18, 2025

Uh oh!

elasticsearchmachine commented Jun 18, 2025

Uh oh!

elasticsearchmachine commented Jun 18, 2025

Uh oh!

elasticsearchmachine commented Jun 18, 2025

Uh oh!

elasticsearchmachine commented Jun 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kderusso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Samiul-TheSoccerFan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmpailis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!