Migration does not detect custom SQLDeclaration change

[michnovka]

Bug Report

If declaring custom Type and its getSQLDeclaration() method changes, no migration is generated.

Q	A
Version	3.3.2

Summary

Implement ENUM like in the official docs Then if you add/remove new ENUM element, the return value of getSQLDeclaration() changes. But this is not detected by Comparer and no migration is generated

How to reproduce

Create type:

<?php
namespace MyProject\DBAL;

use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Platforms\AbstractPlatform;

class EnumVisibilityType extends Type
{
    const ENUM_VISIBILITY = 'enumvisibility';
    const STATUS_VISIBLE = 'visible';
    const STATUS_INVISIBLE = 'invisible';

    public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
    {
        return "ENUM('visible', 'invisible')";
    }

    public function convertToPHPValue($value, AbstractPlatform $platform)
    {
        return $value;
    }

    public function convertToDatabaseValue($value, AbstractPlatform $platform)
    {
        if (!in_array($value, array(self::STATUS_VISIBLE, self::STATUS_INVISIBLE))) {
            throw new \InvalidArgumentException("Invalid status");
        }
        return $value;
    }

    public function getName()
    {
        return self::ENUM_VISIBILITY;
    }

    public function requiresSQLCommentHint(AbstractPlatform $platform)
    {
        return true;
    }
}

2. Create Entity with column of this type

<?php
/** @Entity */
class Article
{
    /** @Column(type="enumvisibility") */
    private $status;
}

3. Migrate and save DB schema change.
4. Change getSQLDeclaration to

        return "ENUM('visible', 'invisible','halfvisible')";

5. Do migrations:diff

Expected behaviour

New migration is created reflecting the change

[michnovka]

The issue is that AbstractPlatform::colmnsEqual returns updated $this->getColumnDeclarationSQL('', $column1Array) also for $this->getColumnDeclarationSQL('', $column2Array). Any idea where we can check for the actual current column definition in DB?

[michnovka]

I dug real deep into the code and I came to conclusion that this is unfixable until we implement ENUM as valid DB type in Doctrine. The issue is that in the end DBAL\Schema\MySQLSchemeManager::_getPortableTableColumnDefinition checks the column type and determines type-specific properties (length for text, precision for doubles etc.) This all is then passed to a Column object in $options array.
The proper way would be to add $options['enum_options'] and parse ENUM options into it. Then we can have column diff to actually check for change in these and properly report ENUM type as changed.

[morozov]

I don't believe schema migrations could ever work correctly when the column definition is specified manually. See #3647 (comment). May be related to #5223.

[morozov]

After digging a bit more through the codebase, I believe the types defined as the documentation suggests will never work properly with the current design.

On the one hand, a type can declare an arbitrary DDL to represent itself. E.g.:

dbal/tests/Functional/Schema/MySQL/PointType.php

Lines 15 to 18 in c826c18

	public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
	{
	return 'POINT';
	}

Besides that, a type can use arbitrary undocumented properties that influence the resulting DDL. E.g. (#5307):

dbal/src/Platforms/AbstractMySQLPlatform.php

Lines 154 to 161 in 9b155f1

	public function getDateTimeTypeDeclarationSQL(array $column): string
	{
	if (isset($column['version']) && $column['version'] === true) {
	return 'TIMESTAMP';
	}
	return 'DATETIME';
	}

This way, the API for building type DDL is quite flexible, composable, and appealing to use.

On the other hand, the logic of type introspection is absolutely rigid and hard-coded:

dbal/src/Schema/MySQLSchemaManager.php

Lines 113 to 226 in 9b155f1

	protected function _getPortableTableColumnDefinition(array $tableColumn): Column
	{
	$tableColumn = array_change_key_case($tableColumn, CASE_LOWER);
	$dbType = strtolower($tableColumn['type']);
	$dbType = strtok($dbType, '(), ');
	assert(is_string($dbType));
	$length = $tableColumn['length'] ?? strtok('(), ');
	$fixed = false;
	if (! isset($tableColumn['name'])) {
	$tableColumn['name'] = '';
	}
	$scale = 0;
	$precision = null;
	$type = $this->_platform->getDoctrineTypeMapping($dbType);
	switch ($dbType) {
	case 'char':
	case 'binary':
	$fixed = true;
	break;
	case 'float':
	case 'double':
	case 'real':
	case 'numeric':
	case 'decimal':
	if (
	preg_match(
	'([A-Za-z]+\(([0-9]+),([0-9]+)\))',
	$tableColumn['type'],
	$match
	) === 1
	) {
	$precision = (int) $match[1];
	$scale = (int) $match[2];
	$length = null;
	}
	break;
	case 'tinytext':
	$length = AbstractMySQLPlatform::LENGTH_LIMIT_TINYTEXT;
	break;
	case 'text':
	$length = AbstractMySQLPlatform::LENGTH_LIMIT_TEXT;
	break;
	case 'mediumtext':
	$length = AbstractMySQLPlatform::LENGTH_LIMIT_MEDIUMTEXT;
	break;
	case 'tinyblob':
	$length = AbstractMySQLPlatform::LENGTH_LIMIT_TINYBLOB;
	break;
	case 'blob':
	$length = AbstractMySQLPlatform::LENGTH_LIMIT_BLOB;
	break;
	case 'mediumblob':
	$length = AbstractMySQLPlatform::LENGTH_LIMIT_MEDIUMBLOB;
	break;
	case 'tinyint':
	case 'smallint':
	case 'mediumint':
	case 'int':
	case 'integer':
	case 'bigint':
	case 'year':
	$length = null;
	break;
	}
	if ($this->_platform instanceof MariaDBPlatform) {
	$columnDefault = $this->getMariaDBColumnDefault($this->_platform, $tableColumn['default']);
	} else {
	$columnDefault = $tableColumn['default'];
	}
	$options = [
	'length' => $length !== null ? (int) $length : null,
	'unsigned' => str_contains($tableColumn['type'], 'unsigned'),
	'fixed' => $fixed,
	'default' => $columnDefault,
	'notnull' => $tableColumn['null'] !== 'YES',
	'scale' => $scale,
	'precision' => $precision,
	'autoincrement' => str_contains($tableColumn['extra'], 'auto_increment'),
	];
	if (isset($tableColumn['comment'])) {
	$options['comment'] = $tableColumn['comment'];
	}
	$column = new Column($tableColumn['field'], Type::getType($type), $options);
	if (isset($tableColumn['characterset'])) {
	$column->setPlatformOption('charset', $tableColumn['characterset']);
	}
	if (isset($tableColumn['collation'])) {
	$column->setPlatformOption('collation', $tableColumn['collation']);
	}
	return $column;
	}

Not only that it's non-extensible, but it's also absolutely incoherent with its DDL counterpart which makes it prone to the issues like this.