PHPC-1195: BulkWrite::update() should append arrayFilters as array #842
Conversation
jmikola
changed the title
derickr
left a comment
derickr
left a comment
There was a problem hiding this comment.
Added a comment, but haven't LGTM'ed it as it says [WAIT], so I expect more changes?
src/MongoDB/BulkWrite.c
Outdated
There was a problem hiding this comment.
Can you explain with a comment what a "valid key" is?
These tests were only using Mongo Orchestration to ensure a 3.6 server. skip_if_server_version() can be used instead.
jmikola
changed the title
derickr
left a comment
derickr
left a comment
There was a problem hiding this comment.
Needs the code fix, and I had some questions.
src/MongoDB/BulkWrite.c
Outdated
There was a problem hiding this comment.
This has an extra zend_get_type_by_const — Travis says so too: https://travis-ci.org/mongodb/mongo-php-driver/jobs/383741731#L1198
src/MongoDB/BulkWrite.c
Outdated
There was a problem hiding this comment.
And this is an equivalent to bson-encode.c's php_phongo_is_array_or_document? Should they be placed together perhaps? I don't mind, so please do what makes sense for you.
There was a problem hiding this comment.
php_phongo_is_array_or_document() operates on a zval, while this validates a bson_t.
We can't work with the zval here, as we're not inspecting it until it's already been converted to a bson_t. In theory, this allows a Serializable value to produce a BSON array for this option.
There was a problem hiding this comment.
This is a bit misleading, innit? skip_if_not_clean will also clean first?
There was a problem hiding this comment.
This change was made during #787.
Technically, the function attempts to drop the test collection and skips the test if that fails for some reason. I'll admit it's unlike other skip_if functions because it attempts to change the state of the system; however, it can actually skip if something goes wrong. In the past, CLEAN() ignored any errors.
If you have some ideas for renaming or revising this, a new ticket would be best 👍
2 participants
https://jira.mongodb.org/browse/PHPC-1195
Note: this is waiting on CDRIVER-2661. Once that is merged, this PR can be updated to bump the libmongoc submodule to 1.10-dev (assuming the issue is also backported to the
r1.10branch).