Validation schema failure after disabling DC2Type generation in Doctrine

[berkut1]

Hello,

I have some issues. According to this doctrine/DoctrineBundle#1714, it is possible to disable DC2Type generation. However, after enabling it, it is no longer possible to pass doctrine:schema:validate for the database. And it doesn't even work for date*_immutable types.

Here is an example of my code.
Initial migration:

$this->addSql('CREATE TABLE tbl (
			id                    UUID NOT NULL,
			recorded_at           TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL DEFAULT NOW(),
			PRIMARY KEY (id))');
//$this->addSql('COMMENT ON COLUMN tbl.id IS \'(DC2Type:tbl_id)\'');
//$this->addSql('COMMENT ON COLUMN tbl.recorded_at IS \'(DC2Type:datetime_immutable)\'');

Configuration:

doctrine:
    dbal:
        disable_type_comments: true

Entity:

#[ORM\Entity]
class AuditLog
{
    #[ORM\Column(name: 'id', type: 'tbl_id', nullable: false)]
    #[ORM\Id]
    #[ORM\GeneratedValue(strategy: 'NONE')]
    private Id $id;

    #[ORM\Column(name: 'recorded_at', type: 'datetime_immutable', nullable: false, options: ['default' => 'now()'])]
    private \DateTimeImmutable $date;
	
}

When I run doctrine:schema:validate -v, I get the following output expecting DC2Type.

Mapping
-------

                                                                                                                        
 [OK] The mapping files are correct.                                                                                    
                                                                                                                        

Database
--------

                                                                                                                        
 [ERROR] The database schema is not in sync with the current mapping file.                                              
                                                                                                                        

 // 2 schema diff(s) detected:                                                                                          

     ALTER TABLE tbl ALTER id TYPE UUID;
     ALTER TABLE tbl ALTER recorded_at TYPE TIMESTAMP(0) WITHOUT TIME ZONE;
     

Am I doing something wrong, or am I misunderstanding something, or is this a bug?

Thanks.

doctrine/cache                      2.2.0   PHP Doctrine Cache library is a popular cache implementation that supports many different drivers such as redis, memcache, apc, mongodb and others.
doctrine/collections                2.2.2   PHP Doctrine Collections library that adds additional functionality on top of PHP arrays.
doctrine/common                     3.4.4   PHP Doctrine Common project is a library that provides additional functionality that other Doctrine projects depend on such as better reflection support, proxies and much more.
doctrine/data-fixtures              1.7.0   Data Fixtures for all Doctrine Object Managers
doctrine/dbal                       3.8.4   Powerful PHP database abstraction layer (DBAL) with many features for database schema introspection and management.
doctrine/deprecations               1.1.3   A small layer on top of trigger_error(E_USER_DEPRECATED) or PSR-3 logging with options to disable all deprecations or selectively for packages.
doctrine/doctrine-bundle            2.12.0  Symfony DoctrineBundle
doctrine/doctrine-fixtures-bundle   3.6.1   Symfony DoctrineFixturesBundle
doctrine/doctrine-migrations-bundle 3.3.1   Symfony DoctrineMigrationsBundle
doctrine/event-manager              2.0.1   The Doctrine Event Manager is a simple PHP event system that was built to be used with the various Doctrine projects.
doctrine/inflector                  2.0.10  PHP Doctrine Inflector is a small library that can perform string manipulations with regard to upper/lowercase and singular/plural forms of words.
doctrine/instantiator               2.0.0   A small, lightweight utility to instantiate objects in PHP without invoking their constructors
doctrine/lexer                      3.0.1   PHP Doctrine Lexer parser library that can be used in Top-Down, Recursive Descent Parsers.
doctrine/migrations                 3.7.4   PHP Doctrine Migrations project offer additional functionality on top of the database abstraction layer (DBAL) for versioning your database schema and easily deploying changes to it...  
doctrine/orm                        2.19.5  Object-Relational-Mapper for PHP
doctrine/persistence                3.3.2   The Doctrine Persistence project is a set of shared interfaces and functionality that the different Doctrine object mappers share.
doctrine/sql-formatter              1.4.0   a PHP SQL highlighting library

[greg0ire]

the following output expecting DC2Type

What do you mean by that?

[berkut1]

I mean that if I add these lines back:

//$this->addSql('COMMENT ON COLUMN tbl.id IS \'(DC2Type:tbl_id)\'');
//$this->addSql('COMMENT ON COLUMN tbl.recorded_at IS \'(DC2Type:datetime_immutable)\'');

the error will disappear. However, then what is the point of disable_type_comments: true, other than it just disables comments for data types, but at the same time causes issues with doctrine:schema:validate?

As I understand it, this option is supposed to simplify the transition to ORM 3, where these comments are not used, but it does not work as I expect.

[greg0ire]

Ok I get it now. I think you've found a bug, there should be no error indeed. There must be some place in the code where disable_type_comments is not taken into account.

In case you want to debug, the comparison happens here: https://github.com/doctrine/orm/blob/3a7d7c9f574517a19239523026e603b9dfed33bc/src/Tools/SchemaTool.php#L980-L997

Can you check whether you go through this line: https://github.com/doctrine/orm/blob/3a7d7c9f574517a19239523026e603b9dfed33bc/src/Tools/SchemaTool.php#L984 ?

[berkut1]

I tried debugging and found the following:

Here

dbal/src/Schema/Comparator.php

Line 324 in 080aab5

$fromTableColumns = $fromTable->getColumns();

we have data from the database that doesn't have types because comments are missing. We compare this with $toTableColumns, which has types, so there will always be differences when comparing.

Here the table properties are compared:

dbal/src/Schema/Comparator.php

Lines 622 to 623 in 080aab5

	$properties1 = $column1->toArray();
	$properties2 = $column2->toArray();

As seen in the screenshot, the property types are different and $changedProperties[] = 'type' will always be triggered.

UPD:
I tried to find where it might be fixed with minimal intervention, but I couldn't find it.
I don't quite understand the purpose of this filter; could the problem be with it?
https://github.com/doctrine/orm/blob/59c8bc09abf1f878694346986dc0755a6aed9fd2/src/Tools/SchemaTool.php#L1018-L1022

However, if the purpose of the variable $fromSchema = $this->createSchemaForComparison($toSchema); is to store the schema in the same way it is represented in the database (without non-standard types), then there are no problems with filter.

[greg0ire]

The goal of the filter is to remove tables not managed by Doctrine, or the migrations table which is a special case.

[greg0ire]

I notice that diffColumns is deprecated in favor of columnsEqual, which should work as you expect. You should figure out where diffColumns is called.

[berkut1]

diffColumns is called from here:

dbal/src/Schema/Comparator.php

Line 348 in 080aab5

$changedProperties = $this->diffColumn($column, $toColumn);

[greg0ire]

That's diffColumn, I was asking for diffColumns.

EDIT: my bad, I should have asked for diffColumn 😅

[greg0ire]

As you can see here

dbal/src/Schema/Comparator.php

Lines 350 to 356 in 080aab5

	if ($this->platform !== null) {
	if ($this->columnsEqual($column, $toColumn)) {
	continue;
	}
	} elseif (count($changedProperties) === 0) {
	continue;
	}

, if platform is not null, then it does not matter if $changedProperties is not empty.

[berkut1]

Yeah, but if ($this->columnsEqual($column, $toColumn)) always will be false, because $column and $toColumn are different.

[greg0ire]

2 different columns will be considered equal if they lead to the same column declaration SQL: that's what the platform-aware comparison is all about. Therefore, dumping both column object is not enough to prove that they are different.

[berkut1]

Yeah it pass correctly column declaration SQL here:

dbal/src/Platforms/AbstractPlatform.php

Lines 4676 to 4678 in 080aab5

	if (
	$this->getColumnDeclarationSQL('', $column1Array)
	!== $this->getColumnDeclarationSQL('', $column2Array)

But this line ruins everything:

dbal/src/Platforms/AbstractPlatform.php

Line 4696 in 080aab5

return $column1->getType() === $column2->getType();

I suppose there should be:

if($this->disableTypeComments){
            return true;
}        
return $column1->getType() === $column2->getType();

UPD:
This check seems to solve the problem.

[greg0ire]

I've had the exact same conversation 3 weeks ago in fact: #6257 (comment) 😅

[berkut1]

Oh... I see, and the solution is exactly the same. I feel strange right now. :D

[greg0ire]

Maybe you're going to be the one contributing the fix ? 🙏

[berkut1]

Alright, I'll give it a try. It doesn't seem too difficult.

[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.