Add support for returning document vector data when get one document #672
Conversation
WalkthroughA new optional boolean field, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DocumentQuery
participant Index
User->>Index: Obtain Index reference
User->>DocumentQuery: Call DocumentQuery::new(&Index)
User->>DocumentQuery: Call .with_fields(...)
User->>DocumentQuery: Call .with_retrieve_vectors(true)
Note right of DocumentQuery: Sets retrieve_vectors = Some(true)
User->>DocumentQuery: Execute query
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/documents.rs (1)
139-145: Builder method is correct but consider eliding thefalsecaseWhen the user calls
.with_retrieve_vectors(false)we still serialiseretrieveVectors=false.
If the server treats the absence of the parameter as “false” already (which is common), sending an explicitfalseis redundant and increases payload size.A leaner alternative:
- pub fn with_retrieve_vectors( - &mut self, - retrieve_vectors: bool, - ) -> &mut DocumentQuery<'a, Http> { - self.retrieve_vectors = Some(retrieve_vectors); + pub fn with_retrieve_vectors( + &mut self, + retrieve_vectors: bool, + ) -> &mut DocumentQuery<'a, Http> { + if retrieve_vectors { + self.retrieve_vectors = Some(true); + } else { + self.retrieve_vectors = None; + } self }This keeps the query string clean while still allowing an opt-in behaviour.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/documents.rs(4 hunks)
🔇 Additional comments (2)
src/documents.rs (2)
91-96: Constructor update looks good
DocumentQuery::newnow initialisesretrieve_vectorstoNone, keeping backward compatibility (flag is absent unless explicitly set).
No issues spotted.
83-87:❓ Verification inconclusive
Confirm query-string key matches the Meilisearch API
retrieve_vectorsis serialized asretrieveVectors, which is camel-cased and differs from the usual snake-case keys (fields,limit, …).
Before merging, make sure the server really expects the exact key"retrieveVectors"; otherwise every call will silently drop the flag.If no other occurrence is found or the API expects a different name (e.g.
vectororreturnVectors), adjust therenameattribute accordingly.
🏁 Script executed:
#!/bin/bash # Grep for other occurrences so that the new key stays consistent rg -n '"retrieveVectors"' --type rustLength of output: 141
Verify Meilisearch search param “retrieveVectors”
The
retrieve_vectorsfield is being serialized asretrieveVectors, which deviates from the usual snake_case query keys (fields,limit, …). We only found this single occurrence:
- File: src/documents.rs
Line 85:#[serde(skip_serializing_if = "Option::is_none", rename = "retrieveVectors")]Before merging, please confirm in the Meilisearch API documentation that the search endpoint expects exactly
"retrieveVectors". If the actual parameter name differs (for example,vectororreturnVectors), update therenameattribute accordingly.
JiaYingZhang
force-pushed
the
doc-retrieve-vec-support
branch
from
3130f8c to
161dff1
Compare
Pull Request
Related issue
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit