[5.7] Makes sure changing a database field to JSON does not include a collation#25741
[5.7] Makes sure changing a database field to JSON does not include a collation#25741taylorotwell merged 2 commits intolaravel:5.7from
Conversation
|
This fix would affect all databases while the original issue is only about Mysql. Is the collation unsupported in other databases too? |
|
@sisve The DBAL docs mention that collation is supported by MySQL, PostgreSQL, Sqlite, SQL Server and Drizzle. I've asked the maintainers if that parameter is silently ignored in other engines or if it renders invalid SQL. The former would pose no problems while the latter would force us to detect the engine and execute the conditional in my PR accordingly (which would suck big time). |
|
@sisve, I just got word back from the guys over at DBAL and can confirm that unsupported database engines silently ignores a passed collation so this change on the Laravel side will not have detrimental effects on such engines. The actual conditional can be found here: https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php#L2642-L2654 (the whole process starts here though: https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php#L2278-L2308) |
|
As a temporary workaround I found out that by exploiting the fact that you can pass anything to a Fluent command, a migration can explicitly pass a $table->json('my_column')->customSchemaOptions(['collation' => ''])->change();This would force DBAL to not use |
|
My 2 cents is that this is the appropriate solution. The end user should be responsible for specifying all the necessary data required as I think it will be very hard and error prone to handle all the different possible combinations of column types and collations across the different database types. |
|
For whoever is wondering, I couldn't get this working with only setting the collation. I also had to change charset. For example |
Full issue is discussed here: #25735