Skip to content
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

Bug with server_version for MariaDB >10.2.7 #2985

Closed
tristanbes opened this issue Jan 22, 2018 · 41 comments
Closed

Bug with server_version for MariaDB >10.2.7 #2985

tristanbes opened this issue Jan 22, 2018 · 41 comments

Comments

@tristanbes
Copy link
Contributor

tristanbes commented Jan 22, 2018

Hello,

I think there is a problem when providing the server_version parameter.

when having a server_version set to 10.2.12, the bug discussed here is still present. (it generates endless migrations diff)

When removing the server_version parameter, it works as excepted.

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                server_version: "%database_server_version%"  // << resolves to 10.2.12

We also tried

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                server_version: 10.2.12 

Could it be that the excepted version is something else than 10.2.x ?

@lcobucci
Copy link
Member

@tristanbes could you please send us a failing test case that reproduces that behaviour? It would help us a lot to identify and fix the issue you're describing.

@tristanbes
Copy link
Contributor Author

ping @Kocal

@Kocal
Copy link

Kocal commented Jan 22, 2018

Hey!

I re-used the testing project made here by @Bukashk0zzz, in 3 different versions:

server_version Travis GitHub
'10.2.12' (quotes) Failing Branch
10.2.12 (no quotes) Passing Branch
No server_version (commented) Failed Branch

@Ocramius
Copy link
Member

Ocramius commented Jan 24, 2018 via email

@Kocal
Copy link

Kocal commented Jan 28, 2018

Hey,

I isolated the project from Symfony and this is what I found:

serverVersion Travis GitHub
'10.2.12' (quotes) Failing Branch
10.2.12 (no quotes) Failing Branch
No serverVersion (commented) Passing Branch

It seems that outside Symfony, 3rd and 4nd versions are failing ... 🤔

EDIT: I forgotten to use doctrine/dbal:dev-master, now all versions are working 😨

EDIT 2: I renamed server_version to serverVersion (thanks @nick4fake). Now the first and second tests are failing, and it's not really coherent with previous results inside Symfony context... 😞

@tristanbes
Copy link
Contributor Author

@Kocal you didn't require doctrine/dbal master, (see https://github.com/Kocal/project-doctrine-migrations-and-mariadb-issues/blob/master/composer.json#L3)

@Kocal
Copy link

Kocal commented Jan 28, 2018

Oh, I forgotten it again ... thx 👍

@nick4fake
Copy link

Still an issue with doctrine/dbal master.

Setting server_version to any version returns update bug back

@Kocal
Copy link

Kocal commented Jan 28, 2018

Inside or oustide Symfony context?

@nick4fake
Copy link

Inside Symfony context with bitnami/mariadb:10.2.12-r0 image. Commenting out version helps, but this is not something we can do (as doctrine tries to connect to db on cache:clear)

@nick4fake
Copy link

@Kocal BTW, it seems you are using "server_version" parameter in your test, but isn't the correct naming serverVersion?

Doctrine bundle in symfony translates server_version to serverVersion.

@Kocal
Copy link

Kocal commented Jan 28, 2018

Oh, you are right, I wasn't aware about this.
I'm working on it 😄

@Kocal
Copy link

Kocal commented Jan 28, 2018

Updated, now it's when I don't specify serverVersion that everything works

@tristanbes
Copy link
Contributor Author

which is the exact problem we have in our Symfony application, so you just reproduced the bug we've been experiencing inside our Symfony app 👍

@ciscospirit
Copy link

Hello,

we also have the same problem with mariadb 10.2.8 and typo3 installations.
can you give us a step to step description how to fix that issues?

I already read #2825 but i don't know how to patch my typo3 installation to get it to work...

please help me out...

@Ocramius
Copy link
Member

This still doesn't have an isolated test case - anything to be done here, or can I close it as dupe of #2825?

@ciscospirit you will need to wait for a release.

@ciscospirit
Copy link

@Ocramius thanks for your fast reply... is there no manual fix for this? like patching some files until a official fix will be released? otherwise i can just downgrade to mariadb 10.1

@tristanbes
Copy link
Contributor Author

tristanbes commented Feb 26, 2018

@Ocramius how is this a dupe of #2825 ?

It's not a duplicate of it. Right now in all our Symfony application we need to remove all server_version (or serverVersion when outside Symfony) because of some bug that you can see the result in this comment : #2985 (comment)

@Kocal spent some time isolating, testing , and reproducing inside a Symfony application. If it's not enough to qualify a bug, then ok, but closing the issue is not ok :(

@tristanbes
Copy link
Contributor Author

Also, the test made on this very same comment were made outside Symfony like you asked
#2985 (comment)

What's missing when you say "isolated test case" ?
Thanks.

@Ocramius
Copy link
Member

@Kocal spent some time isolating, testing , and reproducing inside a Symfony application. If it's not enough to qualify a bug, then ok, but closing the issue is not ok :(

What we need is something to be added in the DBAL test suite.

@tristanbes
Copy link
Contributor Author

tristanbes commented Mar 30, 2018

Can someone help me to add this failing test please ? We need this issue fixed for the doctrine/dbal 2.7.
MariaDB 10.2 has been out for 2 years already :(
I don't even know where to begin to add this failing test.

@Ocramius
Copy link
Member

@tristanbes this bit:

y and this is what I found:

serverVersion Travis GitHub
'10.2.12' (quotes) Failing Branch
10.2.12 (no quotes) Failing Branch
No serverVersion (commented) Passing Branch

Can we add testing for it in https://github.com/doctrine/dbal/blob/master/tests/Doctrine/Tests/DBAL/DriverManagerTest.php maybe? Or maybe

Specifically, what are the driver instances (class name) and platforms instances (class names) with the permutations you worked with above?

@Ocramius
Copy link
Member

MariaDB 10.2 has been out for 2 years already :(

They broke compat all over the place in a minor - can't do much about that.

@Majkl578
Copy link
Contributor

Just locally inspected the repository mentioned above. The config file is parsed in bootstrap.php and then only passed as already-loaded PHP array.
In both "with-quotes" and "without-quotes" branches the value of serverVersion is parsed as 10.2.12 string without quotes.
Since the config loading is out of DBAL's scope, this looks like a bug in some upper layer.

@tristanbes
Copy link
Contributor Author

tristanbes commented Apr 4, 2018

Following your logic @Majkl578, if there is a bug in the upper layer, wouldn't it mean that it should break when specifiying serverVersion for MySQL (let's say 5.5 or 5.6, which seems not to be the case.

@Ocramius I don't understand very well your question:

Specifically, what are the driver instances (class name) and platforms instances (class names) with the permutations you worked with above?

The only difference on the projects you linked is specifying the serverVersion (or not). We didn't change drviers/platforms between tests. The database was a
mysql Ver 15.1 Distrib 10.2.12-MariaDB, for debian-linux-gnu (x86_64) using readline 5.2

@Ocramius
Copy link
Member

Ocramius commented Apr 4, 2018

@Ocramius I don't understand very well your question:

Specifically, what are the driver instances (class name) and platforms instances (class names) with the permutations you worked with above?

Do a var_dump(get_class($connection->getDriver())) and var_dump(get_class($connection->getPlatform())) under these different configurations.

@tristanbes
Copy link
Contributor Author

tristanbes commented Apr 4, 2018

Thanks for the clarification.

Well, when giving any values to serverVersion, it takes the MySQL57Platform which is incorrect
When not specifying serverVersion it takes MariaDb1027Platform

// in ConnectionHelper.php
var_dump(get_class($connection->getDriver()), get_class($connection->getDatabasePlatform()));
serverVersion Travis Driver + DatabasePlatform
'10.2.12' ❌ Failing PDOMySql + MySQL57Platform
10.2.12 ❌ Failing PDOMySql + MySQL57Platform
No serverVersion ✅ Passing PDOMySql + MariaDb1027Platform

@tristanbes
Copy link
Contributor Author

tristanbes commented Apr 4, 2018

As pointed out by @Majkl578,
For MariaDB, the correct serverVersion is mariadb-x.x.x
You can check the condition there:

$mariadb = false !== stripos($version, 'mariadb');

Putting mariadb-10.2.12 for serverVersion is WORKING ! 👍🎉
Now i'm going to try to fix it in the docs because all mariaDB ppl are doing it wrong :D

tristanbes added a commit to tristanbes/symfonyflex-bridge that referenced this issue Apr 4, 2018
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)
javiereguiluz added a commit to symfony/symfony-docs that referenced this issue Apr 16, 2018
…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
@violuke
Copy link

violuke commented Apr 16, 2018

For me on doctrine v2.6.1 (with Symfony) we need a server_version of 10.2.14-mariadb (so the mariadb bit at the end, not the beginning. I hope this helps others.

@tristanbes
Copy link
Contributor Author

well it doesn't seem to make a difference since the condition is
$mariadb = false !== stripos($version, 'mariadb');

So as soon as mariadb is detected, it should be ok ?

@violuke
Copy link

violuke commented Apr 16, 2018

When I use it with mariadb- at the start, I get the following, but with it at the end it seems to be ok.

Invalid  platform version "mariadb-10.2.14" specified. The platform version has  to be specified in the format:  "<major_version>.<minor_version>.<patch_version>".

in DBALException.php (line 80)
at DBALException::invalidPlatformVersionSpecified('mariadb-10.2.14', '<major_version>.<minor_version>.<patch_version>')in AbstractMySQLDriver.php (line 132)
at AbstractMySQLDriver->createDatabasePlatformForVersion('mariadb-10.2.14')in Connection.php (line 399)
at Connection->detectDatabasePlatform()in Connection.php (line 338)
at Connection->getDatabasePlatform()in ConnectionFactory.php (line 93)
at ConnectionFactory->getDatabasePlatform(object(Connection))in ConnectionFactory.php (line 63)

Thanks

@tristanbes
Copy link
Contributor Author

and are you sure you are using doctrine/dbal >=2.7 @violuke

What does tell you composer show doctrine/dbal ?

@violuke
Copy link

violuke commented Apr 17, 2018

@tristanbes, no I'm using 2.6.1 of doctrine/orm and 2.6.3 of doctrine/dbal. I said I was on 2.6 originally. Does this change from 2.6 to 2.7?

name     : doctrine/dbal
descrip. : Database Abstraction Layer
keywords : database, dbal, persistence, queryobject
versions : * v2.6.3
type     : library
license  : MIT License (MIT) (OSI approved) https://spdx.org/licenses/MIT.html#licenseText
source   : [git] https://github.com/doctrine/dbal.git e3eed9b1facbb0ced3a0995244843a189e7d1b13
dist     : [zip] https://api.github.com/repos/doctrine/dbal/zipball/e3eed9b1facbb0ced3a0995244843a189e7d1b13 e3eed9b1facbb0ced3a0995244843a189e7d1b13
names    : doctrine/dbal

autoload
psr-0
Doctrine\DBAL\ => lib/

requires
doctrine/common ^2.7.1
ext-pdo *
php ^7.1

requires (dev)
phpunit/phpunit ^5.4.6
phpunit/phpunit-mock-objects !=3.2.4,!=3.2.5
symfony/console 2.*||^3.0

suggests
symfony/console For helpful console commands such as SQL execution and import of files.

Thanks

@tristanbes
Copy link
Contributor Author

tristanbes commented Apr 17, 2018

Yes, MariaDB is only supported with dbal >= 2.7

@mrtnzagustin
Copy link

I have problems with endless migration files when dbal is not detecting nulls. The problem is with doctrine/dbal 2.9.2 and mariadb 1.3.x and 1.4.x. Tried everything

@w3sami
Copy link

w3sami commented Apr 22, 2020

I'm too still (for years now) having the same issue. current versions
dbal 2.10.2
10.4.12-mariadb
symfony 5

@mrtnzagustin
Copy link

@w3sami we make a deep analysis and we were having problems with "default" values sintax in our annotations, make sure to double check that!

@w3sami
Copy link

w3sami commented Apr 22, 2020

Hi, thanks for the reply. I'm not using any default values. Probles are in every field, even in relations eg.
/**
* @Orm\OneToOne(targetEntity="App\Entity\Menu", inversedBy="Page")
* @Orm\JoinColumn(name="menu_id", referencedColumnName="id", unique=true, onDelete="CASCADE")
*/
private $Menu;
for this i always get
ALTER TABLE page CHANGE menu_id menu_id INT DEFAULT NULL
due to the NULL != 'NULL' bug.
Are you saying, you somehow went around the problem by adding default values to your entity annotations?

@mrtnzagustin
Copy link

Exactly, we had problems with default '0' vs 0 in booleans for example, and we focus to explicity add defaults values until fix the issue. It's a workaround, but it better than having buggy migrations outputs.

@w3sami
Copy link

w3sami commented Apr 22, 2020

yes, sadly it does not help for the join column as the option is to add nullable=flase, which yields the same results, as it is the default already.
BUT! I actually ran the suggested var_dump(get_class($connection->getDriver()), get_class($connection->getDatabasePlatform())); from above, and found out, that it tried to use mysql5.7, even tho I had setted the config. The problem was, that in the default symfony configs, the server version is in the .env file! I cleared that up, and now it works just fine :)

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants