Using a user-provided PDO connection breaks assertion

[morozov]

Q	A
Regression	yes
Version	2.10.0-dev

The following code is expected to work:

var_dump(
    DriverManager::getConnection([
        'pdo' => new PDO('sqlite::memory:'),
    ])->ping()
);

// bool(true)

On master, it currently produces:

Warning: assert(): assert($this->_conn instanceof DriverConnection) failed in lib/Doctrine/DBAL/Connection.php on line 1429

bool(true)

On develop:

Warning: assert(): assert($this->_conn instanceof DriverConnection) failed in /home/morozov/Projects/dbal/lib/Doctrine/DBAL/Connection.php on line 1374

TypeError: Return value of Doctrine\DBAL\Connection::query() must be an instance of Doctrine\DBAL\Driver\ResultStatement, instance of PDOStatement returned in lib/Doctrine/DBAL/Connection.php on line 995

The existing implementation relies on weak/duck typing since instead of injecting the PDO instance into PDOConnection, the latter gets replaced. Besides the poor design, this "feature" is completely untested.

We can loosen or remove the assertion from master but before fixing it in develop I'd like to understand the use cases this feature was implemented for.

[lcobucci]

We can loosen or remove the assertion from master but before fixing it in develop I'd like to understand the use cases this feature was implemented for.

I think that loosing doesn't address the root issue. Passing a PDO instance is kind of a can of worms that we barely support.

There're a couple use cases that I can think of:

• having a PDO instance with arbitrary attributes: imagine that in a long long time ago (pre v2.x) the driverOptions argument was not a thing
• legacy software: reusing a connection opened outside of DBAL/ORM control (not opening multiple transactions and all)

I think that the way we implemented is definitely not ideal, IMHO we should rather have an adapter to our interface (that wraps the instance).

Now, looking at the future (v3 or v4), maybe we should drop it?

[jwage]

I've seen this used quite a bit in apps when migrating to Doctrine. They'll have an existing PDO connection open already or some other ORM with a PDO connection. If we don't support it, then they would be forced to have two connections open. I talked to @morozov a few days ago about this and I am going to take a look at properly refactoring this for 3.0.

[morozov]

What if we just recommend an approach of extending PDO-specific DBAL objects in a project-specific way and maintaining it outside of DBAL. The approach of using DBAL in the applications with existing DB layers is not unique to PDO. E.g. at our company, we use an existing connection with mysqli, oci8, ibm_db2 and sqlsrv drivers in DBAL w/o official support from the latter.

While I do understand the use case, I'd rather help solve it outside the DBAL instead of maintaining a non-portable and not-tested behavior.

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