Skip to content

Fix MariaDB fetching of default table character-set#6425

Merged
derrabus merged 1 commit intodoctrine:3.8.xfrom
grooverdan:fix_6361
Jun 7, 2024
Merged

Fix MariaDB fetching of default table character-set#6425
derrabus merged 1 commit intodoctrine:3.8.xfrom
grooverdan:fix_6361

Conversation

@grooverdan
Copy link
Copy Markdown
Contributor

@grooverdan grooverdan commented Jun 7, 2024 •
edited
Loading

Q A
Type bug
Fixed issues #6361

Summary

From MariaDB-10.10.1, where uca1400 was added, the information_schema.COLLATION_CHARACTER_SET_APPLICABILITY was extended to have FULL_COLLATION_NAME which corresponds to the information_schema.TABLES.TABLE_COLLATION value.

Executable comment syntax is used to limited to the applicable versions.

To preserve compatibility with older MariaDB versions, and MySQL versions where the previous COLLATION_NAME was the match is left as a JOIN criteria. In new MariaDB versions this won't result in an extra row match.

Closes: #6361

The lack of this fix did case the CI test to fail with a MariaDB-11.0+ container:

1) Doctrine\DBAL\Tests\Functional\Schema\MySQLSchemaManagerTest::testEnsureTableWithoutOptionsAreReflectedInMetadata
Undefined array key "engine"

/home/runner/work/dbal/dbal/src/Schema/Table.php:927
/home/runner/work/dbal/dbal/tests/Functional/Schema/MySQLSchemaManagerTest.php:550

With this fix all version of MySQL and MariaDB (even those new ones soon to be in a different PR) will pass like: https://github.com/grooverdan/dbal/actions/runs/9412110648/job/25926481038

@grooverdan grooverdan changed the base branch from 4.0.x to 3.8.x June 7, 2024 05:47
@grooverdan grooverdan mentioned this pull request Jun 7, 2024
Merged
@grooverdan
Copy link
Copy Markdown
Contributor Author

grooverdan commented Jun 7, 2024

SQL example:

MariaDB [test]> select version();
+----------------+
| version()      |
+----------------+
| 11.5.2-MariaDB |
+----------------+
1 row in set (0.001 sec)


MariaDB [test]> create table t (c varchar(30)) charset 'utf32' collate 'uca1400_ai_ci';
Query OK, 0 rows affected (0.002 sec)

MariaDB [test]> SELECT t.TABLE_NAME,            t.ENGINE,            t.AUTO_INCREMENT,            t.TABLE_COMMENT,            t.CREATE_OPTIONS,            t.TABLE_COLLATION,            ccsa.CHARACTER_SET_NAME       FROM information_schema.TABLES t         INNER JOIN information_schema.COLLATION_CHARACTER_SET_APPLICABILITY ccsa             ON /*M!101001 ccsa.FULL_COLLATION_NAME = t.TABLE_COLLATION OR */ COLLATION_NAME = t.TABLE_COLLATION WHERE t.TABLE_NAME='t';
+------------+--------+----------------+---------------+----------------+---------------------+--------------------+
| TABLE_NAME | ENGINE | AUTO_INCREMENT | TABLE_COMMENT | CREATE_OPTIONS | TABLE_COLLATION     | CHARACTER_SET_NAME |
+------------+--------+----------------+---------------+----------------+---------------------+--------------------+
| t          | InnoDB |           NULL |               |                | utf32_uca1400_ai_ci | utf32              |
+------------+--------+----------------+---------------+----------------+---------------------+--------------------+

@grooverdan
Fix MariaDB fetching of default table character-set (#6361)
5ff986d 
From MariaDB-10.10.1, where uca1400 was added, the
information_schema.COLLATION_CHARACTER_SET_APPLICABILITY was
extended to have FULL_COLLATION_NAME which corresponds to
the information_schema.TABLES.TABLE_COLLATION value.

Executable comment syntax is used to limited to the applicable
versions.

To preserve compatibility with older MariaDB versions, and
MySQL versions where the previous COLLATION_NAME was the match
is left as a JOIN critieria. In new MariaDB versions this won't
result in an extra row match.

Closes: #6361
derrabus
derrabus reviewed Jun 7, 2024
View reviewed changes
Comment thread src/Schema/MySQLSchemaManager.php
Comment on lines -570 to +576
ON ccsa.COLLATION_NAME = t.TABLE_COLLATION
ON /*M!101001 ccsa.FULL_COLLATION_NAME = t.TABLE_COLLATION OR */
ccsa.COLLATION_NAME = t.TABLE_COLLATION
Copy link
Copy Markdown
Member

@derrabus derrabus Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to avoid version conditionals in SQL. We have our platform class hierarchy for this purpose.

I'll accept this change as a hotfix for MariaDB 11.4 on the 3.8.x branch. Can you work on a PR for 3.9.x that moves the SQL statement to AbstractMySQLPlatform and that introduces a new platform class for MariaDB 10.10 which overrides the generated SQL?

@derrabus derrabus added this to the 3.8.5 milestone Jun 7, 2024
@derrabus derrabus merged commit 4aa9cf9 into doctrine:3.8.x Jun 7, 2024
@derrabus derrabus changed the title Fix MariaDB fetching of default table character-set (#6361) Fix MariaDB fetching of default table character-set Jun 7, 2024
derrabus added a commit that referenced this pull request Jun 7, 2024
@derrabus
Merge branch '3.8.x' into 3.9.x
df1bd13 
* 3.8.x:
  Fix MariaDB fetching of default table character-set (#6361) (#6425)
  Fix the portability documentation (#6429)
  Update tests/Platforms/AbstractPlatformTestCase.php
  Update tests/Platforms/AbstractPlatformTestCase.php
  add test
  Fix: Skip type comparison if disableTypeComments is true
derrabus pushed a commit that referenced this pull request Jun 8, 2024
@grooverdan
CI: Update MariaDB versions (#6426)
0e3536b 
<!-- Fill in the relevant information below to help triage your pull
request. -->

|      Q       |   A
|------------- | -----------
| Type         | improvement
| Fixed issues | N/A <!-- use #NUM format to reference an issue -->

#### Summary

Improve the CI to cover newer versions of MariaDB 11.4+.

Also cover the quay.io/mariadb-foundation/mariadb-devel images from
https://quay.io/repository/mariadb-foundation/mariadb-devel?tab=tags
where they are updated with completed, reviewed and tested changes in
the MariaDB server main branch.

These are completed features/bug fixes in the next server version. As
such this is a good thing to test before the users get the final
product.

ref:
https://mariadb.org/new-service-quay-io-mariadb-foundation-mariadb-devel/

The test failures here are fixed by #6425.

<!-- Provide a summary of your change. -->
@grooverdan grooverdan mentioned this pull request Jun 9, 2024
Merged
derrabus added a commit that referenced this pull request Jun 14, 2024
@derrabus
Merge branch '4.0.x' into 4.1.x
5caaee4 
* 4.0.x:
  CI: Update MariaDB versions (#6426)
  CI MariaDB: add 11.4, remove 11.0 (#6432)
  Display warnings when running PHPUnit in CI (#6431)
  Fix typo in the portability documentation (#6430)
  Fix MariaDB fetching of default table character-set (#6361) (#6425)
  Fix the portability documentation (#6429)
  Update tests/Platforms/AbstractPlatformTestCase.php
  Update tests/Platforms/AbstractPlatformTestCase.php
  add test
  Fix: Skip type comparison if disableTypeComments is true
derrabus added a commit to derrabus/dbal that referenced this pull request Jun 19, 2024
@derrabus
Merge branch '4.1.x' into 5.0.x
819ac6a 
* 4.1.x: (25 commits)
  Simplify signature of fetchTableOptionsByTable
  Add MariaDb1010Platform for fetchTableOptionsByTable
  PHPUnit 10.5.21 (doctrine#6447)
  Move schema split for SQLite CREATE INDEX only (doctrine#6352)
  PHPStan 1.11.5 (doctrine#6446)
  Default to distinct union queries (doctrine#6439)
  Revert "Merge pull request doctrine#6413 from achterin/bugfix/foreign_key_name_change_detection"
  Add `QueryBuilder` support for `UNION` clause (doctrine#6369)
  CI: Update MariaDB versions (doctrine#6426)
  CI MariaDB: add 11.4, remove 11.0 (doctrine#6432)
  Display warnings when running PHPUnit in CI (doctrine#6431)
  Fix typo in the portability documentation (doctrine#6430)
  Fix MariaDB fetching of default table character-set (doctrine#6361) (doctrine#6425)
  Fix the portability documentation (doctrine#6429)
  Update tests/Platforms/AbstractPlatformTestCase.php
  Update tests/Platforms/AbstractPlatformTestCase.php
  add test
  Fix: Skip type comparison if disableTypeComments is true
  Remove redundant variable (doctrine#6326)
  Fix test names to reflect their actual purpose
  ...
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@derrabus derrabus derrabus approved these changes

Assignees

No one assigned

Labels

Bug MariaDB Schema Introspection

Projects

None yet

Milestone

3.8.5

Development

Successfully merging this pull request may close these issues.

COLLATION_CHARACTER_SET_APPLICABILITY.COLLATION_NAME vs MariaDB FULL_COLLATION_NAME

2 participants

@grooverdan @derrabus