Erroneous warning about default mysql version during migration

[thidlay]

Environment

Knex version: 0.15.0
Database + version: MySQL 5.7
OS: Ubuntu Linux

Bug

When running a database migration, I'm seeing the following warning messages:

To get rid of this warning you should specify the mysql dialect version in your knex configuration. Currently this defaults to 5.5, but in a future release it will default to 5.6 which supports high precision timestamps. See http://knexjs.org/#Schema-timestamps for more information.

In my knex configuration, I have the following setup:

const knex = require({ client: 'mysql', version: '5.7', connection: ... });

This is exactly what is suggested on the knex docs site (copy/pasting relevant portion):

Note: The database version can be specified when using the MySQL adapter to take advantage of increased timestamp precision that is available in versions 5.6.4 and newer. In older versions the microsecond information is lost when storing datetime or timestamp values. The default in Knex is to not use microsecond precision, but this will change in a future version so, if you are using a version of MySQL older than 5.6.4, it is strongly advised to include the version in your options.

var knex = require('knex')({ client: 'mysql', version: '5.7', connection: { host : '127.0.0.1', user : 'your_database_user', password : 'your_database_password', database : 'myapp_test' } });

It looks like the message originates from here:
https://github.com/tgriesser/knex/blob/232fe9f1517dba927f6a3a1fb1b8842d7c0a4007/src/dialects/mysql/schema/columncompiler.js#L10

Could it be that in the previous line client.version needs to be client.config.version?

[ricardograca]

This is strange. I don't think the problem is that it should be client.config.version because we have tests for this functionality and they are passing: https://github.com/tgriesser/knex/blob/232fe9f1517dba927f6a3a1fb1b8842d7c0a4007/test/unit/schema/mysql.js#L774-L788

You don't really have this next line exactly like this do you?

const knex = require({ client: 'mysql', version: '5.7', connection: ... });

[elhigu]

I'm going to add another test for this, but I actually hand tested that this works some days ago when this was reported the last time.

There might be some place where copy is made of client (like for transaction) and version string is not copied. Complete test case would be useful though.

[tgriesser]

client.version only appears to be set on postgres dialect. do you remember where it was added for mysql?

[tgriesser]

But yes @thidlay client.config.version would fix it, if you want to open a PR with that change.

[ricardograca]

@tgriesser It was added on #2542, but I'm pretty sure this was working properly before. Not sure how it could be producing errors now.

[tgriesser]

@ricardograca the test seems to add the version directly to the client, which doesn't seem to be what happens in the current implementation. Opened #2709 to fix.

[ricardograca]

Yeah, good catch. I just copied the existing postgresql test code without thinking much about it.