-
Notifications
You must be signed in to change notification settings - Fork 211
PHPC-2478: Implement API for bulkWrite command #1763
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
985a161 to
7c6d456
Compare
7a7b316 to
b1353e6
Compare
cc280df to
d5d3c10
Compare
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.
Initial feedback on the code - I haven't yet gone through all the tests.
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.
Yep, using bson_iter_init_find and bson_iter_value is probably significantly faster than walking the entire BSON structure 👍
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.
Created PHPC-2516 to track this.
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.
We do have some macros that we use for queries - would it make sense to make these macros universal? I'm thinking of something like this:
PHONGO_EXTRACT_OPTION_BOOL(zoptions, "bypassDocumentValidation", intern->bypass)
If so, I can create a ticket for this.
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.
I don't think there is sufficient overlap with the macros in Query.c to make that worthwhile. In Query, the parsed options go directly to a bson_t. Here, they're stashed on the object struct. For BulkWriteCommand's operation methods, they're going to be assigned via distinct setter methods (similar to most of the option handling in ClientEncryption.c).
It was possible to reuse WriteConcernError and WriteError with slight changes to their init functions.
This was not necessary for compilation, but it makes the header internally consistent.
Always return arrays for writeErrors and writeConcernErrors
d5d3c10 to
88e2508
Compare
7542cba to
3a45310
Compare
Per CDRIVER-5843, libmongoc does not consistently populate this field. It also isn't required by the spec, so omit it for now.
f65025a to
cd1622e
Compare
Superseded by #1790, which is rebased on v2.x.
https://jira.mongodb.org/browse/PHPC-2478