-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add $rename
update operator
#271
Conversation
$rename
update operator$rename
update operator
src/main/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfiguration.java
Show resolved
Hide resolved
src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/RenameOperation.java
Outdated
Show resolved
Hide resolved
// If there is a value will be a modification (since source value will | ||
// disappear), regardless of whether target changes | ||
modified = true; | ||
PathMatch dst = action.targetLocator().findOrCreate(doc); |
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.
There is document which has address sub-document like {"address" : {"address line" : "123 main st", "city" : "NYC", "state": "NY"}}
If the update request is to rename city to town, the sub-document fields (city and state) order will change as {"address" : {"address line" : "123 main st", "state": "NY", "city" : "NYC"}}? Is this the expected behavior?
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.
@maheshrajamani As far as I can see, mongo docs state (and Atlas seem to implement it like so) that $rename
is implemented as "$unset followed by $set"; and $set appends at the end.
So I think it would indeed be expected that field "moves" the way you describe; city
removed, town
appended.
One thing I may want to do is to check the case of 'src == dst` -- looks like that needs to become an error too. EDIT: done. |
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.
LGTM 👍
What this PR does:
Adds
$rename
general update operatorWhich issue(s) this PR fixes:
Fixes #165
Checklist