Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diff on column with collation problem #2538

Closed
MassiveHiggsField opened this issue Oct 16, 2016 · 9 comments
Closed

Diff on column with collation problem #2538

MassiveHiggsField opened this issue Oct 16, 2016 · 9 comments

Comments

@MassiveHiggsField
Copy link

MassiveHiggsField commented Oct 16, 2016

Copied from doctrine/migrations#495

Used versions:

  • os: Debian GNU/Linux 8
  • mysql: Ver 14.14 Distrib 5.7.8-rc, for Linux (x86_64) using EditLine wrapper
  • doctrine/orm v2.5.4
  • doctrine/migrations 1.4.1

Entity:

use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Table(name="Contest")
 * @ORM\Entity
 */
class Contest
{
    /**
     * @var string
     *
     * @ORM\Column(name="id", type="string", length=32, nullable=false, options={"collation":"utf8_unicode_ci"})
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    private $id;
}

Current table:

CREATE TABLE `Contest` (
  `id` int(11) NOT NULL AUTO_INCREMENT
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci 

Migration generated from diff:

class Version20161014011453 extends AbstractMigration
{
    /**
     * @param Schema $schema
     */
    public function up(Schema $schema)
    {
        // this up() migration is auto-generated, please modify it to your needs
        $this->abortIf($this->connection->getDatabasePlatform()->getName() != 'mysql', 'Migration can only be executed safely on \'mysql\'.');

        $this->addSql('ALTER TABLE Contest CHANGE id id VARCHAR(32) NOT NULL COLLATE utf8_unicode_ci');
    }

    /**
     * @param Schema $schema
     */
    public function down(Schema $schema)
    {
        // this down() migration is auto-generated, please modify it to your needs
        $this->abortIf($this->connection->getDatabasePlatform()->getName() != 'mysql', 'Migration can only be executed safely on \'mysql\'.');

        $this->addSql('ALTER TABLE Contest CHANGE id id VARCHAR(32) NOT NULL COLLATE utf8_unicode_ci');
    }
}

So up() and down() do the same thing and if i execute this migration and rerun diff, it will generate the same migration again.

Any help appreciated.

@Ocramius
Copy link
Member

@mikeSimonson can you check this?

@mikeSimonson
Copy link
Contributor

I will look at it

@mikeSimonson
Copy link
Contributor

The issue can either be the schema object that is generated from the database connection, the schema that is generated from the ORM metadata or the diff between those 2.

The SQL that I used to generate the table is slightly different from the bug report. It includes the Primary key directive because it's required by mysql5.7.

CREATE TABLE `Contest` (
  `id` int(11) NOT NULL PRIMARY KEY AUTO_INCREMENT
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci 

So far the differences that I can see between the 2 generated schema objects includes

x connection ORM
table.column.id._length null int(32)
table.column.id._precision int(10) int(0)
table.column.id._platformOptions [ ] ['version' => false]
table.column.id._comment '' null
table.column.id._customSchemaOptions [ ] ['collation' => 'utf8_unicode_ci']
table._indexes.primary._name 'PRIMARY' 'primary'
table._indexes._idGeneratorType bool(false) int(0)

The output of the diff is

ALTER TABLE Contest CHANGE id id VARCHAR(32) NOT NULL

I don't have the collation in the diff but it's the default on my setup.

Applying the diff leads to those schemas differences

x connection ORM
table.column.id._precision int(10) int(0)
table.column.id._platformOptions [ ] ['version' => false]
table.column.id._comment '' null
table.column.id._customSchemaOptions [ ] ['collation' => 'utf8_unicode_ci']
table._indexes.primary._name 'PRIMARY' 'primary'
table._indexes._idGeneratorType bool(false) int(0)

So the lenght difference has been fixed.

Any rerun of the diff will now continue to output the same alter table as before.

@mikeSimonson
Copy link
Contributor

@Ocramius I would say that this bug is in two parts.

The 2 differences that are detected are the collation and the auto_increment.

  • About the collation : The method getColumnCollationDeclarationSQL($collation) is not implemented in the Mysql Driver. The PR is coming.
  • About the auto_increment : The ORM should probably not allow the use of @Orm\GeneratedValue(strategy="IDENTITY") on a varchar.

What do you think @Ocramius ?

@Ocramius
Copy link
Member

Ocramius commented Nov 6, 2016

About the collation : The method getColumnCollationDeclarationSQL($collation) is not implemented in the Mysql Driver. The PR is coming.

Is that PR #2551?

The ORM should probably not allow the use of @ORM\GeneratedValue(strategy="IDENTITY") on a varchar.

Can't do this, as we don't really know what the DB-side type may be doing. It may be an auto-fill integer mapped as string, for example.

@mikeSimonson
Copy link
Contributor

mikeSimonson commented Nov 6, 2016

Is that PR #2551?

Yes but it's not right yet

Can't do this, as we don't really know what the DB-side type may be doing. It may be an auto-fill integer mapped as string, for example.

Does that mean that I should verify that all types in DBAL/Types look for the autoincrement field property ? The stringType for instance doesn't look for it.

@Ocramius
Copy link
Member

Ocramius commented Nov 7, 2016

No, it means that we can't assume whether a type supports auto-increment or not (also custom types)

@morozov
Copy link
Member

morozov commented Aug 25, 2021

Cannot reproduce on 3.2.x:

$connection->executeStatement('DROP TABLE IF EXISTS `Contest`');
$connection->executeStatement('CREATE TABLE `Contest` (
  `id` int(11) NOT NULL AUTO_INCREMENT PRIMARY KEY
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci');

$desiredTable = new Table('Contest', [
    new Column('id', new StringType(), [
        'length'  => 32,
        'notnull' => true,
        'platformOptions' => [
            'collate' => 'utf8_unicode_ci',
        ],
    ])
]);

$sm = $connection->createSchemaManager();
$actualTable = $sm->listTableDetails('Contest');

$comparator = new Comparator();
$diff = $comparator->diffTable($actualTable, $desiredTable);
$sm->alterTable($diff);
echo $connection->fetchNumeric('SHOW CREATE TABLE `Contest`')[1], PHP_EOL;

// CREATE TABLE `Contest` (
//   `id` varchar(32) COLLATE utf8_unicode_ci NOT NULL
// ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8_unicode_ci

$actualTable = $sm->listTableDetails('Contest');
$diff = $comparator->diffTable($actualTable, $desiredTable);
var_dump($diff);
// false

@morozov morozov closed this as completed Aug 25, 2021
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants