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

Deprecate fallback connection to determine platform #5707

Merged

Conversation

morozov
Copy link
Member

@morozov morozov commented Oct 1, 2022

The fallback connection only allows detection of the server version but doesn't allow anything else. E.g. it cannot be used to create the database used in the configuration that doesn't yet exist (which one would think would be the intended use case).

$connection = DriverManager::getConnection([
    'driver' => 'mysqli',
    'host'   => '127.0.0.1',
    'user'   => 'root',
    'dbname' => 'non_existing_database',
]);

echo get_class($connection->getDatabasePlatform()), PHP_EOL;
// Doctrine\DBAL\Platforms\MySQL80Platform

try {
    $connection->executeStatement('CREATE DATABASE non_existing_database');
} catch (Exception $e) {
    echo $e->getMessage(), PHP_EOL;
    // An exception occurred in the driver: Unknown database 'non_existing_database'
}

If the configured database doesn't exist, in order to create it, the user will have to use an admin connection anyway (like the test suite does). The feature seems to have worked like the above since its implementation (#2671).

Besides having questionable value and not being fully portable and not having integration tests, this code has required quite some maintenance, most recently in #4764 (comment).

@morozov morozov force-pushed the deprecate-platform-detection-fallback branch from 14891a5 to e4be6fa Compare October 1, 2022 16:03
@morozov morozov requested a review from greg0ire October 1, 2022 16:15
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering what this change would affect and I thought the command doctrine:database:create from the Symfony bundle would be it, but it appears to already be doing what you recommend here (by unsetting dbname): https://github.com/doctrine/DoctrineBundle/blob/1af518a2fd74fdf2d8b78de1a99899e44e8db8b9/Command/CreateDatabaseDoctrineCommand.php#L95-L97

@morozov morozov merged commit b377a07 into doctrine:3.5.x Oct 1, 2022
@morozov morozov deleted the deprecate-platform-detection-fallback branch October 1, 2022 17:10
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants