Implement renameCollection command
#2343
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2343 +/- ##
==========================================
+ Coverage 65.58% 65.69% +0.10%
==========================================
Files 411 415 +4
Lines 20148 20343 +195
==========================================
+ Hits 13215 13364 +149
- Misses 5990 6027 +37
- Partials 943 952 +9
... and 1 file with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. |
Co-authored-by: Dmitry <dmitry.eremenko@ferretdb.io>
| break | ||
| } | ||
|
|
||
| tableName = fmt.Sprintf("%s_%d", defaultTableName, i) |
There was a problem hiding this comment.
Do we know is it possible to exceed the PostgreSQL table name limit by renaming and creating collection with a long name and renaming + recreating it multiple times? It doesn't sound like a serious issue although it's nice if we have it in mind :)
There was a problem hiding this comment.
test> db.runCommand({renameCollection:"test.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", to:"test.b" })
{ ok: 1 }
test> db.runCommand({renameCollection:"test.b", to:"test.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" })
MongoServerError: error with target namespace: Invalid collection name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
There was a problem hiding this comment.
It seems to me that we have different validation on rename than on create, exceeding the number is even not possible for a <235 x 'b' which wasn't inserted to the collection previously.
test> db.runCommand({renameCollection:"test.values2", to:"test.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" })
MongoServerError: error with target namespace: Invalid collection name: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
As this is very specific corner case we could fix that in followup either :)
There was a problem hiding this comment.
test> db.runCommand({renameCollection:"test.values2", to:"test.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" })
{ ok: 1 }
I was able to rename it to 63 character long collection name (max of pg table name), so we would probably need to reuse the same functions that are already used for collection creation
There was a problem hiding this comment.
Yeah, it's possible. Ideally, in this case, we should remove some symbols from the table name... I think I'll create a separate PR about it, as otherwise, this one will be even bigger (especially, with tests).
There was a problem hiding this comment.
Please do that in this issue (if not in this PR). That seems to be a serious problem
There was a problem hiding this comment.
test> db.values.renameCollection("values2")
MongoServerError: renameCollection: support for field "dropTarget" with value false is not implemented yet
test> db.values.renameCollection("values2",true)
MongoServerError: renameCollection: support for field "dropTarget" with value true is not implemented yet
test> db.values.renameCollection("values2",false)
MongoServerError: renameCollection: support for field "dropTarget" with value false is not implemented yet
test>
It also seems that we are very strict in terms of dropTarget field. I understand it's not in the scope, but returning the error even if it's false make it impossible to use renameCollection by mongosh commands and one is forced to use runCommand.
As dropTarget is false by default and it's behavior when false is exactly the same as our current behavior (we don't drop target collection if it already exists), can we return error only if dropTarget is set to true?
If we want to prioritize merging of this PR we could even do that in followup PR
|
@noisersup nice catch! |
There was a problem hiding this comment.
Looking great 🤗 There are some linter warnings and unhappy tests. Let's tackle those. Aside from comments addressed by @noisersup and @AlekSi, looks good to me!
| compatNSTo: targetDB.Name() + "." + strings.Repeat("aB", 150), | ||
| resultType: emptyResult, | ||
| altMessage: "error with target namespace: Invalid collection name: " + strings.Repeat("aB", 150), | ||
| }, |
There was a problem hiding this comment.
Great coverage of tests 🤗 At the moment, some of them are unhappy, let's fix them.
Description
Closes #1517.
See also FerretDB/dance#422 for the additional name-related diff tests.
Readiness checklist
task all, and it passed.@FerretDB/core), Assignee, Labels, Project and project's Sprint fields.