Skip to content

Commit 693ace8

Browse files
MasterOdinMasterOdin
authored
Rework sqlite fk pragma handling (#2365)
1 parent 83f83ec commit 693ace8

File tree

6 files changed

+151
-69
lines changed

6 files changed

+151
-69
lines changed

‎src/Phinx/Db/Adapter/AbstractAdapter.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,4 +410,19 @@ protected function hasCreatedTable(string $tableName): bool
410410

411411
return in_array($tableName, $this->createdTables, true);
412412
}
413+
414+
/**
415+
* {@inheritDoc}
416+
*/
417+
public function preExecuteActions(array $updateSequences): array
418+
{
419+
return [];
420+
}
421+
422+
/**
423+
* {@inheritDoc}
424+
*/
425+
public function postExecuteActions(array $tableNames, array $preOptions): void
426+
{
427+
}
413428
}

‎src/Phinx/Db/Adapter/AdapterInterface.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,14 @@ public function rollbackTransaction(): void;
275275
*/
276276
public function execute(string $sql, array $params = []): int;
277277

278+
/**
279+
* Function to be called before executing any migration actions.
280+
*
281+
* @param \Phinx\Db\Plan\AlterTable[][] $updateSequences List of update sequences to be executed
282+
* @return array
283+
*/
284+
public function preExecuteActions(array $updateSequences): array;
285+
278286
/**
279287
* Executes a list of migration actions for the given table
280288
*
@@ -284,6 +292,15 @@ public function execute(string $sql, array $params = []): int;
284292
*/
285293
public function executeActions(Table $table, array $actions): void;
286294

295+
/**
296+
* Function to be called after executing any migration actions.
297+
*
298+
* @param array $tableNames List of table names that were affected by the actions
299+
* @param array $preOptions Options that were set before executing the actions
300+
* @return void
301+
*/
302+
public function postExecuteActions(array $tableNames, array $preOptions): void;
303+
287304
/**
288305
* Returns a new Query object
289306
*

‎src/Phinx/Db/Adapter/AdapterWrapper.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,14 @@ public function getConnection(): PDO
474474
return $this->getAdapter()->getConnection();
475475
}
476476

477+
/**
478+
* {@inheritDoc}
479+
*/
480+
public function preExecuteActions(array $updateSequences): array
481+
{
482+
return $this->getAdapter()->preExecuteActions($updateSequences);
483+
}
484+
477485
/**
478486
* @inheritDoc
479487
*/
@@ -482,6 +490,14 @@ public function executeActions(Table $table, array $actions): void
482490
$this->getAdapter()->executeActions($table, $actions);
483491
}
484492

493+
/**
494+
* {@inheritDoc}
495+
*/
496+
public function postExecuteActions(array $tableNames, array $preOptions): void
497+
{
498+
$this->getAdapter()->postExecuteActions($tableNames, $preOptions);
499+
}
500+
485501
/**
486502
* @inheritDoc
487503
*/

‎src/Phinx/Db/Adapter/SQLiteAdapter.php‎

Lines changed: 79 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use InvalidArgumentException;
1515
use PDO;
1616
use PDOException;
17+
use Phinx\Db\Action\AddForeignKey;
1718
use Phinx\Db\Table\Column;
1819
use Phinx\Db\Table\ForeignKey;
1920
use Phinx\Db\Table\Index;
@@ -1034,57 +1035,52 @@ protected function recreateIndicesAndTriggers(AlterInstructions $instructions):
10341035
* the given table, and of those tables whose constraints are
10351036
* targeting it.
10361037
*
1037-
* @param \Phinx\Db\Util\AlterInstructions $instructions The instructions to process
1038-
* @param string $tableName The name of the table for which to check constraints.
1039-
* @return \Phinx\Db\Util\AlterInstructions
1038+
* @param string|array<string> $tableNames The name of the table for which to check constraints.
1039+
* @return void
10401040
*/
1041-
protected function validateForeignKeys(AlterInstructions $instructions, string $tableName): AlterInstructions
1041+
protected function validateForeignKeys(string|array $tableNames): void
10421042
{
1043-
$instructions->addPostStep(function ($state) use ($tableName) {
1044-
$tablesToCheck = [
1045-
$tableName,
1046-
];
1047-
1048-
$otherTables = $this
1049-
->query(
1050-
"SELECT name FROM sqlite_master WHERE type = 'table' AND name != ?",
1051-
[$tableName],
1052-
)
1053-
->fetchAll();
1054-
1055-
foreach ($otherTables as $otherTable) {
1056-
$foreignKeyList = $this->getTableInfo($otherTable['name'], 'foreign_key_list');
1057-
foreach ($foreignKeyList as $foreignKey) {
1058-
if (strcasecmp($foreignKey['table'], $tableName) === 0) {
1059-
$tablesToCheck[] = $otherTable['name'];
1060-
break;
1061-
}
1062-
}
1063-
}
1064-
1065-
$tablesToCheck = array_unique(array_map('strtolower', $tablesToCheck));
1043+
if (!is_array($tableNames)) {
1044+
$tableNames = [$tableNames];
1045+
}
10661046

1067-
foreach ($tablesToCheck as $tableToCheck) {
1068-
$schema = $this->getSchemaName($tableToCheck, true)['schema'];
1047+
$tablesToCheck = $tableNames;
10691048

1070-
$stmt = $this->query(
1071-
sprintf('PRAGMA %sforeign_key_check(%s)', $schema, $this->quoteTableName($tableToCheck)),
1072-
);
1073-
$row = $stmt->fetch();
1074-
$stmt->closeCursor();
1049+
$otherTables = $this
1050+
->query(
1051+
"SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT IN (" . implode(',', array_fill(0, count($tableNames), '?')) . ')',
1052+
$tableNames,
1053+
)
1054+
->fetchAll();
10751055

1076-
if (is_array($row)) {
1077-
throw new RuntimeException(sprintf(
1078-
'Integrity constraint violation: FOREIGN KEY constraint on `%s` failed.',
1079-
$tableToCheck,
1080-
));
1056+
foreach ($otherTables as $otherTable) {
1057+
$foreignKeyList = $this->getTableInfo($otherTable['name'], 'foreign_key_list');
1058+
foreach ($foreignKeyList as $foreignKey) {
1059+
if (in_array(strtolower($foreignKey['table']), $tableNames)) {
1060+
$tablesToCheck[] = $otherTable['name'];
1061+
break;
10811062
}
10821063
}
1064+
}
10831065

1084-
return $state;
1085-
});
1066+
$tablesToCheck = array_unique(array_map('strtolower', $tablesToCheck));
10861067

1087-
return $instructions;
1068+
foreach ($tablesToCheck as $tableToCheck) {
1069+
$schema = $this->getSchemaName($tableToCheck, true)['schema'];
1070+
1071+
$stmt = $this->query(
1072+
sprintf('PRAGMA %sforeign_key_check(%s)', $schema, $this->quoteTableName($tableToCheck)),
1073+
);
1074+
$row = $stmt->fetch();
1075+
$stmt->closeCursor();
1076+
1077+
if (is_array($row)) {
1078+
throw new RuntimeException(sprintf(
1079+
'Integrity constraint violation: FOREIGN KEY constraint on `%s` failed.',
1080+
$tableToCheck,
1081+
));
1082+
}
1083+
}
10881084
}
10891085

10901086
/**
@@ -1230,16 +1226,13 @@ protected function beginAlterByCopyTable(string $tableName): AlterInstructions
12301226
* @param ?string $renamedOrRemovedColumnName The name of the renamed or removed column when part of a column
12311227
* rename/drop operation.
12321228
* @param ?string $newColumnName The new column name when part of a column rename operation.
1233-
* @param bool $validateForeignKeys Whether to validate foreign keys after the copy and drop operations. Note that
1234-
* enabling this option only has an effect when the `foreign_keys` PRAGMA is set to `ON`!
12351229
* @return \Phinx\Db\Util\AlterInstructions
12361230
*/
12371231
protected function endAlterByCopyTable(
12381232
AlterInstructions $instructions,
12391233
string $tableName,
12401234
?string $renamedOrRemovedColumnName = null,
12411235
?string $newColumnName = null,
1242-
bool $validateForeignKeys = true,
12431236
): AlterInstructions {
12441237
$instructions = $this->bufferIndicesAndTriggers($instructions, $tableName);
12451238

@@ -1251,26 +1244,9 @@ protected function endAlterByCopyTable(
12511244
}
12521245
}
12531246

1254-
$foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'];
1255-
1256-
if ($foreignKeysEnabled) {
1257-
$instructions->addPostStep('PRAGMA foreign_keys = OFF');
1258-
}
1259-
12601247
$instructions = $this->copyAndDropTmpTable($instructions, $tableName);
12611248
$instructions = $this->recreateIndicesAndTriggers($instructions);
12621249

1263-
if ($foreignKeysEnabled) {
1264-
$instructions->addPostStep('PRAGMA foreign_keys = ON');
1265-
}
1266-
1267-
if (
1268-
$foreignKeysEnabled &&
1269-
$validateForeignKeys
1270-
) {
1271-
$instructions = $this->validateForeignKeys($instructions, $tableName);
1272-
}
1273-
12741250
return $instructions;
12751251
}
12761252

@@ -1661,7 +1637,7 @@ protected function getDropPrimaryKeyInstructions(Table $table, string $column):
16611637
return $newState + $state;
16621638
});
16631639

1664-
return $this->endAlterByCopyTable($instructions, $tableName, null, null, false);
1640+
return $this->endAlterByCopyTable($instructions, $tableName, null, null);
16651641
}
16661642

16671643
/**
@@ -1673,7 +1649,6 @@ protected function getAddForeignKeyInstructions(Table $table, ForeignKey $foreig
16731649

16741650
$tableName = $table->getName();
16751651
$instructions->addPostStep(function ($state) use ($foreignKey, $tableName) {
1676-
$this->execute('pragma foreign_keys = ON');
16771652
$sql = substr($state['createSQL'], 0, -1) . ',' . $this->getForeignKeySqlDefinition($foreignKey) . '); ';
16781653

16791654
//Delete indexes from original table and recreate them in temporary table
@@ -2013,4 +1988,44 @@ public function getDecoratedConnection(): Connection
20131988

20141989
return $this->decoratedConnection = $this->buildConnection(SqliteDriver::class, $options);
20151990
}
1991+
1992+
/**
1993+
* {@inheritDoc}
1994+
*/
1995+
public function preExecuteActions(array $updateSequences): array
1996+
{
1997+
$foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'];
1998+
1999+
if (!$foreignKeysEnabled) {
2000+
foreach ($updateSequences as $updates) {
2001+
foreach ($updates as $update) {
2002+
foreach ($update->getActions() as $action) {
2003+
if ($action instanceof AddForeignKey) {
2004+
$foreignKeysEnabled = true;
2005+
break 3;
2006+
}
2007+
}
2008+
}
2009+
}
2010+
}
2011+
2012+
if ($foreignKeysEnabled) {
2013+
$this->execute('PRAGMA foreign_keys = OFF');
2014+
}
2015+
2016+
return [
2017+
'foreignKeysEnabled' => $foreignKeysEnabled,
2018+
];
2019+
}
2020+
2021+
/**
2022+
* {@inheritDoc}
2023+
*/
2024+
public function postExecuteActions(array $tableNames, array $preOptions): void
2025+
{
2026+
if ($preOptions['foreignKeysEnabled']) {
2027+
$this->execute('PRAGMA foreign_keys = ON');
2028+
$this->validateForeignKeys($tableNames);
2029+
}
2030+
}
20162031
}

‎src/Phinx/Db/Plan/Plan.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,22 @@ protected function inverseUpdatesSequence(): array
143143
*/
144144
public function execute(AdapterInterface $executor): void
145145
{
146+
$updatesSequence = $this->updatesSequence();
147+
$preOptions = $executor->preExecuteActions($updatesSequence);
148+
146149
foreach ($this->tableCreates as $newTable) {
147150
$executor->createTable($newTable->getTable(), $newTable->getColumns(), $newTable->getIndexes());
148151
}
149152

150-
foreach ($this->updatesSequence() as $updates) {
153+
$tables = [];
154+
foreach ($updatesSequence as $updates) {
151155
foreach ($updates as $update) {
156+
$tables[] = $update->getTable()->getName();
152157
$executor->executeActions($update->getTable(), $update->getActions());
153158
}
154159
}
160+
161+
$executor->postExecuteActions(array_unique($tables), $preOptions);
155162
}
156163

157164
/**
@@ -162,15 +169,21 @@ public function execute(AdapterInterface $executor): void
162169
*/
163170
public function executeInverse(AdapterInterface $executor): void
164171
{
172+
$preOptions = $executor->preExecuteActions($this->inverseUpdatesSequence());
173+
$tables = [];
174+
165175
foreach ($this->inverseUpdatesSequence() as $updates) {
166176
foreach ($updates as $update) {
177+
$tables[] = $update->getTable()->getName();
167178
$executor->executeActions($update->getTable(), $update->getActions());
168179
}
169180
}
170181

171182
foreach ($this->tableCreates as $newTable) {
172183
$executor->createTable($newTable->getTable(), $newTable->getColumns(), $newTable->getIndexes());
173184
}
185+
186+
$executor->postExecuteActions(array_unique($tables), $preOptions);
174187
}
175188

176189
/**

‎tests/Phinx/Db/Adapter/SQLiteAdapterTest.php‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2387,6 +2387,7 @@ public function testAlterTableDoesViolateForeignKeyConstraintOnTargetTableChange
23872387
*/
23882388
public function testAlterTableDoesViolateForeignKeyConstraintOnSourceTableChange()
23892389
{
2390+
/** @var \Phinx\Db\Adapter\AdapterInterface&\PHPUnit\Framework\MockObject\MockObject $adapter */
23902391
$adapter = $this
23912392
->getMockBuilder(SQLiteAdapter::class)
23922393
->setConstructorArgs([SQLITE_DB_CONFIG, new ArrayInput([]), new NullOutput()])
@@ -2396,14 +2397,19 @@ public function testAlterTableDoesViolateForeignKeyConstraintOnSourceTableChange
23962397
$adapterReflection = new ReflectionObject($adapter);
23972398
$queryReflection = $adapterReflection->getParentClass()->getMethod('query');
23982399

2400+
$count = 0;
23992401
$adapter
24002402
->expects($this->atLeastOnce())
24012403
->method('query')
2402-
->willReturnCallback(function (string $sql, array $params = []) use ($adapter, $queryReflection) {
2404+
->willReturnCallback(function (string $sql, array $params = []) use ($adapter, &$count, $queryReflection) {
24032405
if ($sql === 'PRAGMA foreign_key_check(`comments`)') {
2404-
$adapter->execute('PRAGMA foreign_keys = OFF');
2405-
$adapter->execute('DELETE FROM articles');
2406-
$adapter->execute('PRAGMA foreign_keys = ON');
2406+
$count++;
2407+
2408+
if ($count > 1) {
2409+
$adapter->execute('PRAGMA foreign_keys = OFF');
2410+
$adapter->execute('DELETE FROM articles');
2411+
$adapter->execute('PRAGMA foreign_keys = ON');
2412+
}
24072413
}
24082414

24092415
return $queryReflection->invoke($adapter, $sql, $params);

0 commit comments

Comments
 (0)