Improve MySQL charset and collation comparison #5523
Conversation
morozov
force-pushed
the
issues/4631
branch
3 times, most recently
from
941005a to
e70834d
Compare
|
|
||
| self::assertContains( | ||
| 'ALTER TABLE test_column_charset_change CHANGE col_string' | ||
| . ' col_string VARCHAR(100) CHARACTER SET ascii NOT NULL', |
There was a problem hiding this comment.
This patch affects the exact DDL used to alter the table: since now the column definition being compared always has an explicit charset and collation, it leaks into the resulting DDL (COLLATION ascii_general_ci is appended).
This is not a bug, since this is the default collation for the ascii charset according to the information schema.
Keeping this DDL unchanged would require additional juggling of column options. At the same time, a functional test shouldn't make assertions against SQL.
| */ | ||
| final class CachingCharsetMetadataProvider implements CharsetMetadataProvider | ||
| { | ||
| private CharsetMetadataProvider $charsetMetadataProvider; |
There was a problem hiding this comment.
You can use constructor property promotion here and below (maybe also with the readonly keyword?)
There was a problem hiding this comment.
👍🏻 for CPP. 🙂
(maybe also with the
readonlykeyword?)
We would need to bump to PHP 8.1 for that.
There was a problem hiding this comment.
It would be nice to enforce constructor property promotion in the coding standard. Otherwise, all code merged up from older branches will remain not using this syntax.
src/Platforms/MySQL/CharsetMetadataProvider/ConnectionCharsetMetadataProvider.php
Outdated
Show resolved
Hide resolved
Closes #5338.
This is an improvement of the logic implemented in #5471. Up until this patch, the comparator didn't work correctly if the column definition contained some options that would eventually match the table options but the table options weren't explicitly specified (i.e. they defaulted to the database defaults).
In order to address the issue, we need to normalize the table and column definitions and make all the default options explicit. Then compare the normalized definitions.
Backporting the fix to 3.x would require additional effort due to the MySQL platform there enforcing made-up defaults for table collation but not for column collation:
dbal/src/Platforms/AbstractMySQLPlatform.php
Lines 460 to 462 in 8532d49
In order to support this logic, we'd need to use two implementations of the charset metadata provider: one for columns that would detect the default charset based on the information schema; and one for tables that mimicks the behavior implemented in the platform.