Postgres: prepend className to unique indexes #6741
Conversation
…r different classes
Codecov Report
@@ Coverage Diff @@
## master #6741 +/- ##
==========================================
- Coverage 93.78% 93.75% -0.04%
==========================================
Files 169 169
Lines 12221 12221
==========================================
- Hits 11462 11458 -4
- Misses 759 763 +4
Continue to review full report at Codecov.
|
|
Ready for review... I tested this PR and it's testcase on postgres 10, 11, and 12 and the change passes on all 3. The testcase fails on all 3 versions without the change as only 1 unique index is created instead of 2 unique indexes (1 for each table). |
…xists before creating it
|
Added another small fix to get rid of console warnings due to attempting to create an index every time during a restart of Postgres. The fix simply adds, |
…E doesn't seem to be needed
|
Fixed error messages outputting on restarts in the previous comment. There are still a set of errors below that show on initial start, but I believe those are okay and I don't know how to fix them. From what I see online, it seems this is due to the driver (node-postgres) having multiple connections and issuing the same command over multiple threads. It looks like postgres creates what's necessary and then throws the error from the other threads (I'm guessing). Not a big deal IMO |
|
@dplewis this one is ready for review |
|
@dplewis and @mtrezza can you review this? I believe it’s a pretty big deal for parse-server users who use Postgres and attempt to create index’s through parse-server because if a field with the same name exists in multiple tables, an index can only be created for 1 of those tables because of a bug in the code. |
| @@ -2063,13 +2066,11 @@ export class PostgresStorageAdapter implements StorageAdapter { | |||
| schema: SchemaType, | |||
| fieldNames: string[] | |||
| ) { | |||
| // Use the same name for every ensureUniqueness attempt, because postgres | |||
There was a problem hiding this comment.
Here’s the main part of the bug. The assumption that was made in the comments is incorrect
When using the PostgresStorageAdapter.ensureUniqueness, the current index name is always "unique_fieldName". This isn't an issue as long as an index field name isn't contained in another table, but causes issues if you want to create a unique index for "uuid" for two different tables. I deleted the comments that mentioned postgres allows the same index name multiple times. I don't know if that was true for older versions of postgres (it doesn't work on postgres 10, 11, and 12), but even the autogenerated primaryKey unique index prepends the table name. The update in this PR should work for older and newer.
The Uniqueness.spec.js makes sure the indexes are created. Update: I added a test case to make sure multiple unique indexes can be added to different classes that have the same field.
Probably need to add a recommendation: After upgrading to this version of parse-server, you should log into postgres and drop the old constraints. This should be done manually as it can block writes to the database:
The above way seems to be recommended by Heroku, as this is essentially is reindexing
unique_username,unique_email, andunique_namewithout blocking writes on the table. Below is a blurb from the link: