Skip to content

Commit

Permalink
Remove handling orphaned foreign keys
Browse files Browse the repository at this point in the history
  • Loading branch information
morozov committed Oct 15, 2022
1 parent 73fa2c8 commit 0c08553
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 144 deletions.
7 changes: 4 additions & 3 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ awareness about deprecated code.

# Upgrade to 4.0

## Removed `SchemaDiff::$orphanedForeignKeys`

The functionality of automatically dropping the foreign keys referencing the tables being dropped has been removed.

## BC Break: Removed registration of user defined functions for SQLite

DBAL does not register functions for SQLite anymore. The following functions
Expand Down Expand Up @@ -571,9 +575,6 @@ Reference from `ForeignKeyConstraint` to its local (referencing) `Table` is remo
- `getLocalTable()`,
- `getLocalTableName()`.

The type of `SchemaDiff::$orphanedForeignKeys` has changed from list of foreign keys (`list<ForeignKeyConstraint>`)
to map of referencing tables to their list of foreign keys (`array<string,list<ForeignKeyConstraint>>`).

## BC BREAK: Removed redundant `AbstractPlatform` methods.

The following redundant `AbstractPlatform` methods have been removed:
Expand Down
55 changes: 3 additions & 52 deletions src/Schema/Comparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ public function compareSchemas(Schema $fromSchema, Schema $toSchema): SchemaDiff
$alteredSequences = [];
$droppedSequences = [];

$orphanedForeignKeys = [];

/** @var array<string,list<array{ForeignKeyConstraint,string}>> $foreignKeysToTable */
$foreignKeysToTable = [];

/** @var array<string,array<string,null>> $localTablesByForeignTable */
$localTablesByForeignTable = [];

foreach ($toSchema->getNamespaces() as $namespace) {
if ($fromSchema->hasNamespace($namespace)) {
continue;
Expand Down Expand Up @@ -81,48 +73,11 @@ public function compareSchemas(Schema $fromSchema, Schema $toSchema): SchemaDiff
$tableName = $table->getShortestName($fromSchema->getName());

$table = $fromSchema->getTable($tableName);
if (! $toSchema->hasTable($tableName)) {
$droppedTables[$tableName] = $table;
}

// also remember all foreign keys that point to a specific table
foreach ($table->getForeignKeys() as $foreignKey) {
$foreignTable = strtolower($foreignKey->getForeignTableName());

$foreignKeysToTable[$foreignTable][] = [$foreignKey, $table->getName()];
$localTablesByForeignTable[$foreignTable][$tableName] = null;
}
}

foreach ($droppedTables as $tableName => $table) {
if (! isset($foreignKeysToTable[$tableName])) {
if ($toSchema->hasTable($tableName)) {
continue;
}

foreach ($foreignKeysToTable[$tableName] as [$foreignKey, $localTableName]) {
if (isset($droppedTables[strtolower($localTableName)])) {
continue;
}

$orphanedForeignKeys[$tableName][] = $foreignKey;
}

// deleting duplicated foreign keys present on both on the orphanedForeignKey
// and the removedForeignKeys from changedTables
foreach ($localTablesByForeignTable[$tableName] as $localTableName => $_) {
if (! isset($alteredTables[$localTableName])) {
continue;
}

foreach ($alteredTables[$localTableName]->getDroppedForeignKeys() as $droppedForeignKey) {
// We check if the key is from the removed table if not we skip.
if ($tableName !== strtolower($droppedForeignKey->getForeignTableName())) {
continue;
}

$alteredTables[$localTableName]->unsetDroppedForeignKey($droppedForeignKey);
}
}
$droppedTables[$tableName] = $table;
}

foreach ($toSchema->getSequences() as $sequence) {
Expand Down Expand Up @@ -152,7 +107,7 @@ public function compareSchemas(Schema $fromSchema, Schema $toSchema): SchemaDiff
$droppedSequences[] = $sequence;
}

$diff = new SchemaDiff(
return new SchemaDiff(
$createdTables,
$alteredTables,
$droppedTables,
Expand All @@ -162,10 +117,6 @@ public function compareSchemas(Schema $fromSchema, Schema $toSchema): SchemaDiff
$alteredSequences,
$droppedSequences,
);

$diff->orphanedForeignKeys = $orphanedForeignKeys;

return $diff;
}

private function isAutoIncrementSequenceInSchema(Schema $schema, Sequence $sequence): bool
Expand Down
18 changes: 0 additions & 18 deletions src/Schema/SchemaDiff.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,6 @@ class SchemaDiff
*/
public array $removedSequences = [];

/**
* @deprecated
*
* @var array<string,list<ForeignKeyConstraint>>
*/
public array $orphanedForeignKeys = [];

/**
* Constructs an SchemaDiff object.
*
Expand Down Expand Up @@ -175,17 +168,6 @@ protected function _toSql(AbstractPlatform $platform, bool $saveMode = false): a
}
}

if ($saveMode === false) {
foreach ($this->orphanedForeignKeys as $localTableName => $tableOrphanedForeignKey) {
foreach ($tableOrphanedForeignKey as $orphanedForeignKey) {
$sql[] = $platform->getDropForeignKeySQL(
$orphanedForeignKey->getQuotedName($platform),
$localTableName,
);
}
}
}

if ($platform->supportsSequences()) {
foreach ($this->getAlteredSequences() as $sequence) {
$sql[] = $platform->getAlterSequenceSQL($sequence);
Expand Down
13 changes: 1 addition & 12 deletions src/Schema/TableDiff.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function __construct(
private readonly array $renamedIndexes,
private readonly array $addedForeignKeys,
private readonly array $modifiedForeignKeys,
private array $droppedForeignKeys,
private readonly array $droppedForeignKeys,
) {
}

Expand Down Expand Up @@ -142,15 +142,4 @@ public function getDroppedForeignKeys(): array
{
return $this->droppedForeignKeys;
}

/** @internal This method exists only for compatibility with the current implementation of the schema comparator. */
public function unsetDroppedForeignKey(ForeignKeyConstraint $foreignKey): void
{
$this->droppedForeignKeys = array_filter(
$this->droppedForeignKeys,
static function (ForeignKeyConstraint $droppedForeignKey) use ($foreignKey): bool {
return $droppedForeignKey !== $foreignKey;
},
);
}
}
59 changes: 0 additions & 59 deletions tests/Schema/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -619,44 +619,6 @@ public function testAutoIncrementNoSequences(): void
self::assertCount(0, $diff->newSequences);
}

/**
* You can get multiple drops for a FK when a table referenced by a foreign
* key is deleted, as this FK is referenced twice, once on the orphanedForeignKeys
* array because of the dropped table, and once on changedTables array. We
* now check that the key is present once.
*/
public function testAvoidMultipleDropForeignKey(): void
{
$oldSchema = new Schema();

$tableA = $oldSchema->createTable('table_a');
$tableA->addColumn('id', 'integer');

$tableB = $oldSchema->createTable('table_b');
$tableB->addColumn('id', 'integer');

$tableC = $oldSchema->createTable('table_c');
$tableC->addColumn('id', 'integer');
$tableC->addColumn('table_a_id', 'integer');
$tableC->addColumn('table_b_id', 'integer');

$tableC->addForeignKeyConstraint($tableA->getName(), ['table_a_id'], ['id']);
$tableC->addForeignKeyConstraint($tableB->getName(), ['table_b_id'], ['id']);

$newSchema = new Schema();

$tableB = $newSchema->createTable('table_b');
$tableB->addColumn('id', 'integer');

$tableC = $newSchema->createTable('table_c');
$tableC->addColumn('id', 'integer');

$schemaDiff = $this->comparator->compareSchemas($oldSchema, $newSchema);

self::assertCount(1, $schemaDiff->changedTables['table_c']->getDroppedForeignKeys());
self::assertCount(1, $schemaDiff->orphanedForeignKeys);
}

public function assertSchemaTableChangeCount(
SchemaDiff $diff,
int $newTableCount = 0,
Expand Down Expand Up @@ -790,8 +752,6 @@ public function testForeignKeyRemovalWithRenamedLocalColumn(): void
$schemaDiff = $this->comparator->compareSchemas($fromSchema, $toSchema);

self::assertArrayHasKey('table2', $schemaDiff->changedTables);
self::assertCount(1, $schemaDiff->orphanedForeignKeys);
self::assertEquals('fk_table2_table1', $schemaDiff->orphanedForeignKeys['table1'][0]->getName());

$tableDiff = $schemaDiff->changedTables['table2'];

Expand Down Expand Up @@ -837,25 +797,6 @@ public function testWillNotProduceSchemaDiffOnTableWithAddedCustomSchemaDefiniti
);
}

public function testNoOrphanedForeignKeyIfReferencingTableIsDropped(): void
{
$schema1 = new Schema();

$parent = $schema1->createTable('parent');
$parent->addColumn('id', 'integer');

$child = $schema1->createTable('child');
$child->addColumn('id', 'integer');
$child->addColumn('parent_id', 'integer');
$child->addForeignKeyConstraint('parent', ['parent_id'], ['id']);

$schema2 = new Schema();

$diff = $this->comparator->compareSchemas($schema1, $schema2);

self::assertEmpty($diff->orphanedForeignKeys);
}

/**
* @param array<AbstractAsset> $assets
*
Expand Down

0 comments on commit 0c08553

Please sign in to comment.