No info about inserted_ids on insert_many when there were some exceptions

[max-frai]

Hey ho! :) It works, but now we don't have info about inserted_ids when ordered = false.

https://github.com/H2CO3/avocado/blob/e43035388b0c2b26d16dd560cb6e6452d4bd7ad2/avocado/src/coll.rs#L149-L157

So you check result and if there were bulk error, drop the request as failed. But InsertManyResult also contains info about inserted ids:

pub struct InsertManyResult {
    pub acknowledged: bool,
    pub inserted_ids: Option<BTreeMap<i64, Bson>>,
    pub bulk_write_exception: Option<BulkWriteException>,
}

And I can't understand in code that transaction is okay and how many rows were anyway inserted because of ordered = false.

I'm not sure what's the correct way to handle such situation. Maybe when ordered=false in insert many, don't return Err if result was BulkWriteException and mark it always as success?

[H2CO3]

Reporting an error as success is not a viable option IMO, although I'm not exactly sure what you are asking here. In particular, what do you mean by this?

And I can't understand in code that transaction is okay

If an error occurred, I wouldn't consider it as okay.

[max-frai]

I want to know how many documents were inserted and their ids. So when there is bulk error mongodb returns array of results including how many docs inserted and how many exceptions appeared.

So in my code I send some transaction to add list of objects and I want to know how many of them were added into database. Now returned error does not contain this information.

My concert about why it could be success: Bulk error for insert_many is just indicator that some of docs failed to insert, but overall request was applied to database. But yes, it's probably bad idea.

So maybe on bulk error return not only bulk exception, but full result object?

[H2CO3]

Oh, I see. I think the more appropriate solution would be for the error to contain this information.

[max-frai]

Sorry, do you have some deadlines for this? Because we are rolling into production in 2 days :) If you don't have time, I'll just fix it manually on our side

[H2CO3]

Hey, I'm working on it. I was just busy in the last two days but if you look at the latest commit I've got the foundations for it and I'm trying to figure out a nice design in this very moment 😄

[max-frai]

@H2CO3 No problems! I think you will like the project we built with your library ;) I will write about this later after going to prod.

[H2CO3]

@max-frai can you please look at HEAD now. The last 4 commits should solve this issue. Unfortunately I had to put a 'static lifetime bound on the document type because I couldn't figure out how to make a UnitStruct(PhantomData<T>) 'static without marking T itself as 'static.

[max-frai]

Looks good for me! We are going to test it now, thank you!

[H2CO3]

No problem – please make sure to check back, because if it works, I'll close the issue and release a new version on crates.io.

[max-frai]

Confirm everything is okay, thank you for fast fix ;) I don't think that making 'static for T would broke some use-cases

[H2CO3]

Awesome, thanks for the feedback. Version 0.6.0 is now on crates.io.

[H2CO3]

@max-frai Just to follow up on these, have you successfully deployed to prod? I'm curious what you've built as this is probably the first serious use of this library.

[max-frai]

@H2CO3 Everything is going okay, thank you! I will write result article tomorrow on reddit and will send you the link ;)

[H2CO3]

I'm glad to hear that, thanks! :)

[max-frai]

https://www.reddit.com/r/rust/comments/cdg5b4/rust_in_the_on_of_the_biggest_music_festival/ Here it is, thank you again ;)

[H2CO3]

Awesome, great post, thanks!