-
Notifications
You must be signed in to change notification settings - Fork 100
Implement federated multi search #681
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
This is useful for testing federated stuff.
…i-search Implement federated multi search
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughFederated multi-search support is introduced, allowing queries across multiple indexes with merged results. New types and methods for federated queries and responses are added, including per-query weighting and federation options. The test macro is enhanced to handle multiple Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant MeilisearchServer
User->>Client: Construct FederatedMultiSearchQuery
User->>Client: Call execute_federated_multi_search_query()
Client->>MeilisearchServer: POST /multi-search with federation options
MeilisearchServer-->>Client: FederatedMultiSearchResponse
Client-->>User: Return merged results, facets, and errors
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #681 +/- ##
==========================================
- Coverage 93.81% 93.78% -0.03%
==========================================
Files 19 19
Lines 7792 7905 +113
Branches 7792 7905 +113
==========================================
+ Hits 7310 7414 +104
Misses 258 258
- Partials 224 233 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
meilisearch-test-macro/src/lib.rs (1)
167-167
: Minor: Unnecessary clone() on args.The
clone()
call onargs
might be unnecessary sinceExpr
values are typically cheap to clone or could be passed by reference.Consider removing the clone if not needed:
- let result = #inner_ident(#(#args.clone()),*).await; + let result = #inner_ident(#(#args),*).await;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
meilisearch-test-macro/README.md
(1 hunks)meilisearch-test-macro/src/lib.rs
(1 hunks)src/client.rs
(2 hunks)src/search.rs
(9 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: LukasKalbertodt
PR: meilisearch/meilisearch-rust#625
File: src/search.rs:368-370
Timestamp: 2025-06-12T13:28:23.700Z
Learning: In the Meilisearch Rust client, `SearchQuery` serializes its per-query federation settings under the key `federationOptions`; only the top-level multi-search parameter is named `federation`.
Learnt from: LukasKalbertodt
PR: meilisearch/meilisearch-rust#625
File: src/search.rs:758-759
Timestamp: 2025-06-12T12:31:13.079Z
Learning: In Meilisearch federated multi-search responses, `_federation.weightedRankingScore` is always returned, independent of the `showRankingScore` query parameter, so the `weighted_ranking_score` field in `FederationHitInfo` should remain a non-optional `f32`.
src/client.rs (2)
Learnt from: LukasKalbertodt
PR: meilisearch/meilisearch-rust#625
File: src/search.rs:368-370
Timestamp: 2025-06-12T13:28:23.700Z
Learning: In the Meilisearch Rust client, `SearchQuery` serializes its per-query federation settings under the key `federationOptions`; only the top-level multi-search parameter is named `federation`.
Learnt from: LukasKalbertodt
PR: meilisearch/meilisearch-rust#625
File: src/search.rs:758-759
Timestamp: 2025-06-12T12:31:13.079Z
Learning: In Meilisearch federated multi-search responses, `_federation.weightedRankingScore` is always returned, independent of the `showRankingScore` query parameter, so the `weighted_ranking_score` field in `FederationHitInfo` should remain a non-optional `f32`.
src/search.rs (2)
Learnt from: LukasKalbertodt
PR: meilisearch/meilisearch-rust#625
File: src/search.rs:368-370
Timestamp: 2025-06-12T13:28:23.700Z
Learning: In the Meilisearch Rust client, `SearchQuery` serializes its per-query federation settings under the key `federationOptions`; only the top-level multi-search parameter is named `federation`.
Learnt from: LukasKalbertodt
PR: meilisearch/meilisearch-rust#625
File: src/search.rs:758-759
Timestamp: 2025-06-12T12:31:13.079Z
Learning: In Meilisearch federated multi-search responses, `_federation.weightedRankingScore` is always returned, independent of the `showRankingScore` query parameter, so the `weighted_ranking_score` field in `FederationHitInfo` should remain a non-optional `f32`.
🧬 Code Graph Analysis (2)
src/client.rs (1)
src/request.rs (1)
body
(49-56)
src/search.rs (7)
src/indexes.rs (28)
client
(185-187)client
(231-233)client
(319-321)client
(375-377)client
(427-429)client
(472-474)client
(521-524)client
(555-557)client
(633-635)client
(699-701)client
(969-971)client
(1037-1039)client
(1088-1090)client
(1132-1134)client
(1185-1187)client
(1244-1246)execute
(1819-1831)execute
(2036-2038)index
(2106-2106)index
(2123-2124)index
(2161-2161)index
(2187-2187)index
(2215-2215)index
(2241-2241)index
(2268-2268)new
(80-88)new
(1730-1736)new
(1929-1935)src/request.rs (2)
request
(71-91)request
(147-159)src/client.rs (2)
index
(420-422)new
(50-62)meilisearch-test-macro/src/lib.rs (1)
meilisearch_test
(14-201)src/settings.rs (1)
new
(246-248)src/features.rs (1)
new
(52-61)src/reqwest.rs (1)
new
(23-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Code Coverage
🔇 Additional comments (9)
meilisearch-test-macro/README.md (1)
71-71
: Documentation accurately reflects the new functionality.The addition clearly explains that multiple
Index
parameters are now supported by the macro.src/client.rs (2)
131-144
: Well-structured implementation of federated multi-search execution.The method follows the established pattern from
execute_multi_search_query
and correctly handles the federated query type with appropriate return type mapping.
188-203
: Excellent documentation for federated search usage.The documentation clearly explains:
- How to create federated queries using
with_federation
- The challenges with generic type
T
when merging different document types- Practical solutions using either enums with
#[serde(untagged)]
orserde_json::Map
This will help users understand the nuances of federated search implementation.
meilisearch-test-macro/src/lib.rs (1)
108-184
: Excellent implementation of multi-index support in the test macro.The changes properly handle multiple
Index
parameters by:
- Generating unique variable names for each index
- Creating indexes with unique UIDs using the pattern
{test_name}_{index_number}
- Ensuring proper cleanup of all created indexes after test completion
This implementation maintains backward compatibility while enabling more complex test scenarios.
src/search.rs (5)
62-85
: Good design choice for backward compatibility.The addition of
skip_serializing_if
attributes reduces payload size, and the optionalfederation
field maintains backward compatibility while supporting federated search metadata.
405-414
: Correctly implements per-query federation options.The
QueryFederationOptions
struct with camelCase serialization aligns with the Meilisearch API specification where per-query federation settings serialize underfederationOptions
.
722-789
: Well-designed API for federated multi-search.The separation between
MultiSearchQuery
andFederatedMultiSearchQuery
provides a clean API. The builder methods for adding weighted queries and the conversion method maintain type safety while avoiding breaking changes.
885-901
: Correctly implements federation hit metadata.The
weighted_ranking_score
field is appropriately non-optional, aligning with the Meilisearch API specification that always returns this value for federated searches.
1261-1355
: Excellent test coverage for federated multi-search.The test comprehensively covers:
- Weighted query result ordering
- Heterogeneous document type handling via enum
- Federation options application
- Multiple index support through the enhanced test macro
Pull Request
Rework on top of #625
Related issue
Fixes #609
What does this PR do?
This PR adds types and methods to use the federated multi search API. There are multiple ways to add this to this library, depending on how strictly one wants to type everything. I decided to:
FederatedMultiSearchResponse
, which also required a newClient::execute_federated_multi_search_query
. The response of federated searches is just very different from normal multi searches, and I didn't think having an enum returned byexecute_multi_search_query
was a particularly nice design (and it would have been a breaking change).federation: Option<FederationHitInfo>
to eachSearchResult
(which isNone
for non-federated searches). I don't think it's worth to have a dedicatedFederatedSearchResult
.MultiSearchQuery::with_federation
which returns a newFederatedMultiSearchQuery
. One could also changeMultiSearchQuery
to be able to represent federated queries, but I had a slight preference for my solution.Summary by CodeRabbit
New Features
Bug Fixes
Documentation