-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: allow top level dollar keys with findOneAndUpdate(), update() #13786
Conversation
since when does mongodb allow that? because mongodb 4.4 tests fail with "not allowed" |
tests failing, mongodb 4.4 seemingly does not support it
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 second hasezoey's concern, I feel like we should either:
A) err on the side of caution for now and keep blocking adding keys that start with $
.
B) Make it conditional, by fetching the server information and making sure it supports those keys.
A more performant alternative approach to B is to keep things as is in the PR, but add error-handling that throws an error an error along these lines: mongo does not allow storing keys that start with a $ sign, please upgrade your mongodb server version to version x.y or later
.
It looks like MongoDB 5 was the first version to support top-level keys that start with $. I disagree that we should be checking the MongoDB server version for 2 reasons:
|
Fair points, we can just leave it up to the server to throw the proper errors when needed while supporting the feature for those who have more recent versions. |
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.
looks good to me, but this should definitely be mentioned in the changelog and in the documentation that only mongodb 5 and up support top-level $
keys
Summary
Right now, MongoDB allows storing top-level keys that start with $, but our
castUpdate()
function doesn't allow top-level dollar keys.Examples