Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 0 additions & 35 deletions src/Platforms/PostgreSQLPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -265,10 +260,6 @@ public function getAlterTableSQL(TableDiff $diff): array
continue;
}

if ($this->isUnchangedBinaryColumn($columnDiff)) {
continue;
}

$fromColumn = $columnDiff->fromColumn;

$oldColumnName = $fromColumn->getQuotedName($this);
Expand Down Expand Up @@ -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}
*/
Expand Down
95 changes: 95 additions & 0 deletions tests/Functional/Schema/PostgreSQL/ComparatorTest.php
Original file line number Diff line number Diff line change
@@ -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,
));
}
}