Doctrine\DBAL\Connection issue with "platform" key of $params array

[Perf]

Bug Report

Q	A
BC Break	no
Version	2.7.1

Summary

I have a Symfony 4.1 project with installed Doctrine packages.
I found an issue, when trying to use my custom database Platform class, which I created to workaround another issue with default MySql Platform class - when you create database it's not adding CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci to create table statement.

I created class with just one method:

namespace App\Doctrine\DBAL\Platforms;

class MySQL57Platform extends \Doctrine\DBAL\Platforms\MySQL57Platform
{
    public function getCreateDatabaseSQL($name)
    {
        return parent::getCreateDatabaseSQL($name).' CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci';
    }
}

Then added it to doctrine configuration:

doctrine:
    dbal:
        ...
        platform_service: App\Doctrine\DBAL\Platforms\MySQL57Platform
        ...

When trying to execute doctrine:database:create command it creates database with default character set and collation, but should with utf8mb4.
I performed a debug session with script \Doctrine\Bundle\DoctrineBundle\Command\CreateDatabaseDoctrineCommand and found that:

• in execute() method at line 47 $connection = $this->getDoctrineConnection($connectionName); return correct connection object with correct platform property set.
• at line 51 $params = $connection->getParams(); and $params doesn't contain platform key
• at line 83 $tmpConnection = DriverManager::getConnection($params); returns connection object without platform property set.

Tbh, I don't know why it is necessary to get another $tmpConnection instead of using already set $connection, but new connection object is created without proper platform, as $params doesn't have key platform.

Current behavior

Command doctrine:database:create doesn't use configured platform_service object.

How to reproduce

Create custom platform class (see above) and configure doctrine dbal to use it via parameter platform_service.
Execute command doctrine:database:create and check created database. It will be created with default character set and collation, instead of enforced utf8mb4 in custom Platform object.

Expected behavior

Any creation of a connection object should respect configured value of platform_service.
As a possible fix in \Doctrine\DBAL\Connection::__construct at line 234:

• remove code unset($this->_params["platform"]);

after that everything works correctly.
Or, probably method \Doctrine\Bundle\DoctrineBundle\Command\CreateDatabaseDoctrineCommand::execute should be refactored if favour of using correct $connection instead of incorrect $tmpConection object.

[morozov]

As a bug fix, I think the easiest solution would be either to remove the unset() in:

dbal/lib/Doctrine/DBAL/Connection.php

Line 234 in 11037b4

unset($this->_params["platform"]);

or append the platform to the list of returned parameters in:

dbal/lib/Doctrine/DBAL/Connection.php

Line 261 in 11037b4

return $this->_params;

As a longer term solution, we should think about getting rid of Connection::getParams() entirely since it breaks the class encapsulation: instead of just being able to initialize from the parameters, the class now is held responsible for keeping them in the original state.

At a glance, the use cases are:

1. Query cache keys are derived from the parameters (the connection could provide its own key).
2. Some master/slave and sharding functionality (the connection could implement clonning itself with() augmented parameters).
3. To be identified.

[morozov]

Actually, returning the platform (or any other object like $params['pdo']) as part of getParams() potentially can break other use cases. For instance,

dbal/lib/Doctrine/DBAL/Cache/QueryCacheProfile.php

Lines 100 to 105 in 017991a

	public function generateCacheKeys($query, $params, $types, array $connectionParams = [])
	{
	$realCacheKey = 'query=' . $query .
	'&params=' . serialize($params) .
	'&types=' . serialize($types) .
	'&connectionParams=' . hash('sha256', serialize($connectionParams));

QueryCacheProfile::generateCacheKeys() derives the cache key from connection parameters. It means that if the platform object has any state in it, the generated keys will be always different and the cache will not work.

Unless there's a non-breaking solution proposed for 2.x, I'm moving it to 3.0 where this method will be removed completely.

[Perf]

Thank you @morozov,
if you will find any non-BC workaround, please post it here.

[jwage]

Tbh, I don't know why it is necessary to get another $tmpConnection instead of using already set $connection, but new connection object is created without proper platform, as $params doesn't have key platform.

If I remember correctly, the reason is that since the connection has a defined database, when you try to connect to it to create the database, you get an error saying the database does not exist. So you need to connect without the database configured in order to create the database from the connection.

See https://github.com/doctrine/DoctrineBundle/blob/5c13e8160375fc7daa13a9602690d37a3fead31c/Command/CreateDatabaseDoctrineCommand.php#L84

[Perf]

hey @jwage
thanks for clarification!
Good to see a bugfix PR pending ))

[github-actions]

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.