Clean up MySQL version detection logic

[morozov]

Q	A
Type	improvement

The logic of MySQL or MariaDB version detection is unnecessarily complicated:

1. We do not need to parse the version using regular expressions. Comparing the version via version_compare() is sufficient.
2. As for MariaDB, we just need to detect it by the "MariaDB" substring and strip the "5.5.5-" prefix, which the older MariaDB versions supply for compatibility with Oracle MySQL. See: https://mariadb.com/kb/en/version/

[ausi]

@morozov is it intentional that ?serverVersion=8.0 and ?serverVersion=8 no longer work now?
This broke some of my configurations where I specified the version 8.0.

[morozov]

No but those don't seem to be valid versions.

[bohanyang]

This commit introduces a huge bug: #5794

[ausi]

No but those don't seem to be valid versions.

We used 8.0 as a possible selection to CMS users when setting up the connection, see https://github.com/contao/contao-manager/blob/b84abfb4d24688324017455b6cc72031bf14234a/src/components/setup/DatabaseConnection.vue#L99-L114

In doctrine/dbal 3.4.* this was also covered by unit tests:

dbal/tests/Driver/AbstractMySQLDriverTest.php

Line 57 in 3ce132f

['8.0', MySQL80Platform::class],

Are you open to pull requets to fix the behavior for 8.0 and 8?

[morozov]

Can you replace 8.0 with 8.0.0 in your settings? Platform detection is supposed to work off of the real server version, not from arbitrary user input. User input is only to avoid unnecessary connections to the database.

[ausi]

Can you replace 8.0 with 8.0.0 in your settings?

Yes, but every user would now have to alter the existing settings then.

User input is only to avoid unnecessary connections to the database.

Yes, that is why we explicitly store the version in the connection settings to avoid this unnecessary connection in production. (and because of other issues too)

[morozov]

I'm sorry that it causes extra work on your end but your connection configuration is invalid and you need to fix it.

[ausi]

So a pull request to support 8.0 and 8 would not get approved?

your connection configuration is invalid

In this case updating the documentation might help others to avoid this issue in the future, by specifying that a specific format needs to be used here. As previously ordinary version strings were supported, I think supporting them at least until version 4.0.0 would be very helpful to many users.

The docs even mention the version 8.0 explicitly: https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/platforms.html#mysql

[bohanyang]

We should keep the compatibility.
If it have to be changed, make it a breaking change in 4.0.

[morozov]

So a pull request to support 8.0 and 8 would not get approved?

No, given my current understanding of the problem.

The docs even mention the version 8.0 explicitly: https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/platforms.html#mysql

This documentation focuses on supported platforms, not the connection configuration.

[ausi]

So a pull request to support 8.0 and 8 would not get approved?

No, given my current understanding of the problem.

I think I’m not fully understanding this so far.
serverVersion=8.0 worked ever since (and was unit tested) and does not work anymore in version 3.5.0 and this is not considered a BC break?

[m-vo]

I don't want to annoy anyone but couldn't this simply be solved by comparing against the literal 8 instead of 8.0.0?

- version_compare($version, '8.0.0', '>=')
+ version_compare($version, '8', '>=')

See https://3v4l.org/1UKa2.

Unfortunately this change leads to a lot of trouble on our side if left as is. Would you be open for a PR @morozov?

[TomAdam]

This also affected me today. My Oracle MySQL server_version was 5.7 which previously was patched to 5.7.9 due to to:

https://github.com/doctrine/dbal/pull/5779/files#diff-28b47b91b6b91252a7758495476fc8b338019cccae59cbbd67bd0fe6c87529a5L85-L91

but now is taken as a version < 5.7.9 resulting in the earlier MySQLPlatform that does not support JSON types. There is a test that was specifically covering my situation that has been removed here:

https://github.com/doctrine/dbal/pull/5779/files#diff-ad520844a97906fb3e243e4c6a26747e767eb1b2a32242cb834be420f5cf6ca6L51

Also note that the docs referenced above (https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/platforms.html#mysql) only mention MySQLPlatform for >= 5.7.9 where it should be MySQLPlatform / 5.7.0 - 5.7.8 and MySQL57Platform / 5.7.9+

[greg0ire]

Also note that the docs referenced above (https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/platforms.html#mysql) only mention MySQLPlatform for >= 5.7.9 where it should be MySQLPlatform / 5.7.0 - 5.7.8 and MySQL57Platform / 5.7.9+

That's because those are the docs for v4.0 (that's what latest refers to).

[derrabus]

Reverted in #5795. Everyone who has commented here: Can you please verify that updating DBAL to "~3.5.1@dev" in your composer.json fixes the regression for you?

[ausi]

Can you please verify that updating DBAL to "~3.5.1@dev" in your composer.json fixes the regression for you?

Can confirm that 3.5.1 fixes the issue for me. Thank you! ❤️

[TomAdam]

3.5.1 fixes the issue for me too. Thanks for the rapid response.