provides default dsn value for charset in factories #56
Conversation
|
wrong for postgres - build fails |
|
must be something like: |
|
how about to provide the dsn from the entire factory instead of in the abstract one? |
|
Changes Unknown when pulling 1785398 on oqq:improvement/charset_in_factories into ** on prooph:master**. |
|
Here is how doctrine get around this.. |
|
I have added my proposal and have extended tests. Please review again @prolic . |
| public function defaultOptions(): iterable | ||
| { | ||
| return [ | ||
| 'connection_options' => [ | ||
| 'driver' => 'pdo_pgsql', |
tests/TestUtil.php
Outdated
| @@ -45,6 +45,7 @@ public static function getConnection(): PDO | |||
| $dsn .= 'dbname=' . $connectionParams['dbname'] . $separator; | |||
| $dsn = rtrim($dsn); | |||
| self::$connection = new PDO($dsn, $connectionParams['user'], $connectionParams['password']); | |||
| self::$connection->query("SET NAMES '".$connectionParams['charset']."'"); | |||
There was a problem hiding this comment.
I have not found a connection option which would work for postgre. Thats the way how doctrine is bypass this issue.
| ); | ||
|
|
||
| if (isset($config['connection_options']['charset'])) { | ||
| $connection->query("SET NAMES '".$config['connection_options']['charset']."'"); |
There was a problem hiding this comment.
The connection option for charset is used in mysql factory. This is a workaround for postgre.
|
Try options='--client_encoding=UTF8' for PostgreSQL |
|
@prolic any reason why you have used a space as separator for postgre? It seems that postgre is fine with ';' as separator. |
As far as I know ; doesn't work for postgres. |
If anyone trusts the default config (e.g. me), than this person could get bad experience with the mysql event store without this PR.
json_decode() would return null with an invalid json string, e.g. a not correct utf-8 encoded string.
Which would end up with
Assert\InvalidArgumentException: payload must be an array.Anyway.. if there is no reason why the default charset should not be utf-8, this PR should be merged before next stable release to prevent for bc breaks.