Add automatic platform version detection #487

deeky666 · 2014-01-02T13:51:41Z

As more and more vendor version specific platforms evolve, it is time to make an approach of autodetecting and using the correct platform on connection. This is done by introducing two new interfaces Doctrine\DBAL\VersionAwarePlatformDriver and Doctrine\DBAL\Driver\ServerInfoAwareConnection which are implemented by Doctrine's driver and driver connection classes. They are introduced to keep BC for custom driver / driver connection classes.
VersionAwarePlatformDriver::createDatabasePlatformForVersion($version) is responsible for evaluating a given vendor specific version number and instantiating the correct platform for it. The current implementations normalize the given version string and use version_compare() for evaluation.
ServerInfoAwareConnection::getServerVersion() is responsible for returning the normalized version string of the database server currently connected to. ServerInfoAwareConnection::requiresQueryForServerVersion() defines whether receiving that information requires an additional database query or not which is important for Doctrine\DBAL\Connection as described in the following.
Doctrine\DBAL\Connection now takes an additional (optional) connection parameter serverVersion which the user can pass to skip platform autodetection and define the desired platform version straight away. This is also required for drivers that cannot return the database server version without an additional query (performance reasons). Platform version detection is now done in the following order of precedence:

Evaluate platform connection parameter and return an instance of that class if given.
Evaluate if the underlying driver is capable of creating platform instances by version. If not, return the default platform instance that would be returned by the current implementation.
Evaluate if serverVersion connection parameter is given and return the appropriate platform instance for that version.
Evaluate if the underlying driver connection can return the server version and can return it without the need of an additional query -> return the appropriate platform instance for that version.
Otherwise return the default platform instance that would be returned by the current implementation.

As a positive side effect while implementing the new interfaces, the driver classes were refactored by extracting abstract base driver classes for each vendor. This removes a lot of duplicated code in the driver classes that are related to the same database vendor. Please also note, that DrizzlePDOMySql now directly inherits from Doctrine\DBAL\Driver\PDOMySql\Driver to come around additional code duplication.

The only BC break introduced by this PR should be the visibility change of Doctrine\DBAL\Connection::$_platform from protected to private (as wished by @beberlei).
The only drawback of this implementation is that Doctrine\DBAL\Connection::getDatabasePlatform() now needs to do the real connect (if uninitialized) under some circumstances when it needs to autodetect the server version from the driver connection. Therefore I had to change the initialization of temporary connections in the functional test suite a little bit, so that calling Doctrine\DBAL\Connection::getDatabasePlatform() does not raise an error if the supplied temporary database does not exist. I think this is acceptable as it is not necessary to connect to a specific database anyways when only needing to drop and create the real test database.

As soon as the doctrine team agrees on this approach, I will add some more test and documentation for this. Until then this is a WIP PR.

doctrinebot · 2014-01-02T13:51:47Z

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-757

We use Jira to track the state of pull requests and the versions they got
included in.

deeky666 · 2014-01-02T13:51:58Z

@doctrine/team-doctrine2 review please :)

stof · 2014-01-02T13:57:53Z

lib/Doctrine/DBAL/Connection.php

@@ -1521,7 +1588,7 @@ public function ping()
        }

        try {
-            $this->query($this->_platform->getDummySelectSQL());
+            $this->query($this->platform->getDummySelectSQL());


shoudln't this use the getter to ensure it has been created ?

@stof connect() ensures that it is set which called at the beginning of the method.

deeky666 · 2014-02-08T18:31:49Z

@beberlei rebased with master and removed version awareness for DB2, Oracle and SQLite. Otherwise what else (except for tests) is missing here? Do you think the implementation is ready to go? Then I would start adding tests.

beberlei · 2014-02-09T10:12:50Z

@deeky666 Start with the tests I guess, then we can release another Beta after merging this.

…ersion detection

deeky666 · 2014-02-12T18:24:29Z

@doctrine/team-doctrine2 finished this beast. Please review. Hope you like it. Cheers!

Ocramius · 2014-02-12T19:21:12Z

lib/Doctrine/DBAL/Connection.php

@@ -365,6 +360,10 @@ public function connect()
        $this->_conn = $this->_driver->connect($this->_params, $user, $password, $driverOptions);
        $this->_isConnected = true;

+        if (null === $this->platform) {


Just call $this->getDatabasePlatform()

deeky666 · 2014-02-12T22:29:27Z

travis hates me -.- why don't I get this error?

Ocramius · 2014-02-12T22:33:39Z

lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php

+    public function getServerVersion()
+    {
+        if ( ! preg_match('/\s+(\d+\.\d+\.\d+\.\d+\.\d+)\s+/', oci_server_version($this->dbh), $version)) {
+            return null;


null is not a valid return value - this should throw an exception.

Hmmm good point. I would have to add it to the contract then. But that seems rather stupid because I would have to throw some kind of LogicException as it is not a driver/database problem but rather an "unknown version string". You know what I mean? I cannot 100% tell what each version string looks like that the driver returns because it heavily depends on the database server. For Oracle I did the null return here because Oracle seems to like returning something like "Oracle Enterprise blabla Edition III.2 with a lot of extra features for commercial use ---version---. Thank you for paying a lot of money using this sh*t." as a "version" string. What would you suggest putting in the contract? Please also note BC when thinking of throwing an exception here.

Add automatic platform version detection

…drianolek) This PR was squashed before being merged into the 1.3.x-dev branch (closes #329). Discussion ---------- Added support for server_version connection parameter Doctrine DBAL 2.5 will support platform version autodetection, additionally it allows specifying serverVersion connection parameter: > which the user can pass to skip platform autodetection and define the desired platform version straight away. This is also required for drivers that cannot return the database server version without an additional query (performance reasons). > doctrine/dbal#487 In symfony it can be useful eg. when warming up cache during build, so you don't need the db connection, but dbal 2.5 will try to autodetect server version anyway eg. during entity class metadata loading, then it will throw an exception because of unavailable connection. server_version parameter can be used to skip the autodetection. Commits ------- 523b59a Added support for server_version connection parameter

klaussilveira · 2014-12-05T01:42:34Z

This introduces a heavy BC for Symfony users. When using the clear:cache command with the warmup enabled, Doctrine's ProxyCacheWarmer is called and tries to connect to the database. If there is no database configured properly, or running, everything crashes. Including the composer install. All our Jenkins builds started to fail after upgrading to DBAL 2.5.0. Jenkins tries to run composer install before running our test suite and crashes hard.

In order to make it work, i had to manually add the following option to the Doctrine Bridge configuration: server_version: "5.6"

👎

Ocramius · 2014-12-05T11:10:00Z

In order to make it work, i had to manually add the following option to the Doctrine Bridge configuration: server_version: "5.6"

You can survive that :-)

Further discussion on this bug should happen in http://www.doctrine-project.org/jira/browse/DBAL-1057.

gerryvdm · 2014-12-12T15:03:46Z

In the same boat as @klaussilveira, this gave us a hell of a headache on this Friday afternoon :)

Anyway, wrote a blogpost on it:

http://www.king-foo.be/2014/12/connection-errors-after-upgrading-to-doctrine-dbal-2-5/

…implementation detail Testing the implementations of this method requires partial mocking of the implementing class which makes it impossible to make them `final` (doctrine#3590). Additionally, this API breaks the encapsulation of the driver layer: instead of exposing the fact of whether The rationale behind introducing this method (doctrine#487) is really questionable: > This is also required for drivers that cannot return the database server version without an additional query (performance reasons). 1. There's no evidence that an underlying driver that exposes the server version via its API doesn't make a request of any kind to the server. 2. For an application that works with any realistic database, a query like `SELECT VERSION()` wouldn't be a performance bottleneck. 3. Even if it was, it's always possible to specify the platform version upfront. In addition to the above, the only driver that doesn't support the platform version detection via the underlying driver API is `sqsql` which is barely supported.

…implementation detail Testing the implementations of this method requires partial mocking of the implementing class which makes it impossible to make them `final` (doctrine#3590). Additionally, this API breaks the encapsulation of the driver layer: instead of exposing the fact of whether the connection will perform a query to detect the server version, the driver should just instantiate a platform corresponding to a connection. The rationale behind introducing this method (doctrine#487) is really questionable: > This is also required for drivers that cannot return the database server version without an additional query (performance reasons). 1. There's no evidence that an underlying driver that exposes the server version via its API doesn't make a request of any kind to the server. 2. For an application that works with any realistic database, a query like `SELECT VERSION()` wouldn't be a performance bottleneck. 3. Even if it was, it's always possible to specify the platform version upfront. In addition to the above, the only driver that doesn't support the platform version detection via the underlying driver API is `sqsql` which is barely supported.

…implementation detail Testing the implementations of this method requires partial mocking of the implementing class which makes it impossible to make them `final` (doctrine#3590). Additionally, this API breaks the encapsulation of the driver layer: instead of exposing the fact of whether the connection will perform a query to detect the server version, the driver should just instantiate a platform corresponding to a connection. The rationale behind introducing this method (doctrine#487) is really questionable: > This is also required for drivers that cannot return the database server version without an additional query (performance reasons). 1. There's no evidence that an underlying driver that exposes the server version via its API doesn't make a request of any kind to the server. 2. For an application that works with any realistic database, a query like `SELECT VERSION()` wouldn't be a performance bottleneck. 3. Even if it was, it's always possible to specify the platform version upfront. Otherwise, the current logic of falling back to a default platform may cause undefined behavior of the application (we don't test the compatibility of the lowest level of the DBAL platform with all supported server versions). Remember, “If it doesn’t work, it doesn’t matter how fast it doesn’t work.” In addition to the above, the only driver that doesn't support the platform version detection via the underlying driver API is `sqsql` which is barely supported.

…n implementation detail Testing the implementations of this method requires partial mocking of the implementing class which makes it impossible to make them `final` (doctrine#3590). Additionally, this API breaks the encapsulation of the driver layer: instead of exposing the fact of whether the connection will perform a query to detect the server version, the driver should just instantiate a platform corresponding to a connection. The rationale behind introducing this method (doctrine#487) is really questionable: > This is also required for drivers that cannot return the database server version without an additional query (performance reasons). 1. There's no evidence that an underlying driver that exposes the server version via its API doesn't make a request of any kind to the server. 2. For an application that works with any realistic database, a query like `SELECT VERSION()` wouldn't be a performance bottleneck. 3. Even if it was, it's always possible to specify the platform version upfront. Otherwise, the current logic of falling back to a default platform may cause undefined behavior of the application (we don't test the compatibility of the lowest level of the DBAL platform with all supported server versions). Remember, “If it doesn’t work, it doesn’t matter how fast it doesn’t work.”

stof reviewed Jan 2, 2014
View reviewed changes

deeky666 added 10 commits February 12, 2014 19:17

add support for evaluating the correct platform to use in a connection

3176f51

extract abstract base driver classes for vendors

f12cf9c

remove unnecessary parenthesis

12132c5

fix database version string evaluation for SQL Server

70ea43a

fix wrapping PDO exception properties

b7d6303

add tests for driver abstraction refactoring and automatic platform v…

fb26c17

…ersion detection

move mysqli connection test to new namespace

802fd80

document automatic platform version detection

180ea45

add upgrade instructions due to automatic platform version detection

f43bebc

add automatic platform version detection for PostgreSQL 9.1

d68603c

Ocramius reviewed Feb 12, 2014
View reviewed changes

skip tests for uninstalled extensions

cd68d48

deeky666 added 4 commits February 13, 2014 00:32

revert PDO exception wrapping tests

d165e52

throw exception on unexpected Oracle database version string

6498bfd

add doc blocks for abstract methods

a8ca780

simplify database platform retrieval

61eb1ee

deeky666 referenced this pull request Feb 14, 2014

refactor, consolidate and extend driver exception conversion

262deeb

beberlei added a commit that referenced this pull request Feb 16, 2014

Merge pull request #487 from deeky666/add-platform-version-switch

b362492

Add automatic platform version detection

beberlei merged commit b362492 into doctrine:master Feb 16, 2014

deeky666 mentioned this pull request Mar 23, 2014

Fix truncate on MySQL >= 5.5 #549

Closed

adrianolek mentioned this pull request Sep 4, 2014

Added support for server_version connection parameter doctrine/DoctrineBundle#329

Closed

This was referenced Dec 6, 2015

DDC-2092: [GH-487] Fix DDC-2069 doctrine/orm#2776

Closed

DBAL-757: Add automatic platform version detection #1983

Closed

kimhemsoe mentioned this pull request Oct 12, 2016

Add native JSON type support for MySQL >=5.7.8 #2266

Closed

morozov mentioned this pull request Jan 3, 2020

Removed ServerInfoAwareConnection#requiresQueryForServerVersion() as implementation detail #3807

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add automatic platform version detection #487

Add automatic platform version detection #487

deeky666 commented Jan 2, 2014

doctrinebot commented Jan 2, 2014

deeky666 commented Jan 2, 2014

stof Jan 2, 2014

deeky666 Jan 2, 2014

deeky666 commented Feb 8, 2014

beberlei commented Feb 9, 2014

deeky666 commented Feb 12, 2014

Ocramius Feb 12, 2014

deeky666 commented Feb 12, 2014

Ocramius Feb 12, 2014

deeky666 Feb 12, 2014

klaussilveira commented Dec 5, 2014

Ocramius commented Dec 5, 2014

gerryvdm commented Dec 12, 2014

Add automatic platform version detection #487

Add automatic platform version detection #487

Conversation

deeky666 commented Jan 2, 2014

doctrinebot commented Jan 2, 2014

deeky666 commented Jan 2, 2014

stof Jan 2, 2014

Choose a reason for hiding this comment

deeky666 Jan 2, 2014

Choose a reason for hiding this comment

deeky666 commented Feb 8, 2014

beberlei commented Feb 9, 2014

deeky666 commented Feb 12, 2014

Ocramius Feb 12, 2014

Choose a reason for hiding this comment

deeky666 commented Feb 12, 2014

Ocramius Feb 12, 2014

Choose a reason for hiding this comment

deeky666 Feb 12, 2014

Choose a reason for hiding this comment

klaussilveira commented Dec 5, 2014

Ocramius commented Dec 5, 2014

gerryvdm commented Dec 12, 2014