ensureForwardCompatibilityStatement broke existing code

[bizurkur]

Bug Report

This is likely a low priority.

Q	A
BC Break	yes?
Version	2.13.1, but it looks like 2.12.x branch has the issue too. 2.12.1 worked fine

Summary

Commit 4297f50 broke Connection when passing in $params['pdo']

#4590 says it should have fixed the issue, but I'm using 2.13.1 and it still seems to be broken.

Current behaviour

If you build a Connection object using an already established $params['pdo'] connection, commit 4297f50 caused breaking changes that no longer allow that behavior. Now you get the following error:

An exception occurred while executing 'SHOW FULL TABLES WHERE Table_type = 'BASE TABLE'':  Argument 1 passed to Doctrine\DBAL\Connection::ensureForwardCompatibilityStatement() must be an instance of Doctrine\DBAL\Driver\ResultStatement, instance of PDOStatement given, called in /var/www/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php on line 1314

How to reproduce

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Driver\PDO\MySQL\Driver;

$params = [
    'pdo' => new \PDO('mysql:<whatever>'),
];
$driver = new Driver();

$connection = new Connection($params, $driver);
// This is a query that Doctrine Migrations tool does:
$connection->executeQuery("SHOW FULL TABLES WHERE Table_type = 'BASE TABLE'");

• Within the executeQuery() method, it calls $connection = $this->getWrappedConnection(); which is returning the raw PDO connection.
• Later in the method it calls $stmt = $connection->prepare($sql); which is a raw PDOStatement object.
• At the end of the method the referenced commit changed the code to return $this->ensureForwardCompatibilityStatement($stmt); and it expects $stmt to be an instance of ResultStatement, but of course it's the raw PDOStatement instead.

Expected behaviour

No error to be thrown

[bizurkur]

A possible solution: Modify the change made in #4590 but call $params['pdo']->setAttribute(\PDO::ATTR_STATEMENT_CLASS, [PDODriverStatement::class, []]); inside the Doctrine\DBAL\Connection class instead of the Doctrine\DBAL\DriverManager class. Thoughts?

This is basically what I did as a temporary fix (call the setAttribute() code before passing into Connection), but I'm not sure if design-wise there's a reason it wasn't added inside Connection in the previous fix.

[michaljusiega]

I Can confirm same issue with custom PDO.

[greg0ire]

@mdumoulin can you please advise?

[alexander-schranz]

We have the same problem when using when using https://github.com/crate/crate-dbal implementation which is build on top of https://github.com/crate/crate-pdo which errors with following message:

  Argument 1 passed to Doctrine\DBAL\Connection::ensureForwardCompatibilitySt  
  atement() must be an instance of Doctrine\DBAL\Driver\ResultStatement, inst  
  ance of Crate\PDO\PDOStatement given, called in /vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php on line 1314     

[mdumoulin]

@alexander-schranz The issue is on crate project side, the crate driver connection should implements DBAL\Driver\Connection interface : https://github.com/doctrine/dbal/blob/2.13.x/lib/Doctrine/DBAL/Driver/Connection.php

To be specific, the PDOConnection::exec() method should return a DBAL\Driver\Statement object instead of a \PDOStatement object to be compatible with DBAL.

You may take inspiration of Driver\PDOConnection for solution : https://github.com/doctrine/dbal/blob/2.13.x/lib/Doctrine/DBAL/Driver/PDOConnection.php#L40

[alexander-schranz]

@mdumoulin Thx for the response. The Connection interface is used over the ServerInfoAwareInterface. I hope crate pdo support that attribute for the statement, tried to fix it in there repository.

[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.