Fix creating wrapped connection twice #1087
Conversation
| $params['charset'] = 'utf8'; | ||
| $driver = $connection->getDriver(); | ||
| $wrapperClass = Connection::class; | ||
| if (isset($params['wrapperClass'])) { |
There was a problem hiding this comment.
Not a fan of inlining this logic here, that's what's DriverManager for. Other places using it are still having old behaviour and in case we update DriverManager, it can break ConnectionFactory, since it unsets wrapperClass option.
There was a problem hiding this comment.
I agree this should be ported to DriverManager, but fixing this here was the faster choice for Symfony users. Once this is fixed in DBAL 2.9, we can remove it here and bump the doctrine/dbal dependency.
|
Test added. About your other comments @ostrolucky I didn't understand what you're suggesting instead. To me, this effectively solves the reported issue. And I can't think of any alternative implementation. Do you? |
|
My point was it should be fixed in Besides this being ugly fix (34, poorly tested LOC with half code copy pasted; which could be < 10 in DBAL), it still instantiates original Connection class twice so by avoiding fixing it in DBAL we are implementing subpar solutions. I believe #1070 causes other unreported issues as well. Since we are no longer touching recipe which is generated only at beginning of installing bundle, it changes charset for everybody not specifying one from usually default Perhaps we can also just ignore this problem for a little while longer, since MySQL 8 fixed this itself by defaulting to |
Indeed, fixed. |
|
Thanks @nicolas-grekas! |
As explained in #1070 (comment)