-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat(NODE-6773): add multiple collinfo option #70
base: main
Are you sure you want to change the base?
Conversation
addon/mongocrypt.cc
Outdated
@@ -577,6 +577,8 @@ MongoCrypt::MongoCrypt(const CallbackInfo& info) : ObjectWrap(info) { | |||
|
|||
mongocrypt_setopt_retry_kms(mongo_crypt(), true); | |||
|
|||
mongocrypt_setopt_enable_multiple_collinfo(mongo_crypt()); |
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.
Note: The driver may need a way to know if this is set
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.
This shouldn't be necessary, only an updated libmongocrypt will request more than one collInfo
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.
Isn't the issue when an updated libmongocrypt interacts with a driver that isn't updated though (i.e., driver pre 6.14)?
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.
Yeah, unfortunately having moved the JS code for the bindings into the driver means that that's a case we now need to handle – this option would have to be passed from JS to the constructor instead of being set unconditionally
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.
Added a new option to control this, not sure how I would test this here since you can't inspect the properties of the struct (of course) and there doesn't seem to be a getopt
API.
We can flip the option in the driver, and your $lookups fail as a result.
Description
What is changing?
In support of: mongodb/node-mongodb-native#4427
Is there new documentation needed for these changes?
What is the motivation for this change?
Release Highlight
Enables multiple collection info support for $lookup aggregations
Calls
mongocrypt_setopt_enable_multiple_collinfo
to enable the settingDouble check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript