[GODRIVER-304] update (replace) operations return error that key doesn't start with $ Created: 24/Mar/18 Updated: 10/Sep/18 Resolved: 10/Sep/18 |
|
| Status: | Closed |
| Project: | Go Driver |
| Component/s: | CRUD |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Sam Kleinman (Inactive) | Assignee: | Kristofer Brandow (Inactive) |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | evg | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Epic Link: | mgo migration tool | ||||
| Description |
|
this might be related to I tried to use the driver to specify a replacement-style update, and it returns an error, which it should not. |
| Comments |
| Comment by Sam Kleinman (Inactive) [ 23/May/18 ] | ||||||||||||||||||||||||
|
I've put this in the "mgo migration tool" epic, which is, I think, the best place for this to live? | ||||||||||||||||||||||||
| Comment by Sam Kleinman (Inactive) [ 15/May/18 ] | ||||||||||||||||||||||||
|
If we do I see the rationale in not having it, but not having makes supporting migrations from MGO more difficult. While there are technical solutions to work around this (e.g. implementing your own update document that introspects all update specifications looking for a $ operator), not doing this just makes the initial experience with the new driver less intuitive and complicated for people who are committed to using go and want to use the new driver. These users, and their experience seem like a really important segment for us, and throwing roadblocks in their way in the name of strict compliance with the spec just doesn't feel right, but your call. | ||||||||||||||||||||||||
| Comment by Jeffrey Yemin [ 14/May/18 ] | ||||||||||||||||||||||||
|
Drivers have deprecated the old method, so that will have caused some percentage of legacy applications to move to the new API. As this issue is written up as a bug, I'm inclined to close as Won't Fix. I also don't think we should consider a new feature that works as you described. It's out of (CRUD) spec, and I am not inclined to add a method to a new driver that other drivers have long deprecated. | ||||||||||||||||||||||||
| Comment by Sam Kleinman (Inactive) [ 14/May/18 ] | ||||||||||||||||||||||||
|
It looks like (python and the shell at least) still provide general "update" operations that don't comply with the spec but nevertheless still work. I think you're right that I'm asking for a method to ease the transition. Has any driver actually removed the multi-mode update operation? I'm not convinced that anyone has actually migrated legacy code to the crud-spec methods given that the legacy methods were never removed.
| ||||||||||||||||||||||||
| Comment by Jeffrey Yemin [ 14/May/18 ] | ||||||||||||||||||||||||
|
Legacy drivers generally had a single update method that handled every case. For drivers following the CRUD spec (which the Go driver should), updateOne should reject documents that don't contain update operators. I think your point is that mgo migrators will have a hard time dealing with the change. But in practice users migrating to the CRUD spec in legacy drivers have not complained about this behavior.
| ||||||||||||||||||||||||
| Comment by Sam Kleinman (Inactive) [ 14/May/18 ] | ||||||||||||||||||||||||
|
I'm asking about the inverse case: does update/update_one error in the replace case for legacy drivers. it's reasonable to make replace_one error in the set case, as this is a new method added by the CRUD spec that doesn't exist in legacy drivers (like mgo) and doesn't map directly onto a mongodb operation. | ||||||||||||||||||||||||
| Comment by Jeffrey Yemin [ 26/Apr/18 ] | ||||||||||||||||||||||||
|
ReplaceOne (there is no ReplaceMany) should only accept an update document that has no $-prefixed keys. In PyMongo this throws:
Other drivers work the same. | ||||||||||||||||||||||||
| Comment by Sam Kleinman (Inactive) [ 26/Apr/18 ] | ||||||||||||||||||||||||
|
I understand that this is the content of the specification, but I'm not sure that its particularly workable. mgo doesn't do this validation (and neither, from what I can tell, does pymongo; the server decides its behavior based on the content of the document as it always has.) This will significantly complicate our adoption of the new API since the error can only appear at runtime. cc jeff.yemin for context on the decision and the ways in which the drivers implement this specification. | ||||||||||||||||||||||||
| Comment by Kristofer Brandow (Inactive) [ 24/Apr/18 ] | ||||||||||||||||||||||||
|
For replace operations, you use the ReplaceOne and ReplaceMany methods when you want to do a replace: https://github.com/mongodb/specifications/blob/master/source/crud/crud.rst#update-vs-replace-validation. | ||||||||||||||||||||||||
| Comment by Sam Kleinman (Inactive) [ 24/Apr/18 ] | ||||||||||||||||||||||||
|
I made the following edit to work around:
The driver enforces that the second argument to the update operation use update operators, when in fact its perfectly legal to pass a document without update operators to the update spec which will then replace the document(s) that match the query. | ||||||||||||||||||||||||
| Comment by Kristofer Brandow (Inactive) [ 26/Mar/18 ] | ||||||||||||||||||||||||
|
Hi Sam, Can you please provide an example of the replacement-style update you are attempting to perform and the resulting error? |