doctrine/dbal

A wrong charset migration is generated everytime with doctrine/dbal 3.3.1

Pixelshaped opened this issue · 25 comments

Pixelshaped commented

Bug Report

There is a regression caused by doctrine/dbal 3.3.1 in migrations generation.

Q A
Version 3.3.1

Summary

Using doctrine/migrations-bundle:3.2.1:

  • With doctrine/dbal 3.2.0 to 3.3.0 (I tested them all). The command doctrine:migrations:diff produces no migration if there is no change in the schema.
  • With doctrine/dbal 3.3.1 . The command doctrine:migrations:diff produces an empty up() migration and a wrong down() migration even if there is no change in the schema (and produces it everytime it's ran).

How to reproduce

Not sure about it. I'm using MariaDB 10.5.12, but another user had this issue and reportedly uses MySQL 5.7. At first I posted in #3391 thinking it was maybe older than that.

In Symfony, Doctrine is configured such as:

doctrine:
    dbal:
        url: '%database_url%'

        server_version: 'mariadb-10.5.12'
        charset: utf8mb4
        default_table_options:
            charset: utf8mb4
            collate: utf8mb4_unicode_ci

And the wrong down migration produced is full of charset changes:

public function down(Schema $schema): void
    {
        // this down() migration is auto-generated, please modify it to your needs
        $this->addSql('ALTER TABLE address CHANGE street street VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE city city VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE post_code post_code VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE country country ENUM(\'fr\') NOT NULL COLLATE `utf8mb4_unicode_ci` COMMENT \'(82a9e4d26595c87ab6e442391d8c5bba)(DC2Type:countryenum)\', CHANGE first_name first_name VARCHAR(255) DEFAULT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE last_name last_name VARCHAR(255) DEFAULT NULL COLLATE `utf8mb4_unicode_ci`');
        $this->addSql('ALTER TABLE api_key CHANGE token token VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE role role VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE type type VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`');
        $this->addSql('ALTER TABLE blog_article CHANGE title title VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE excerpt excerpt LONGTEXT NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE external_link external_link VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE slug slug VARCHAR(255) NOT NULL COLLATE `utf8mb4_unicode_ci`, CHANGE image_filename image_filename VARCHAR(255) DEFAULT NULL COLLATE `utf8mb4_unicode_ci`');
        ...
morozov commented

@Pixelshaped is your problem reproducible on 3.3.x with #5224 applied? If yes, please provide the steps to reproduce.

Pixelshaped commented

I required 3.3.x-dev and the problem still happens. I'll try to provide a minimal project to reproduce it asap (which might or might not be this month).

natewiebe13 commented

I'm seeing the same issue with MySQL 8 and using 3.3.x-dev. This issue does not happen for me with 3.3.0.

natewiebe13 commented

I created a minimal project here: https://github.com/natewiebe13/doctrine-dbal-bug

Since it might be dependent on which database/env setup, I've included a docker environment to hopefully help with replication.

  • ❤️1
morozov commented

It would expedite addressing the issue if the bug was reproduced only by using the DBAL. If it takes an entire project to reproduce the issue, it is possible that the issue isn't necessarily in the DBAL. Not all of the maintainers have enough familiarity with the rest of the project dependencies and the time to debug the entire project.

natewiebe13 commented

@morozov I think I've been able to narrow it down a bit: https://github.com/natewiebe13/doctrine-dbal-bug/blob/master/test.php

I tried to replicate the objects used with the Doctrine migration command. What looks to be the issue is a difference between 'collate' and 'collation' when doing the normalize here: https://github.com/doctrine/dbal/blob/3.3.x/src/Platforms/MySQL/Comparator.php#L53

I'm not familiar enough with what should be happening to make much of a call on what's correct or not.

Does this help narrow things down?

natewiebe13 commented

This actually looks related to: doctrine/DoctrineBundle#1468 and #5214

So I guess the question is, what should we use?

morozov commented

@natewiebe13 why does your $fromTable use "collation" while $toTable uses "collate"? This seems related to #5214 which boils down to the fact that the schema manager uses "collation" to represent the schema at the DBAL level (which seems correct and is at least consistent across the platforms):

dbal/src/Schema/MySQLSchemaManager.php

Lines 348 to 350 in dfdb5fb

if ($tableOptions['TABLE_COLLATION'] !== null) {
$table->addOption('collation', $tableOptions['TABLE_COLLATION']);
}

While the MySQL platform class expects "collate" (which matches the MySQL COLLATE keyword but seems to be a bug from the DBAL API standpoint):

dbal/src/Platforms/AbstractMySQLPlatform.php

Lines 456 to 457 in 0ed09e1

if (! isset($options['collate'])) {
$options['collate'] = $options['charset'] . '_unicode_ci';

Replacing "collate" with "collation" in your example solves the problem.

natewiebe13 commented

@natewiebe13 why does your $fromTable use "collation" while $toTable uses "collate"?

This was in efforts to replicate what's happening, ended up noticing that dependency, then thought it was worth pointing it out and wait for recommended next steps. Looking at the related issues, it seems like you came to the same conclusion where the option changed and the previous one needs to be deprecated.

Looks like there's a PR in place, so it looks like things are moving in the right direction.

ioniks commented

Hi,

I have this issue on 3.3.2 too.

  • 👍3
natewiebe13 commented

@ioniks at this point, you're likely running into this one: doctrine/DoctrineBundle#1468 (comment), hopefully this helps.

fdiedler commented

@Pixelshaped @morozov @natewiebe13 I always have the same problem with doctrine/dbal 3.3.2
Wrong charset migrations are generated at each call to php bin/console doctrine:migrations:diff

  • 👍5
Pixelshaped commented

I can confirm this is still happening in my codebase with

doctrine:
    dbal:
        url: '%database_url%'

        server_version: 'mariadb-10.5.4'
        charset: utf8mb4
        default_table_options:
            charset: utf8mb4
            collate: utf8mb4_unicode_ci

Locking dbal to 3.2.2 is a temporary workaround.

anatolykhelmer commented

migrations v3.4.1 (dbal 3.3.3) - issue still here!

fdiedler commented

@Pixelshaped @anatolykhelmer Yes, issue still there :(
Maybe open a new issue ? because this one is closed and so will be forgotton...

Seldaek commented

If you are still seeing the issue (like I was), also check the charset matches the collation. I have collation set to utf8mb4_unicode_ci by default, and some DB fields were set to ascii_general_ci.

Hinted by https://github.com/doctrine/dbal/pull/5224/files I added this to AbstractPlatform::columnsEqual:

if ($column1Array['name'] === 'hash') {
    var_dump($this->getColumnDeclarationSQL('', $column1Array), $this->getColumnDeclarationSQL('', $column2Array));
}

Then ran the schema update command and saw:

string(65) " CHAR(32) CHARACTER SET ascii NOT NULL COLLATE `ascii_general_ci`"
string(45) " CHAR(32) NOT NULL COLLATE `ascii_general_ci`"

Which I could fix by specifying the charset as well on the property's annotation:

     * @ORM\Column(type="string", length=32, nullable=false, options={"fixed": true, "collation":"ascii_general_ci", "charset": "ascii"})

Simply specifying the collation there was not enough to fix it.

Hopefully this helps someone else.

  • 👍1
Pixelshaped commented

If you are still seeing the issue (like I was), also check the charset matches the collation. I have collation set to utf8mb4_unicode_ci by default, and some DB fields were set to ascii_general_ci.

Hinted by https://github.com/doctrine/dbal/pull/5224/files I added this to AbstractPlatform::columnsEqual:

if ($column1Array['name'] === 'hash') {
    var_dump($this->getColumnDeclarationSQL('', $column1Array), $this->getColumnDeclarationSQL('', $column2Array));
}

Then ran the schema update command and saw:

string(65) " CHAR(32) CHARACTER SET ascii NOT NULL COLLATE `ascii_general_ci`"
string(45) " CHAR(32) NOT NULL COLLATE `ascii_general_ci`"

Which I could fix by specifying the charset as well on the property's annotation:

     * @ORM\Column(type="string", length=32, nullable=false, options={"fixed": true, "collation":"ascii_general_ci", "charset": "ascii"})

Simply specifying the collation there was not enough to fix it.

Hopefully this helps someone else.

Thanks for reporting a solution, unfortunately all my columns are standard utf8mb4_unicode_ci and this bug still happens. I'll create another issue if not exists.

  • 👍1
Pixelshaped commented

Quick update: I tested the branch 3.3.x-dev and the problem is gone for me. I don't know which commit has fixed it.

Seldaek commented

@Pixelshaped could be fixed by #5326

  • 👍3
greg0ire commented

@morozov let's do a release?

  • 👍1
  • ❤️1
greg0ire commented

OK, I'm on it.

  • 🎉3
fdiedler commented

@Pixelshaped

Thanks for reporting a solution, unfortunately all my columns are standard utf8mb4_unicode_ci and this bug still happens. I'll create another issue if not exists.

Same problem for me, all my colums are in utf8mb4_unicode_ci
But good new if the bug is gone on the 3.3.x-dev branch but maybe it worths opening a new issue ?

@greg0ire @Seldaek
Hope a fix will be released soon :)

Pixelshaped commented

@Pixelshaped

Thanks for reporting a solution, unfortunately all my columns are standard utf8mb4_unicode_ci and this bug still happens. I'll create another issue if not exists.

Same problem for me, all my colums are in utf8mb4_unicode_ci
But good new if the bug is gone on the 3.3.x-dev branch but maybe it worths opening a new issue ?

@greg0ire @Seldaek
Hope a fix will be released soon :)

The bug is gone for me in 3.3.4. @greg0ire pushed a new release yesterday. Check it before opening a new issue.

  • 👍2
  • 🎉1
fdiedler commented

@Pixelshaped Yes, gone for me too !

github-actions commented

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.