Add support for collation option to MySQL #5246
Conversation
Our documentation advertises it: https://www.doctrine-project.org/projects/doctrine-dbal/en/3.3.1/reference/schema-representation.html The option "collate" stays supported and should be deprecated in 3.4
| 'my_collation', | ||
| $this->platform->getCreateTableSQL($table)[0] | ||
| ); | ||
| } |
There was a problem hiding this comment.
Although both tests look a lot like each other, I elected not to use a data provider here, as the second test should diverge in 3.4 because of the deprecation.
|
@mlocati please review |
| // Collate | ||
| if (! isset($options['collate'])) { | ||
| $options['collate'] = $options['charset'] . '_unicode_ci'; | ||
| if (isset($options['collate'])) { |
There was a problem hiding this comment.
If the collate and collation keys are both defined, here collate takes the precedence over collation.
If we want to deprecate collate in favor of collation I'd do the opposite.
There was a problem hiding this comment.
Maybe I should instead throw if both are defined? Or if both are defined but different? I don't think that it would make sense to have both defined but different. Having both defined but the same might make sense in situation where you want to support 2 versions of DBAL (me might want to do so in DoctrineBundle).
There was a problem hiding this comment.
I like the idea of throwing if they are different. It would also be nice to trigger a deprecation warning if collate is defined.
There was a problem hiding this comment.
I'd refrain from throwing new exceptions and triggering deprecations in a patch release. It's also fine that "collate" takes precedence thereby providing less disruption during the upgrade.
Our documentation advertises it:
https://www.doctrine-project.org/projects/doctrine-dbal/en/3.3.1/reference/schema-representation.html
The option
collatestays supported and should be deprecated in 3.4Fixes #5214
Fixes #5243