Skip to content

Commit

Permalink
Remove code compensating for a schema comparison flaw
Browse files Browse the repository at this point in the history
This code was relevant before the improvement in
#4746 was implemented. The new
comparator API is enforced in
#4771.

Now, the comparator will not produce a false-positive diff which is
enforced by the new test.
  • Loading branch information
morozov committed Aug 26, 2022
1 parent c94c3d3 commit c20d091
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 12 deletions.
12 changes: 0 additions & 12 deletions src/Platforms/OraclePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Doctrine\DBAL\Schema\Sequence;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\TransactionIsolationLevel;
use Doctrine\DBAL\Types\BinaryType;
use InvalidArgumentException;

use function array_merge;
Expand Down Expand Up @@ -573,17 +572,6 @@ public function getAlterTableSQL(TableDiff $diff): array

$column = $columnDiff->column;

// Do not generate column alteration clause if type is binary and only fixed property has changed.
// Oracle only supports binary type columns with variable length.
// Avoids unnecessary table alteration statements.
if (
$column->getType() instanceof BinaryType &&
$columnDiff->hasChanged('fixed') &&
count($columnDiff->changedProperties) === 1
) {
continue;
}

$columnHasChangedComment = $columnDiff->hasChanged('comment');

/**
Expand Down
63 changes: 63 additions & 0 deletions tests/Functional/Schema/Oracle/ComparatorTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests\Functional\Schema\Oracle;

use Doctrine\DBAL\Platforms\OraclePlatform;
use Doctrine\DBAL\Schema\AbstractSchemaManager;
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\Types;

final class ComparatorTest extends FunctionalTestCase
{
private AbstractSchemaManager $schemaManager;

private Comparator $comparator;

protected function setUp(): void
{
if (! $this->connection->getDatabasePlatform() instanceof OraclePlatform) {
self::markTestSkipped('This test covers Oracle-specific schema comparison scenarios.');
}

$this->schemaManager = $this->connection->createSchemaManager();
$this->comparator = $this->schemaManager->createComparator();
}

/**
* Oracle does not support fixed length binary columns. The DBAL will map the {@see Types::BINARY} type
* to the variable-length RAW column type regardless of the "fixed" attribute.
*
* There should not be a diff when comparing a variable-length and a fixed-length column
* that are otherwise the same.
*
* @see OraclePlatform::getBinaryTypeDeclarationSQLSnippet()
*/
public function testChangeBinaryColumnFixed(): void
{
$table = new Table('comparator_test');
$column = $table->addColumn('id', Types::BINARY, [
'length' => 32,
'fixed' => true,
]);
$this->dropAndCreateTable($table);

$column->setFixed(false);

self::assertNull(ComparatorTestUtils::diffFromActualToDesiredTable(
$this->schemaManager,
$this->comparator,
$table
));

self::assertNull(ComparatorTestUtils::diffFromDesiredToActualTable(
$this->schemaManager,
$this->comparator,
$table
));
}
}

0 comments on commit c20d091

Please sign in to comment.