Do not use ColumnDiff::$changedProperties in PostgreSQLPlatform::getAlterTableSQL()

Author: morozov

src/Platforms/PostgreSQLPlatform.php

@@ -7,23 +7,18 @@
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Platforms\Keywords\KeywordList;
 use Doctrine\DBAL\Platforms\Keywords\PostgreSQLKeywords;
-use Doctrine\DBAL\Schema\ColumnDiff;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Doctrine\DBAL\Schema\Identifier;
 use Doctrine\DBAL\Schema\Index;
 use Doctrine\DBAL\Schema\PostgreSQLSchemaManager;
 use Doctrine\DBAL\Schema\Sequence;
 use Doctrine\DBAL\Schema\TableDiff;
 use Doctrine\DBAL\TransactionIsolationLevel;
-use Doctrine\DBAL\Types\BinaryType;
-use Doctrine\DBAL\Types\BlobType;
 use UnexpectedValueException;
 
-use function array_diff;
 use function array_merge;
 use function array_unique;
 use function array_values;
-use function count;
 use function explode;
 use function implode;
 use function in_array;
@@ -265,10 +260,6 @@ public function getAlterTableSQL(TableDiff $diff): array
                 continue;
             }
 
-            if ($this->isUnchangedBinaryColumn($columnDiff)) {
-                continue;
-            }
-
             $fromColumn = $columnDiff->fromColumn;
 
             $oldColumnName = $fromColumn->getQuotedName($this);
@@ -372,32 +363,6 @@ public function getAlterTableSQL(TableDiff $diff): array
         return array_merge($sql, $tableSql, $columnSql);
     }
 
-    /**
-     * Checks whether a given column diff is a logically unchanged binary type column.
-     *
-     * Used to determine whether a column alteration for a binary type column can be skipped.
-     * Doctrine's {@see BinaryType} and {@see BlobType} are mapped to the same database column type on this platform
-     * as this platform does not have a native VARBINARY/BINARY column type. Therefore the comparator
-     * might detect differences for binary type columns which do not have to be propagated
-     * to database as there actually is no difference at database level.
-     */
-    private function isUnchangedBinaryColumn(ColumnDiff $columnDiff): bool
-    {
-        $columnType = $columnDiff->column->getType();
-
-        if (! $columnType instanceof BinaryType && ! $columnType instanceof BlobType) {
-            return false;
-        }
-
-        $fromColumnType = $columnDiff->fromColumn->getType();
-
-        if (! $fromColumnType instanceof BinaryType && ! $fromColumnType instanceof BlobType) {
-            return false;
-        }
-
-        return count(array_diff($columnDiff->changedProperties, ['type', 'length', 'fixed'])) === 0;
-    }
-
     /**
      * {@inheritdoc}
      */

tests/Functional/Schema/PostgreSQL/ComparatorTest.php

@@ -0,0 +1,95 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Tests\Functional\Schema\PostgreSQL;
+
+use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
+use Doctrine\DBAL\Schema\AbstractSchemaManager;
+use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\Comparator;
+use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Tests\Functional\Schema\ComparatorTestUtils;
+use Doctrine\DBAL\Tests\FunctionalTestCase;
+use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\Types\Types;
+
+final class ComparatorTest extends FunctionalTestCase
+{
+    private AbstractSchemaManager $schemaManager;
+
+    private Comparator $comparator;
+
+    protected function setUp(): void
+    {
+        if (! $this->connection->getDatabasePlatform() instanceof PostgreSQLPlatform) {
+            self::markTestSkipped('This test covers PostgreSQL-specific schema comparison scenarios.');
+        }
+
+        $this->schemaManager = $this->connection->createSchemaManager();
+        $this->comparator    = $this->schemaManager->createComparator();
+    }
+
+    /**
+     * The PostgreSQL platform maps both BLOB and BINARY columns to the BYTEA column type.
+     *
+     * @see PostgreSQLPlatform::getBlobTypeDeclarationSQL()
+     */
+    public function testCompareBinaryAndBlob(): void
+    {
+        $this->testColumnModification(static function (Table $table, string $name): Column {
+            return $table->addColumn($name, Types::BINARY);
+        }, static function (Column $column): void {
+            $column->setType(Type::getType(Types::BLOB));
+        });
+    }
+
+    /**
+     * The PostgreSQL platform maps both BINARY and VARBINARY columns to the BYTEA column type.
+     *
+     * @see PostgreSQLPlatform::getVarbinaryTypeDeclarationSQLSnippet()
+     */
+    public function testCompareBinaryAndVarbinary(): void
+    {
+        $this->testColumnModification(static function (Table $table, string $name): Column {
+            return $table->addColumn($name, Types::BINARY);
+        }, static function (Column $column): void {
+            $column->setFixed(true);
+        });
+    }
+
+    /**
+     * The PostgreSQL platform disregards the "length" attribute of BINARY and VARBINARY columns.
+     *
+     * @see PostgreSQLPlatform::getBinaryTypeDeclarationSQLSnippet()
+     */
+    public function testCompareBinariesOfDifferentLength(): void
+    {
+        $this->testColumnModification(static function (Table $table, string $name): Column {
+            return $table->addColumn($name, Types::BINARY, ['length' => 16]);
+        }, static function (Column $column): void {
+            $column->setLength(32);
+        });
+    }
+
+    private function testColumnModification(callable $addColumn, callable $modifyColumn): void
+    {
+        $table  = new Table('comparator_test');
+        $column = $addColumn($table, 'id');
+        $this->dropAndCreateTable($table);
+
+        $modifyColumn($column);
+
+        self::assertNull(ComparatorTestUtils::diffFromActualToDesiredTable(
+            $this->schemaManager,
+            $this->comparator,
+            $table,
+        ));
+
+        self::assertNull(ComparatorTestUtils::diffFromDesiredToActualTable(
+            $this->schemaManager,
+            $this->comparator,
+            $table,
+        ));
+    }
+}