-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Add a note about correct cfg value server_version for MariaDB #9547
Conversation
This small change fixes 2 things: 1/ Incorrect use of `serverVersion` for MariaDB, which *must* be prefixed with `mariadb-`, see: doctrine/dbal#3083 && symfony/symfony-docs#9547 (and doctrine/dbal#2985 (comment) for the discussion) 2/ `doctrine/dbal` now supports `MariaDB >= 10.2.7`. I added the `.12` as a minor version, but we don't need to update it when platform.sh will use .13 or .14 etc.... as long as it's superior as `10.2.7` (see: https://github.com/doctrine/dbal/blob/f76bf5ef631cec551a86c2291fc749534febebf1/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php#L136)
Thanks for proposing this fix! I have some questions about this:
I ask this because, even if I disagree with lots of the decisions made by Doctrine members, I admit that they always try to do things technically correct and this solution feels a bit "hacky". My main concern is that the Thanks! |
The version itself ( For the rest of your question I guess you'll have to ask to a Doctrine maintainer |
reference/configuration/doctrine.rst
Outdated
@@ -363,7 +363,10 @@ The following block shows all possible configuration keys: | |||
to find your PostgreSQL version and ``mysql -V`` to get your MySQL | |||
version). | |||
|
|||
Always wrap the server version number with quotes to parse it as a string | |||
If you are running a MariaDB database, you should prefix the ``server_version`` | |||
with ``mariadb-`` (ex: ``server_version: mariadb-10.2.12``). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nitpicking: maybe use "e.g." instead of "ex:" for consistency with other parts of the docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@tristanbes thanks for this contribution! I've merged it so people can avoid this issue, but I've also opened an issue in DBAL repo to see if this can be solved differently (doctrine/dbal#3110). Cheers! |
…iaDB (tristanbes) This PR was squashed before being merged into the 2.7 branch (closes #9547). Discussion ---------- Add a note about correct cfg value server_version for MariaDB Because when using a MariaDB database, you should prefix the `server_version` with `mariadb-`otherwise doctrine/dbal is not able to recognize it and use `MySQL57Platform` which results in: - possible bugs - endless schema diff generation Because this feature/bug is not documented, it is time to do it properly 📦 👍 * Discussion on doctrine/dbal#2985 (comment) * Also, why it should be prefixed when using `MariaDB`: https://github.com/doctrine/dbal/blob/f76bf5ef631cec551a86c2291fc749534febebf1/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php#L135 Commits ------- e6a9c1d Add a note about correct cfg value server_version for MariaDB
Because when using a MariaDB database, you should prefix the
server_version
withmariadb-
otherwise doctrine/dbal is not able to recognize it and useMySQL57Platform
which results in:Because this feature/bug is not documented, it is time to do it properly 📦 👍
MariaDB
: https://github.com/doctrine/dbal/blob/f76bf5ef631cec551a86c2291fc749534febebf1/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php#L135