Migration generates unnecessary queries

[Rixafy]

Bug Report

Q	A
BC Break	yes
Version	3.3.1 - dev-master, in 3.3.0 there is only 1 (different) sql

Summary

After upgrading from DBAL 2.x and to latest 3.x version, I have ran a diff command and a weird migration was generated. Same output is generated in o:s:u --dump-sql

ALTER TABLE product_badge CHANGE slug slug VARCHAR(15) NOT NULL COLLATE `utf8_bin`
ALTER TABLE parameter CHANGE slug slug VARCHAR(15) NOT NULL COLLATE `utf8_bin`
ALTER TABLE parameter_value CHANGE slug slug VARCHAR(15) NOT NULL COLLATE `utf8_bin`
ALTER TABLE product CHANGE slug slug VARCHAR(15) NOT NULL COLLATE `utf8_bin`
ALTER TABLE product_brand CHANGE slug slug VARCHAR(15) NOT NULL COLLATE `utf8_bin`
ALTER TABLE customer CHANGE slug slug VARCHAR(15) NOT NULL COLLATE `utf8_bin`

In my code there is

#[ORM\Column(length: 15, unique: true, options: ['collation' => 'utf8_bin'])]
private string $slug;

and database structure match the parameters, even when I execute the statements, next migration will be the same.

I'm using PHP 8.1, MariaDB 10.3.34

In version 3.3.0 it works just fine, but it will generate me only this query, because I guess there was some bug with enums, because name is a enum, it disappears when I use type SystemMetaName|string, so I locked my project to that version for now. It is no longer in 3.3.1+, but there are that queries with utf8_bin collation.

ALTER TABLE system_meta CHANGE name name VARCHAR(255) NOT NULL

#[ORM\Column(unique: true)]
private SystemMetaName $name;

There is a diff 3.3.0...3.3.1 but I can't see the probable cause of this bug.

My dbal settings are:

doctrine.dbal:
	connection:
		charset: utf8mb4
		default_table_options:
			charset: utf8mb4
			collate: utf8mb4_unicode_ci
		driver: pdo_mysql
		types:
			uuid_binary:
				class: Ramsey\Uuid\Doctrine\UuidBinaryType
				commented: false

Settings are from nettrine implementation in Nette Framework https://contributte.org/packages/nettrine/dbal.html.

[morozov]

@Rixafy please provide the steps to reproduce the issue using only the DBAL APIs.

[Rixafy]

@morozov, I have found the root cause.

3.3.0...3.3.1#diff-3a87a8ea798d5188662020a7cfc091da8595a6ccfff91b074deec8ac7801f349L60 (see highlighted line in files)

Before, there was comparator created with new self();, without passing $this->platform, so when there is a platform, https://github.com/doctrine/dbal/blob/3.3.x/src/Schema/Comparator.php#L293 this condition passes and $this->columnEqual is called.

I checked the columns passed to that method and dumped both of them. First column is from database, second is from schema
https://gist.github.com/Rixafy/08642e3429ec11c5581b28f81b5a7af1

The difference is that column from the DB has charset to utf8 and column from schema doesn't have the charset.

So I dumped a table schema - https://gist.github.com/Rixafy/35da79ee11f17c8840c2775e8804f0c6

In create table SQL there is

`slug` varchar(15) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL

It has utf8 charset (other columns don't), but when I execute generated diff

ALTER TABLE product_badge CHANGE slug slug VARCHAR(15) NOT NULL COLLATE `utf8_bin`

It still has utf8 charset.

So I found how to fix it, when I add charset to column definition that match that default in database, it will not generate any SQL.
The problem is that my default database and doctrine charset is utf8mb4, but that column has utf8_bin collation and only possible charset is utf8, and that doesn't match by default doctrine charset so it will generate a SQL, but in that SQL only collation is changed.

When I run

ALTER TABLE product_badge CHANGE slug slug VARCHAR(15) CHARACTER SET utf8mb4 NOT NULL COLLATE `utf8mb4_bin`

and I edit entity schema to collation utf8mb4_bin, no SQL is generated.

When I run

ALTER TABLE product_badge CHANGE slug slug VARCHAR(15) CHARACTER SET utf8 NOT NULL COLLATE `utf8_bin`

I need to add charset => utf8 to my entity attributes. So I guess it's better that way and it's problem that older versions didn't make any SQL diff, but anyway, generated SQL diff won't fix a problem, since it doesn't change character set, and it should, because that's the difference when comparing to my default doctrine config, not collation.

[morozov]

I have found the root cause.

Could you please provide the steps to reproduce the problem first?

[Dallas62]

Hi,

I don't know exactly how reproduce this issue, but this is an annoying issue.

The migration process always try to alter columns without any reasons.

In my case, the issue occurs with datetime, when I define:

  /**
   * @var \Datetime $created_at
   */
  #[Column(name: 'created_at', type: 'datetime', precision: 3)]
  private $created_at;

  /**
   * @var \Datetime $updated_at
   */
  #[Column(name: 'updated_at', type: 'datetime', precision: 3)]
  private $updated_at;

With the follow table:

The migration always generate:

        $this->addSql('ALTER TABLE table_name CHANGE created_at created_at DATETIME(3) NOT NULL, CHANGE updated_at updated_at DATETIME(3) NOT NULL');

Apply it doesn't change anything.

Any idea ?

EDIT: Columns are already NOT NULL, but not displayed on the screenshot

[Dallas62]

HI @morozov
Any update on this one ?
Does the @Rixafy helps you to debug ?
The issue is still there on 3.3.7 generating 22 migrations while 1 is required.

[Rixafy]

Hi @Dallas62, I explained how I get rid of the problem, I didn't have charset to utf8 in my column, but I did have collation utf8_bin and my default doctrine collation was utf8mb4_bin, but anyway, it generated the wrong SQL, since it wanted to change collation to utf8 (and it was already utf8) but no charset, and diff was in charset, and I didn't have time to debug it deeper since it's really an edge case.

Later I had similar issue when I used php enum as property, and I had php property nullable and also nullable: true in attribute and it always generated SQL that wanted to remove nullability from enum, but after a week or so, it was probably fixed, because this additional SQL was no longer in my migrations.

What SQLs do you have as additional?

[Dallas62]

Hi @Rixafy,

I wrote it on my previous comment.
In my case, it's generating an Alter table on Datetime to "add" precision which is already applied on the table + applying the patch still produce the same migration queries.

[Dallas62]

It was okay on the version 3.3.0.

[Rixafy]

Ah, I see, that may be the same issue, there is a diff if you want to search deeper 3.3.0...3.3.1, but I think I figured it out what is a cause.

Before, there was comparator created with new self();, without passing $this->platform, so when there is a platform, https://github.com/doctrine/dbal/blob/3.3.x/src/Schema/Comparator.php#L293 this condition passes and $this->columnEqual is called.

in 3.3.0, some code in comparator was not aware of platform type, with that change I think they fixed another bugs, but it seems like it caused some more.

[morozov]

Any update on this one ?

No. It would be nice if somebody built a minimal reproducer script (not an application).

[ludekbenedik]

@morozov Hi, there is/was a issue which describes the same problem with discussion where someone describes and explains the problem with a low level reproduction. Someone else has written that the bug will be fixed in the 3.3.8 version. Today I update doctrine/dbal to 3.4.0 and the problem still remains.
Unfortunately i cannot find the issue again.

[flack]

just chiming in to say I've hit the same issue. If I instantiate Comparator without passing $platform, the diff comes back empty, otherwise I get a ColumDiff for every column in every DB table looking like this:

For now, I've added this workaround in my clientside code:

foreach ($changed_table->changedColumns as $name => $col_diff) {
     if (!$comparator->diffColumn($col_diff->column, $col_diff->fromColumn)) {
          unset($changed_table->changedColumns[$name]);
     }
}

I'd love to work on a minimal reproducer script, but I'm not sure I know how to bootstrap Doctrine without writing at least 2000 lines of code :-) Maybe I'll try to bastardize one of the unit tests when I have time..

[flack]

@morozov it's quite old, but you think something like this could be a viable approach for creating a reproducer script? doctrine/orm#6487

[morozov]

It could be a code snippet like #5582 (comment) or a failing test. Anything that doesn't require external dependencies and could be run as code.

[flack]

@morozov ok, I've cobbled something together with which I can reproduce locally:

$table = new Table('"tester"');
$table->addColumn('"id"', Types::INTEGER);
$table->addColumn('"name"', Types::STRING);

class testclass {}

$cm = new ClassMetadata(testclass::class);
$cm->setTableName('tester');
$cm->setIdentifier(['id']);
$cm->mapField([
    'fieldName' => 'id',
    'type' => Types::INTEGER
]);
$cm->mapField([
    'fieldName' => 'name',
    'type' => Types::STRING
]);


$schemaManager = $conn->createSchemaManager();
$schemaManager->dropAndCreateTable($table);

$current_schema = $schemaManager->createSchema();
$to_schema = (new SchemaTool($em))->getSchemaFromMetadata([$cm]);

$schema_diff = (new Comparator())->compareSchemas($current_schema, $to_schema);
echo "Without \$platform:\n";
dump($schema_diff->changedTables['tester']->changedColumns);

$schema_diff = (new Comparator($conn->getDatabasePlatform()))->compareSchemas($current_schema, $to_schema);
echo "With \$platform:\n";
dump($schema_diff->changedTables['tester']->changedColumns);

This shows the following output (against MySQL 8.0.30, if that is important):

[morozov]

@flack thank you for the example. Unfortunately, the ClassMetadata and the SchemaTool are not part of the DBAL, they are part of the ORM. Could you try excluding them? I'm not that well familiar with the ORM to do this myself.

[flack]

@morozov try this:

$table = new Table('"tester"');
$table->addColumn('"id"', Types::INTEGER);
$table->addColumn('"name"', Types::STRING);

$schemaManager = $conn->createSchemaManager();
$schemaManager->dropAndCreateTable($table);

$current_schema = $schemaManager->createSchema();

$to_schema = new Schema();
$t2 = $to_schema->createTable('tester');
$t2->addColumn('"id"', Types::INTEGER);
$t2->addColumn('"name"', Types::STRING);


$schema_diff = (new Comparator())->compareSchemas($current_schema, $to_schema);
echo "Without \$platform:\n";
dump($schema_diff->changedTables['tester']->changedColumns ?? 'no diff detected');

$schema_diff = (new Comparator($conn->getDatabasePlatform()))->compareSchemas($current_schema, $to_schema);
echo "With \$platform:\n";
dump($schema_diff->changedTables['tester']->changedColumns);

Output:

[morozov]

Thanks. It's reproducible on 3.4.x and 4.0.x.

[morozov]

This code example looks not entirely correct since it uses an internal DBAL API in the second case:

dbal/src/Schema/Comparator.php

Lines 32 to 35 in c60fd26

	/**
	* @internal The comparator can be only instantiated by a schema manager.
	*/
	public function __construct(?AbstractPlatform $platform = null)

This prevents the DBAL from being able to use MySQL-specific schema comparison logic. This bug likely was fixed in #5471 (3.3.8).

The diff in the second case is not reproducible if the comparator is instantiated properly:

$comparator = $schemaManager->createComparator();

See the upgrade notes:

dbal/UPGRADE.md

Lines 538 to 539 in ca2160d

	1. Instantiation of the `Comparator` class outside the DBAL is deprecated. Use `SchemaManager::createComparator()`
	to create the comparator specific to the current database connection and the database platform.

Please try instantiating the comparator as recommended and see if the issue is still reproducible.

[flack]

@morozov thx, that fixed the issue for me. But I suppose for @Rixafy's issue some analoguous change would have to be made in some Nette component

[Rixafy]

Hi, that nette extension just integrates dbal into the nette framework - https://github.com/nettrine/dbal/blob/master/src/DI/DbalExtension.php, I don't know what can affect it from there, there is no comparator registration.

I tried it with 3.4.0 dbal and the issue still remains:

#[ORM\Column(length: 15, unique: true, options: ['collation' => 'utf8_bin'])]
private string $slug;

my default doctrine charset is utf8mb4 and collation utf8mb4_unicode_ci, and when I have this as a column definition in entity, orm schema update will generate this SQL:

ALTER TABLE product CHANGE slug slug VARCHAR(15) NOT NULL COLLATE `utf8_bin`;

which is odd, since slug in database has already collation utf8_bin, I would expect that it would want to change charset instead, but I don't know how my default utf8mb4 charset would work with utf8_bin (in DB, there is utf8_bin collation and utf8 charset in that column).

When I change definition and add a charset to it

#[ORM\Column(length: 15, unique: true, options: ['collation' => 'utf8_bin', 'charset' => 'utf8'])]
private string $slug;

then the problem disappears and no SQL is generated, I understand this may be the edge case, since I switched collations from utf8 to utb8mb4 in database and code, and I left some binary columns in utf8, so if no one is experiencing the problem, this issue may be closed.

[morozov]

@Rixafy as you may see from the above comments, the problem is not necessarily about the model definitions but may be caused by the code that performs the comparison. Without the code reproducing the issue, there isn't much we can do on the DBAL side to address your issue.

[flack]

btw, I guess the dbal docs also needs to be updated, e.g. here it still says new Comparator in the code sample:

dbal/docs/en/reference/schema-representation.rst

Line 50 in f576466

$comparator = new \Doctrine\DBAL\Schema\Comparator();

[morozov]

@Rixafy there must be code in your application that instantiates the comparator: it's either your code or one of the dependencies.

To track what instantiates the comparator, you can do either of the following:

1. Set a breakpoint in the comparator constructor and when it gets hit, observe the stack trace.
2. Modify the code of the comparator constructor and throw an exception from there. Once the exception is thrown, observe the stack trace.

[morozov]

Closing due to the lack of feedback.

[github-actions]

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.