Wrap pdo instances #3544
Wrap pdo instances #3544
Conversation
lcobucci
force-pushed
the
wrap-pdo-instances
branch
from
2bcf0f4
to
8970af9
Compare
| @@ -189,7 +190,7 @@ public function __construct( | |||
| $this->params = $params; | |||
|
|
|||
| if (isset($params['pdo'])) { | |||
| $this->_conn = $params['pdo']; | |||
| $this->_conn = WrappedPDOConnection::fromInstance($params['pdo']); | |||
There was a problem hiding this comment.
It still would be a breaking change since it's no longer a PDO instance (won't match a type hint, can't call PDO-specific methods, etc).
There was a problem hiding this comment.
Not according to our docblocks...
There was a problem hiding this comment.
We never expect to handle raw PDO connection in the internals
There was a problem hiding this comment.
True. I didn't notice it was about the internal API. But still, even if we can afford introducing this class, I don't think we should. Having two PDO connections which share most of the code is not a proper solution. I'd like to propose #3549 instead.
There was a problem hiding this comment.
@morozov I'm fine with not adding this adapter 😄 I'd suggest to port the tests to your PR, just to ensure that the basic stuff works fine.
| use function assert; | ||
| use function func_get_args; | ||
|
|
||
| final class WrappedPDOConnection implements Connection, ServerInfoAwareConnection |
There was a problem hiding this comment.
I'm not really happy naming-wise and am open for suggestions.
PDOConnectionAdapter?
|
Closed as per #3544 (comment) |
Summary
This is an alternative of #3543 created by @alcaeus and @nicolas-grekas.
I'm not really happy naming-wise and am open for suggestions.