According to MySQL documentation, COLLATE must appear after CHARACTER SET https://dev.mysql.com/doc/refman/8.0/en/charset-table.html

Currently this module has it the other way around. This means when attempting to specify a collation and character set like so:

await db.createTable('test_table', {
    columns: {
      id: { type: 'int', notNull: true, primaryKey: true, autoIncrement: true },
    },
    charset: 'utf8mb4',
    collate: 'utf8mb4_unicode_ci'
  })

The following SQL is generated:

CREATE TABLE  `test_table` (`id` INTEGER  PRIMARY KEY AUTO_INCREMENT NOT NULL) COLLATE 'utf8mb4_unicode_ci' CHARACTER SET utf8mb4

This results in the correct utf8mb4 character set, but with a collation set to the default schema utf8mb4_general_ci not utf8mb4_unicode_ci

Correct SQL generated:

CREATE TABLE  `test_table` (`id` INTEGER  PRIMARY KEY AUTO_INCREMENT NOT NULL) CHARACTER SET utf8mb4 COLLATE 'utf8mb4_unicode_ci'

@wzrdtales would you be open to a PR to resolve this? Looks like spec.collate and spec.charset need swapping in _applyTableOptions

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Description

switchDatabase currently uses runSql to actually switch the database. This results in a broken dry-run mode. It won't switch because of the dry-run flag, but it needs to switch to properly simulate the behavior.

Solution

Replace runSqlwith all

References

When running npm audit fix in my repo I'm now reciving critial warnings due to old version of mysql2 in this package.
Output below:

# npm audit report

mysql2  <=3.9.3
Severity: critical
mysql2 Remote Code Execution (RCE) via the readCodeFor function - https://github.com/advisories/GHSA-fpw7-j2hg-69v5
mysql2 vulnerable to Prototype Poisoning - https://github.com/advisories/GHSA-49j4-86m8-q2jw
mysql2 cache poisoning vulnerability - https://github.com/advisories/GHSA-mqr2-w7wj-jjgr
fix available via `npm audit fix`
node_modules/mysql2
  db-migrate-mysql  >=2.2.0
  Depends on vulnerable versions of mysql2
  node_modules/db-migrate-mysql

2 critical severity vulnerabilities

This seams like something that would be good to fix as soon as possible.
I looked it up, db-migrate-mysql is using mysql2 v2.2.5 that was released 4 years ago, since then there has been a lot of releases and currently the latest version is 3.9.4.

Could we bump the version?
Since it's a major version update with potential breaking changes, will this be a big fix or something we could just bump and go on with our day?

You are returning a Promise without returning anything inside of the promise so bluebird throws a warning.

https://github.com/db-migrate/mysql/blob/master/index.js#L388

Warning: a promise was created in a handler but was not returned from it
    at Object.Base.extend.close (/opt/application/node_modules/db-migrate-mysql/index.js:388:12)
    ....

Issue:

Currently when you add a column, it will always be added to the end of the table. However, you often want to group related columns together to ensure that select * queries are more readable and that the column order matches the property order in your application data model.

Solution:

• Add support for MySQL's BEFORE and AFTER clauses to createColumnConstraint() method by adding before: column and after: column properties to the column spec.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

I suppose you can do it simply by changing createColumnDef function by adding one another if and using existing variable len (casual length property is inappropriate for decimals), for example:

  createColumnDef: function(name, spec, options, tableName) {
    var escapedName = util.format('`%s`', name),
        t = this.mapDataType(spec),
        len;
    if(spec.type === type.DECIMAL) {
       if(spec.precision && spec.scale){
         len = '(' + spec.precision + ',' + spec.scale + ')';
       }
    }
    else if(spec.type !== type.TEXT && spec.type !== type.BLOB) {
      len = spec.length ? util.format('(%s)', spec.length) : '';
      if (t === 'VARCHAR' && len === '') {
        len = '(255)';
      }
    }
    var constraint = this.createColumnConstraint(spec, options, tableName, name);
    return { foreignKey: constraint.foreignKey,
             constraints: [escapedName, t, len, constraint.constraints].join(' ') };
  },

Probably it will be more clear to use another variable (not len).

Second idea would be changing base function (from db-migrate-base) but I'm not sure if all databases have decimals with precision and scale. Additionally current function createColumnDef from db-migrate-mysql doesn't use base version so this change would not be sufficient.

I have tested my change and it seems to work and I added pull request with it.

Refers to db-migrate/node-db-migrate#339

showDatabase should reply with an array of database name strings.

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.

warning message when running db-migrate up: "Ignoring invalid configuration option passed to Connection: driver. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection"

patched according to db-migrate/node-db-migrate#753 (comment)

Here is the diff that solved my problem:

diff --git a/node_modules/db-migrate-mysql/index.js b/node_modules/db-migrate-mysql/index.js
index 0c4460d..66841e4 100644
--- a/node_modules/db-migrate-mysql/index.js
+++ b/node_modules/db-migrate-mysql/index.js
@@ -500,6 +500,7 @@ function dummy () {
 
 exports.connect = function (config, intern, callback) {
   let db;
+  delete config.driver;
 
   internals = intern;
   log = internals.mod.log;

This issue body was partially generated by patch-package.

I have some code like this:

db.runSql('INSERT INTO authGroups (id, isActive, isLocked, name, description, created) VALUES(?, TRUE, TRUE, ?, ?, NOW())', [id, name, description])

I expect that the question marks in sql will be replaced with the values from params (as the documentation describes). But the sql is being run with the question marks still there. I'm getting this error:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?, TRUE, TRUE, ?, ?, NOW())'

These are the values of id, name, description:

id: '4dfad450-7c17-11e9-8808-0242ac170002'
name: 'Users'
description: 'Test.'

Btw, I'm using promises, not the callback function.

What's strange is if I add a callback function as the third parameter, the placeholders are replaced. This seems like a bug.

If I use this function instead of db.runSql, it works fine:

const runSql = (db, query, params) => new Promise((resolve, reject) => {
    db.runSql(query, params, (err, data) => {
        if(err) reject(err)
        else resolve(data)
    })
})

For example:

runSql(db, 'INSERT INTO authGroups (id, isActive, isLocked, name, description, created) VALUES(?, TRUE, TRUE, ?, ?, NOW())', [id, name, description])

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Some years ago this same issue came up in db-migrate/node-db-migrate#408. This was fixed by adding a check for a null options param:

    if (spec.primaryKey) {
      if (!options || options.emitPrimaryKey) {
        constraint.push('PRIMARY KEY');
      }
    }

A year later the call to createColumnDef from addColumn was changed to pass {} instead of null as options, which means this check always fails and the PRIMARY KEY constraint is never added. If you add an autoincrement field you are unable to set it as a primary key, resulting in the migration always failing with this error:

ER_WRONG_AUTO_KEY: Incorrect table definition; there can be only one auto column and it must be defined as a key

I have a PR to check for an empty object instead in the mysql adapter. This could be fixed upstream by changing the call to createColumnDef in db-migrate-base, but I'm hesitant to fix it there as other adapters don't seem to be having the issue.

Ignoring invalid configuration option passed to Connection: driver. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection

Please help fix this issue. I have been using this earlier and seemed to be working fine. But since when I changed my database to which I need to connect, it's creating this issue.

Thanks in advance.

I know that TINYINT and MEDIUMINT are specific to mysql/mariadb that why I create this issue here.
Currently you can use both types but get the following warning

[WARN] Using unknown data type MEDIUMINT

Would it not be better to implement the specific types in the driver that can handle these?

I can create a PR for that, but you need to tell me how I should define the new data types, because currently they are defined in db-migrate-shared.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

const mconfig = {
	config: {
		dev: {
			driver: 'mysql',
			host: '', db: '', ....,
			typeCast: (field, next) => {} // this is getting removed or ignored
		},
		'sql-file': true
    },
    env: 'dev'
}

Migrate.getInstance(true, mConfig, callback);

I tried to find where or why but couldn't find it

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.