Comparator cannot differentiate between Boolean and custom Tinyint type

[rixafy]

Bug Report

Q	A
Version	4.2.2
Previous Version if the bug is a regression	3.x
Database	MariaDB 10.11

Summary

Hi, now when comments were removed, Comparator cannot differentiate my custom Tinyint type from Boolean.

class TinyIntType extends Type
{
	public const string NAME = 'tinyint';
	
	public function getName(): string
	{
		return self::NAME;
	}
	
	public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
	{
		return 'TINYINT'.(!empty($column['unsigned']) ? ' UNSIGNED' : '');
	}

	public function convertToPHPValue($value, AbstractPlatform $platform): ?int
	{
		return $value === null ? null : (int) $value;
	}

	public function getBindingType(): ParameterType
	{
		return ParameterType::INTEGER;
	}
}

Current behavior

When generating a diff, or running schema update command, this condition https://github.com/doctrine/dbal/blob/4.2.2/src/Platforms/AbstractPlatform.php#L2259 fails because it is comparing TINYINT(1) NOT NULL with TINYINT UNSIGNED NOT NULL, even though database value is TINYINT UNSIGNED and it has default (3) size, not 1.

Type declaration is read from type of "old" column https://github.com/doctrine/dbal/blob/4.2.2/src/Platforms/AbstractPlatform.php#L1424 which was detected as Doctrine\DBAL\Types\BooleanType, probably because old columns for comparing have data only from database, and since comments like (DC2Type:tinyint) are no longer supported, it guesses that tynyint means boolean.

Expected behavior

I would expect that we could have multiple types defined to same database types, what I think it's possible if the strings were the same.. but AbstractMySQLPlatform does not respects any column arguments, it just returns TINYINT(1) https://github.com/doctrine/dbal/blob/4.2.2/src/Platforms/AbstractMySQLPlatform.php#L195

How to reproduce

I'm not able to provide whole code, because I don't know exactly how is old schema created, and probably this is intended behavior, I'm just curious how can I avoid this diff mismatch, but if it's something not intended, I'll try to reproduce minimal example when I find more free time, the problem will be constructing of existing database schema from live MariaDB data.

[rixafy]

Now when i think about it, it can be resolved just by adding unsigned support to AbstractMySQLPlatform::getBooleanTypeDeclarationSQL, I tried it and the problem went away, and no other problems with bool types occurred, so it may be a solution, if Doctrine\DBAL\Types\BooleanType was expected type as an old column, because data were only read from database which has no knowledge of types declared in ORM entities.

This is what I have changed, also the (1) was removed.

public function getBooleanTypeDeclarationSQL(array $column): string
{
    #return 'TINYINT(1)';
    return 'TINYINT' . $this->_getCommonIntegerTypeDeclarationSQL($column);
}

If you think this is the right solution, I can send PR.

[derrabus]

Making the boolean type aware of UNSIGNED is a terrible idea, sorry.

Shouldn't your problem go away if you register a custom type mapping for TINYINT?

[rixafy]

Making the boolean type aware of UNSIGNED is a terrible idea, sorry.

I know, it's an ugly fix, you don't need to be sorry.

Shouldn't your problem go away if you register a custom type mapping for TINYINT?

Thanks, that was what occurred to me yesterday when I was going to sleep.. so I tried it now and it's working, but I have to type it like TINYINT(1) in my getSQLDeclaration(), I dumped array $column but there is no indication of column width (length is null and scale is 0 in both columns), if there's not (1) then the diff will print out all the boolean types because boolean has definition of TINYINT(1).

Another ugly solution I came up with yesterday was checking if array key columnDefinition exists, which is unset before diff, which works well, but I would like to figure out how to do it properly, I just don't know where can I get the width of the column.

public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
{
    if (!array_key_exists('columnDefinition', $column)) {
        return 'TINYINT(1)';
    }
		
    return "TINYINT" . (!empty($column['unsigned']) ? ' UNSIGNED' : '');
}

[derrabus]

Shouldn't your problem go away if you register a custom type mapping for TINYINT?

Thanks, that was what occurred to me yesterday when I was going to sleep.. so I tried it now and it's working, but I have to type it like TINYINT(1) in my getSQLDeclaration(), I dumped array $column but there is no indication of column width (length is null and scale is 0 in both columns), if there's not (1) then the diff will print out all the boolean types because boolean has definition of TINYINT(1).

I don't get this part, sorry. Shouldn't you be able to support custom widths in your custom TINYINT type?

That being said, we could think about changing the declaration of our booleans on MySQL to not configure a width. This was actually proposed in #6569, but we haven't heard back from the author.

[rixafy]

I don't get this part, sorry. Shouldn't you be able to support custom widths in your custom TINYINT type?

Yes, I want the default (3), but if I do so, I'll get diff of all the boolean columns because they still have the BooleanType which is TINYINT(1) and that does not equal the new detected column (my own tinyint) which I would declare to be TINYINT(3).

That being said, we could think about changing the declaration of our booleans on MySQL to not configure a width. This was actually proposed in https://github.com/doctrine/dbal/pull/6569, but we haven't heard back from the author.

Yes, this would solve the problem, it's a bit useless to define column width for integers. I tried to remove it from AbstractMySQLPlatform and the problem goes away, since I'm not declaring column width in my TinyIntType as well.

[rixafy]

The change would be also backwards compatible, and I think that author intended to change MariaDB as well.

I think that in all MySQL/MariaDB versions it's not required to define column width, I may be wrong but I would be surprised.

There will also be nothing in the diff, because array $column has no info about the column width and it's not compared in Doctrine, and all other integers in abstract platform have no column width as well. The only drawback would be that columns created to this date would be in MySQL/MariaDB defined like TINYINT(1) and new columns would be TINYINT(3), because 3 is the default, and I don't think that affects how doctrine is working with numbers, since it's only mysql metadata information what length of column to display for some systems that read those metadata.

[cybd]

public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
{
    if (!array_key_exists('columnDefinition', $column)) {
        return 'TINYINT(1)';
    }
		
    return "TINYINT" . (!empty($column['unsigned']) ? ' UNSIGNED' : '');
}

@Rixafy Trying to "fool" columnsEqual() in AbstractPlatform.php. Nice try and it works. Thanks.