Skip to content

Conversation

isabelatkinson
Copy link
Contributor

@isabelatkinson isabelatkinson commented Aug 7, 2025

@isabelatkinson isabelatkinson marked this pull request as ready for review August 27, 2025 16:29
@isabelatkinson isabelatkinson requested a review from a team as a code owner August 27, 2025 16:29
@isabelatkinson isabelatkinson requested a review from abr-egn August 27, 2025 16:29
src/bson_util.rs Outdated
fn push(&mut self, doc: RawDocumentBuf);

/// Adds the collection of raw documents to the provided command.
fn add_to_command(self, identifier: &'static CStr, command: &mut Command);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extremely minor nit: I don't think this needs the 'static?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does not, updated!

let index = self.cache.len();
let bytes_added = T::bytes_added(index, &namespace_doc)?;
self.pending_namespace = Some(namespace_doc);
self.cache.insert(namespace, index);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know in current usage it doesn't matter because a pending document not being added terminates the outer loop, but it still bugs me that the namespace is added to the cache even if add_pending is never called. Not sure what a good way to fix that would be, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to defer adding to the cache til add_pending is called

@isabelatkinson isabelatkinson requested a review from abr-egn August 28, 2025 17:28
@isabelatkinson isabelatkinson merged commit 330ced1 into mongodb:main Aug 28, 2025
21 checks passed
@isabelatkinson isabelatkinson deleted the bulk-write-qe branch August 28, 2025 17:52
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

Successfully merging this pull request may close these issues.

2 participants