search

preaction / Minion-Backend-mysql

MySQL backend for the 🐙 Minion job runner
Other
7 stars 14 forks source link

Require at least MySQL version 5.6.5 #9

Closed paultcochrane closed 6 years ago

paultcochrane commented 6 years ago

The reason for this requirement is that the current codebase requires more than one TIMESTAMP column in a table which uses either one of the DEFAULT CURRENT_TIMESTAMP and ON UPDATE CURRENT_TIMESTAMP clauses. This was a restriction in MySQL which was removed in version 5.6.5. It seems that the versions of MariaDB after the 5.5 series (i.e. 10.x and above) don't have this restriction, hence requiring a server version number greater than or equal to 5.6.5 is sufficient to ensure that the code works with the available MySQL server installation. A somewhat confusing thing is that the MariaDB 5.5 and 10.0 series seem to be compatible with the 5.5 series of MySQL as noted on the MariaDB versus MySQL Compatibility page, however the 10.0 series doesn't show the TIMESTAMP restriction. Nevertheless, this change should guard against users trying to use a version of MySQL which doesn't support the required TIMESTAMP functionality.

The background to this patch is that I noticed there were a few test failures on CPAN Testers and the error that kept cropping up was:

Incorrect table definition; there can be only one TIMESTAMP column with CURRENT_TIMESTAMP in DEFAULT or ON UPDATE clause

And after a bit of searching I found out that this was a restriction in MySQL which had been fixed in version 5.6.5. Hence, I thought one probably needs a guard in the test suite to make sure that a compatible version of MySQL or MariaDB is available before proceeding further. I've also updated the documentation to note that one requires at least version 5.6.5 of MySQL for the software to work. I know that the docs say that the code had been tested with version 5.5, however my guess is that some change in the past caused the code to no longer be compatible with the older version.

Anyway, have a look and see what you think. If anything needs to be updated or whatever, just let me know and I'll fix and resubmit.

preaction commented 6 years ago

Looks good to me. If someone later wants to come and fix it to work with older MySQLs, they're welcome (but it seems like that'll be a bit difficult, unless we keep some parallel migrations for different MySQL versions or something...)

I fixed the conflict and merged on the command-line, so closing.

paultcochrane commented 6 years ago

Dunno if this helps much, but here's the script I used to check my theory that MySQL 5.5 was causing problems. Debian Jessie has MySQL 5.5 as its standard installation of MySQL, however also provides MariaDB 10.0.x, so it's a good test bed to check the differences. Anyway, perhaps this can be of some use, if only conceptually.

build-on-jessie.txt

preaction commented 6 years ago

The longer-term goal is to make Travis do that work, but that script is a good start, yes