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 driver name aliases #5697

Merged
merged 1 commit into from
Sep 27, 2022

Conversation

morozov
Copy link
Member

@morozov morozov commented Sep 27, 2022

The aliases were initially implemented in #729 as part of the support for connection URLs but there wasn't any public discussion about their design and purpose.

The existing set of aliases is quite illogical and raises questions:

  1. The mysql alias prefers the pdo_mysql driver over mysqli. Why?
  2. The "MS SQL" and "SQL Server" terms are quite interchangeable but using sqlsrv in the schema will make the sqlsrv driver used while mssql is mapped to pdo_sqlsrv. Why?
  3. The mysql2 alias was introduced for
    'mysql2' => 'pdo_mysql', // Amazon RDS, for some weird reason
    Why should the DBAL support the schema used by AWS RDS?
  4. If the aliases are needed for compatibility with the driver naming of other database-related APIs, how do we validate and maintain the compatibility, and which APIs should the DBAL be compatible with?

Judging by the documentation, the only purpose of using an alias is to avoid using dashes or underscores in connection URLs. Even if that's the case, why do we need more than one alias per driver name? Being able to do one thing in four different ways (pgsql, postgres, postgresql, pdo-pgsql) has rarely been a good idea.

Given that the requirement for using dashes has been documented since the very beginning, I don't believe that this is a problem that requires support for aliases.

@morozov morozov added this to the 3.5.0 milestone Sep 27, 2022
@morozov morozov marked this pull request as ready for review September 27, 2022 01:00
src/DriverManager.php Outdated Show resolved Hide resolved
src/DriverManager.php Outdated Show resolved Hide resolved
@derrabus
Copy link
Member

Being able to do one thing in four different ways (pgsql, postgres, postgresql, pdo-pgsql) has rarely been a good idea.

Even worse: pgsql:// might suggest that we're connecting through ext-pgsql while we're actually using PDO. And if we ever were to develop a driver for ext-pgsql (btw: do we want one?), we'd either need to use a non-intuitive URL schema for that or reassign pgsql:// which would be a breaking change.

So 👍🏻 from my side!

@morozov morozov force-pushed the deprecate-driver-name-aliases branch from e1147ab to 3cedf31 Compare September 27, 2022 14:49
@morozov
Copy link
Member Author

morozov commented Sep 27, 2022

btw: do we want one?

I believe at some point the DBAL supported a non-PDO version of the SQLite or PostgreSQL driver. Not sure what happened but unless it brings in some maintenance issues, I don't see a problem adding it. In my experience, the non-PDO drivers provide a nicer API and allow the usage of more features (says a maintainer of the DBAL).

@morozov morozov merged commit 6bad1f2 into doctrine:3.5.x Sep 27, 2022
@morozov morozov deleted the deprecate-driver-name-aliases branch September 27, 2022 15:07
@stof
Copy link
Member

stof commented Oct 24, 2022

the purpose of those aliases was to support credential URLs provided by common PaaS or IaaS platforms, to make it easier to use doctrine/dbal on them (without having to build a scheme-remapping system in the reading of env variables. Those hosting providers are not aware of doctrine/dbal when exposing those URLs (those hosting providers are often not even specific to PHP)

@derrabus
Copy link
Member

derrabus commented Oct 24, 2022

The problem is that for each database engine (with the exception of Postgres currently) we have two possible drivers that can connect to that database. And it could be more if the application used a custom driver. The decision which of those drivers to use should be made deliberately by the application, especially if the application accesses the native connection in some situations. Right now, the driver being used is the result of a chain of coincidences.

Even worse: Some of the aliases are identical to the name of an existing PHP extension (like sqlite3, pgsql) while in fact they point to the corresponding PDO drivers. In case of sqlite3, we have just added a driver for that extension and removing the alias, which would effectively remap the sqlite3:// scheme from ext-PDO_sqlite to ext-sqlite3 would mean a breaking change.

I understand the problem, but I would still support the deprecation of those hardcoded aliases. If guessing DBAL connection settings from generic database DSNs is a common problem, maybe we can think about moving the DSN parsing to a dedicated class and provide a way to map schemes to drivers. Alternatively, framework integration layers like DoctrineBundle could take over this task.

@stof
Copy link
Member

stof commented Oct 24, 2022

If we specify the driver option explicitly alongside the url, will those schemes still work (favoring the explicit driver above the one that cannot be guessed from the scheme due to not matching) ? Or would the scheme win, giving no chance to configure the driver separately ?
Having to specify the driver option separately would be fine to me (I don't have a use case for dynamically switching driver when the hosting provider updates the credentials in the env variable, as such case will not magically change the platform)

@derrabus derrabus mentioned this pull request Nov 7, 2022
@derrabus derrabus mentioned this pull request Dec 30, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 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.

4 participants