Check if ENUM value exists before ALTER TYPE #4464
Conversation
|
This only works in Postgres 9.3+ not sure if sequelize officially supports older versions. |
|
Hmm, if it does is there a way in sequelize we can use this only if 9.3+ is being used? |
|
We definitely support older versions. But you can use |
|
Oh sweet, I added a check for postgres |
| var enumName = this.pgEnumName(tableName, attr); | ||
| var sql = semver.gte(this.sequelize.options.databaseVersion, '9.3.0') | ||
| ? 'ALTER TYPE ' + enumName + ' ADD VALUE IF NOT EXISTS ' + this.escape(value) | ||
| : 'ALTER TYPE ' + enumName + ' ADD VALUE ' + this.escape(value); |
There was a problem hiding this comment.
sql = 'ALTER TYPE ' + enumName + ' ADD VALUE';
if (semver.gte(this.sequelize.options.databaseVersion, '9.3.0')) {
sql += 'IF NOT EXISTS';
}
sql += this.escape(value)Nitpicking now, but this is slightly more DRY, and less prone to errors where you refactor one case but not the other :)
There was a problem hiding this comment.
I totally agree! I was thinking of doing something like this but thought it might have hurt readability a bit, so decided against it. Let me fix it and squash these commits real quick.
|
LGTM - just needs a couple of SQL unit tests |
Currently sequelize will not check if an ENUM value exists before attempting to alter the type. If it does it can throw an error such as:
```
SequelizeDatabaseError: enum label "[labelName]" already exists
at [object Object].Query.formatError (node_modules/sequelize/lib/dialects/postgres/query.js:433:14)
at [object Object].<anonymous> (node_modules/sequelize/lib/dialects/postgres/query.js:108:19)
at [object Object].Query.handleError (node_modules/pg/lib/query.js:108:8)
at [object Object].<anonymous> (node_modules/pg/lib/client.js:171:26)
at Socket.<anonymous> (node_modules/pg/lib/connection.js:109:12)
at readableAddChunk (_stream_readable.js:146:16)
at Socket.Readable.push (_stream_readable.js:110:10)
at TCP.onread (net.js:523:20)
```
By adding the `IF NOT EXISTS` qualifier it will only throw a `NOTICE` that the enum label exists and skip it.
Check for Postgres 9.3.0 and greater
require in semver
Add check for 9.3.0 ENUM feature
Remove extra quotation mark
Break out conditional SQL statement into IF block
| count++; | ||
| } | ||
| else if (sql.indexOf('joyful') > -1) { | ||
| var cmd = semver.gte(this.sequelize.options.databaseVersion, '9.3.0') | ||
| ? "ALTER TYPE \"enum_UserEnums_mood\" ADD VALUE IF NOT EXISTS 'joyful' AFTER 'meh'" | ||
| : "ALTER TYPE \"enum_UserEnums_mood\" ADD VALUE 'joyful' AFTER 'meh'" | ||
| expect(sql.indexOf("ALTER TYPE \"enum_UserEnums_mood\" ADD VALUE 'joyful' AFTER 'meh'")).to.not.be.equal(-1); |
There was a problem hiding this comment.
Please add sql unit tests, don't ammend the integration tests that test sql (we're in the procress of removing them).
|
Ping @aweary . Please move the test to https://github.com/sequelize/sequelize/blob/master/test/unit/sql/enum.test.js then this should be good to go |
|
👍 I'll get on that as soon as I can |
Currently sequelize will not check if an
ENUMvalue exists before attempting to alter the type. As it stands it can throw an error such as:By adding the
IF NOT EXISTSqualifier it will only throw aNOTICEthat the enum label exists and skip it.