New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require at least MySQL version 5.6.5 #9

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@paultcochrane
Contributor

paultcochrane commented Nov 4, 2017

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.

Require at least MySQL version 5.6.5
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](https://dev.mysql.com/doc/relnotes/mysql/5.6/en/news-5-6-5.html#mysqld-5-6-5-data-types).
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](https://mariadb.com/kb/en/library/mariadb-vs-mysql-compatibility/),
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.
@preaction

This comment has been minimized.

Show comment
Hide comment
@preaction

preaction Nov 6, 2017

Owner

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.

Owner

preaction commented Nov 6, 2017

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.

@preaction preaction closed this Nov 6, 2017

@paultcochrane paultcochrane deleted the paultcochrane:pr/require-mysql-565 branch Nov 6, 2017

@paultcochrane

This comment has been minimized.

Show comment
Hide comment
@paultcochrane

paultcochrane Nov 6, 2017

Contributor

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

Contributor

paultcochrane commented Nov 6, 2017

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

This comment has been minimized.

Show comment
Hide comment
@preaction

preaction Nov 6, 2017

Owner

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

Owner

preaction commented Nov 6, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment