Do not set charset and collate options on postgresql

[QuentinFarizon]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] for the project I'm working on.

Sequelize v7.0.0-alpha.33 complains (rightly so), that collate and charset optinos are not supposed to be passed for postgres dialect. To my knowledge, this is only used for mysql

Here is the diff that solved my problem:

diff --git a/node_modules/umzug/lib/storages/SequelizeStorage.js b/node_modules/umzug/lib/storages/SequelizeStorage.js
index 1277063..469a6cc 100644
--- a/node_modules/umzug/lib/storages/SequelizeStorage.js
+++ b/node_modules/umzug/lib/storages/SequelizeStorage.js
@@ -90,8 +90,8 @@ class SequelizeStorage extends _Storage.default {
       tableName: this.tableName,
       schema: this.schema,
       timestamps: this.timestamps,
-      charset: 'utf8',
-      collate: 'utf8_unicode_ci'
+      charset: this.sequelize.dialect.name === 'mysql' ? 'utf8' : null,
+      collate: this.sequelize.dialect.name === 'mysql' ? 'utf8_unicode_ci' : null
     });
   }
   /**

This issue body was partially generated by patch-package.

[WikiRik]

See #612 for earlier discussion about this

[QuentinFarizon]

@WikiRik What do you think of my solution ?

With something like :

const charsetableDialects = ['mysql', 'mariadb', 'snowflake']
(...)
charset: charsetableDialects.includes(this.sequelize.dialect.name) ? 'utf8' : null,
collate: charsetableDialects.includes(this.sequelize.dialect.name) ? 'utf8_unicode_ci' : null

[WikiRik]

That looks good

[QuentinFarizon]

@WikiRik I've pushed a PR : #624

[github-actions]

Released in v3.4.0.

[github-actions]

Released in v3.4.0.

[qfarizon-qftech]

@WikiRik It would be great to update sequelize-cli to use this new version !

[WikiRik]

@qfarizon-qftech sequelize-cli is not meant tested with or updated for the v7 alphas and still uses umzug v2. The new CLI, which we started work on in the sequelize monorepo (and will be published as @sequelize/cli), will be using umzug v3 with this update.

[qfarizon-qftech]

@WikiRik Can I test @sequelize/cli as of today ?

[WikiRik]

@WikiRik Can I test @sequelize/cli as of today ?

It only has the seed generate and migration generate commands so it's not usable yet. (We're doing a full rewrite)

If @mmkal agrees you might be able to backport your change to umzug v2 and see how much of the old CLI works with the v7 alphas.

[qfarizon-qftech]

For now I have patched umzug@2 using patch-package.

[mmkal]

If anyone would like to backport anything to v2.x I'm fine to review a pull request and publish, just open it against v2.x branch. I won't check it beyond just building and publishing though.