Skip to content
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

Use RawDocument/Buf in doc! #426

Closed
TheDan64 opened this issue Aug 22, 2023 · 3 comments
Closed

Use RawDocument/Buf in doc! #426

TheDan64 opened this issue Aug 22, 2023 · 3 comments
Assignees

Comments

@TheDan64
Copy link

TheDan64 commented Aug 22, 2023

Something I'd like to be able to do is:

let doc = RawDocument::from_bytes(b"<some valid bytes>");
let update = doc! {
    "$set": doc,
};

This doesn't compile today, maybe understandably - since the bytes could be invalid - but the only alternative seems to be to allocate (twice?): doc.to_raw_document_buf().to_document().unwrap() but it would be nice to have a usable RawDocument without extra/intermediate allocation. Maybe RawDocument could have a validate method which returns a Result<ValidatedRawDocument> which could be used in the doc macro?

@isabelatkinson
Copy link
Contributor

Hey Dan, I'd recommend using the TryFrom<RawDocument> implementation for Document to convert between the two:

let raw_doc = RawDocument::from_bytes(b"raw document contents")?;
let update = doc! {
    "$set": Document::try_from(raw_doc)?,
};

This will allow you to avoid the additional allocation of a RawDocumentBuf.

it would be nice to have a usable RawDocument without extra/intermediate allocation

Because RawDocument and Document have different internal representations, it's not possible to avoid allocation if you want to nest a RawDocument inside of a Document. If you need to end up with a Document, it might be more efficient to construct that first doc as a regular non-raw document (if that works for your use case) to avoid conversion between types.

I think there's also some room for improvement in the driver API to accept raw documents in certain places we currently accept regular documents (like update documents, which I believe is your use case here) so that you don't need to allocate a Document in the first place. We'd have to wait until the next major version to change our API like this, but I will discuss this idea with the team as a consideration for 3.0!

@TheDan64
Copy link
Author

If you need to end up with a Document, it might be more efficient to construct that first doc as a regular non-raw document (if that works for your use case) to avoid conversion between types.

Yeah, that makes sense but doesn't work for my use case since raw document bytes are being sent and then deserialized to RawDocument to avoid overhead (since we know it should always be a valid document).

Appreciate the tip, didn't know TryFrom was implemented! & thanks for the consideration for v3! Being able to accept a raw document for updates would be useful!

@isabelatkinson
Copy link
Contributor

Filed RUST-1747. Going to close this out but feel free to open another issue if you run into anything else!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants