Change MySQL bool declaration from TINYINT(1) to TINYINT

[rixafy]

Original PR: #6569

Since 8.0 MySQL throws whis warning "1681 [HY000] Integer display width is deprecated and will be removed in a future release." when integer column with display width is created
This change is backwards compatible and can be merged into all active branches (does not trigger column type update migration)

Q	A
Type	bugfix
Fixed issues	#6743

Summary

There is a problem when I want custom TinyInt type like this - since the new comparator where there are no comment do distinguish custom types.

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;
	}
}

The hack that I proposed in #6743 (comment) no longer works (no idea why, it would require a lot of debugging).

But this change will solve that problem, after the change my migration diff dropped from 20 SQLs back to 0 SQLs, all SQL wanted to update existing bool columns, like ALTER TABLE ban CHANGE unban unban TINYINT(1) NOT NULL, even though in database it's like TINYINT(1).

This is not a BC break, because when diffing, it seems that DBAL does not compare the specified length, it has no effect on column size, it serves just as metadata for that column. Even when I changed TINYINT(1) to TINYINT(3) in database, there were no new diffs.

One thing that's changed is that new migrations will produce TINYINT and not TINYINT(1) and in MySQL/MariaDB it defaults to 3, which has no effect on column size.

[rixafy]

It also fixes partly doctrine/migrations#1435, at least the TINYINT part.

[derrabus]

This feels like it should've been more difficult, tbh. 🙈

One thing though. If we create a schema with a boolean column with the current codebase, we'd create it as TINYINT(1). If we introspect such a table after your change, how would the diff look like? Can we cover this with a test?

[rixafy]

One thing though. If we create a schema with a boolean column with the current codebase, we'd create it as TINYINT(1). If we introspect such a table after your change, how would the diff look like?

@derrabus there would be no diff, it seems like display width in numeric values isn't compared. It's same as if I changed int(11) to int(12) in some integer, no new diffs will be produced, I tried it with INT and TINYINT.

How should I test it? Should I add function testGeneratesTypeDeclarationForBoolean like this one for integers? https://github.com/doctrine/dbal/blob/4.3.x/tests/Platforms/AbstractMySQLPlatformTestCase.php#L107

It would then look like

public function testGeneratesTypeDeclarationForBoolean(): void
{
	self::assertEquals(
		'TINYINT',
		$this->platform->getBooleanTypeDeclarationSQL([])
	);
}

So I don't know if it's very useful, but why not, and if so, should I add it also for other platforms?

[rixafy]

@derrabus what about test like this? It would be in Functional/Platform/ColumnDisplayWidthTest. I have no experience with other platforms, so it's only for MariaDB/MySQL.

It's testing that column display width does not produce a diff (as all default widths are > 1).

class ColumnDisplayWidthTest extends FunctionalTestCase
{
    public function testColumnDisplayWidthDoesNotMatter(): void
    {
        if (
            ! $this->connection->getDatabasePlatform() instanceof MySQLPlatform &&
            ! $this->connection->getDatabasePlatform() instanceof MariaDBPlatform
        ) {
            self::markTestSkipped('This test covers MySQL/MariaDB-specific schema comparison scenarios.');
        }

        $table = Table::editor()
            ->setUnquotedName('column_display_width_test')
            ->setColumns(
                Column::editor()
                    ->setUnquotedName('c1')
                    ->setTypeName(Types::BOOLEAN)
                    ->create(),
                Column::editor()
                    ->setUnquotedName('c2')
                    ->setTypeName(Types::SMALLINT)
                    ->create(),
                Column::editor()
                    ->setUnquotedName('c3')
                    ->setTypeName(Types::INTEGER)
                    ->create(),
                Column::editor()
                    ->setUnquotedName('c4')
                    ->setTypeName(Types::BIGINT)
                    ->create(),
            )
            ->create();

        $this->connection->executeQuery('DROP TABLE IF EXISTS column_display_width_test');
        $this->connection->executeQuery('CREATE TABLE column_display_width_test(
            c1 TINYINT(1) NOT NULL, 
            c2 SMALLINT(1) NOT NULL, 
            c3 INT(1) NOT NULL, 
            c4 BIGINT(1) NOT NULL)',
        );

        $sm   = $this->connection->createSchemaManager();
        $diff = $sm->createComparator()
            ->compareTables($sm->introspectTableByUnquotedName('column_display_width_test'), $table);

        self::assertTrue($diff->isEmpty());
    }
}