Skip to content

Rework sqlite fk pragma handling #2365

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

Open
wants to merge 8 commits into
base: 0.x
Choose a base branch
from
Open
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
15 changes: 15 additions & 0 deletions src/Phinx/Db/Adapter/AbstractAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -410,4 +410,19 @@ protected function hasCreatedTable(string $tableName): bool

return in_array($tableName, $this->createdTables, true);
}

/**
* {@inheritDoc}
*/
public function preExecuteActions(array $updateSequences): array
{
return [];
}

/**
* {@inheritDoc}
*/
public function postExecuteActions(array $tableNames, array $preOptions): void
{
}
}
17 changes: 17 additions & 0 deletions src/Phinx/Db/Adapter/AdapterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,14 @@ public function rollbackTransaction(): void;
*/
public function execute(string $sql, array $params = []): int;

/**
* Function to be called before executing any migration actions.
*
* @param \Phinx\Db\Plan\AlterTable[][] $updateSequences List of update sequences to be executed
* @return array
*/
public function preExecuteActions(array $updateSequences): array;

/**
* Executes a list of migration actions for the given table
*
Expand All @@ -284,6 +292,15 @@ public function execute(string $sql, array $params = []): int;
*/
public function executeActions(Table $table, array $actions): void;

/**
* Function to be called after executing any migration actions.
*
* @param array $tableNames List of table names that were affected by the actions
* @param array $preOptions Options that were set before executing the actions
* @return void
*/
public function postExecuteActions(array $tableNames, array $preOptions): void;

/**
* Returns a new Query object
*
Expand Down
16 changes: 16 additions & 0 deletions src/Phinx/Db/Adapter/AdapterWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,14 @@ public function getConnection(): PDO
return $this->getAdapter()->getConnection();
}

/**
* {@inheritDoc}
*/
public function preExecuteActions(array $updateSequences): array
{
return $this->getAdapter()->preExecuteActions($updateSequences);
}

/**
* @inheritDoc
*/
Expand All @@ -482,6 +490,14 @@ public function executeActions(Table $table, array $actions): void
$this->getAdapter()->executeActions($table, $actions);
}

/**
* {@inheritDoc}
*/
public function postExecuteActions(array $tableNames, array $preOptions): void
{
$this->getAdapter()->postExecuteActions($tableNames, $preOptions);
}

/**
* @inheritDoc
*/
Expand Down
143 changes: 79 additions & 64 deletions src/Phinx/Db/Adapter/SQLiteAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use InvalidArgumentException;
use PDO;
use PDOException;
use Phinx\Db\Action\AddForeignKey;
use Phinx\Db\Table\Column;
use Phinx\Db\Table\ForeignKey;
use Phinx\Db\Table\Index;
Expand Down Expand Up @@ -1034,57 +1035,52 @@
* the given table, and of those tables whose constraints are
* targeting it.
*
* @param \Phinx\Db\Util\AlterInstructions $instructions The instructions to process
* @param string $tableName The name of the table for which to check constraints.
* @return \Phinx\Db\Util\AlterInstructions
* @param string|array<string> $tableNames The name of the table for which to check constraints.
* @return void
*/
protected function validateForeignKeys(AlterInstructions $instructions, string $tableName): AlterInstructions
protected function validateForeignKeys(string|array $tableNames): void
{
$instructions->addPostStep(function ($state) use ($tableName) {
$tablesToCheck = [
$tableName,
];

$otherTables = $this
->query(
"SELECT name FROM sqlite_master WHERE type = 'table' AND name != ?",
[$tableName],
)
->fetchAll();

foreach ($otherTables as $otherTable) {
$foreignKeyList = $this->getTableInfo($otherTable['name'], 'foreign_key_list');
foreach ($foreignKeyList as $foreignKey) {
if (strcasecmp($foreignKey['table'], $tableName) === 0) {
$tablesToCheck[] = $otherTable['name'];
break;
}
}
}

$tablesToCheck = array_unique(array_map('strtolower', $tablesToCheck));
if (!is_array($tableNames)) {
$tableNames = [$tableNames];

Check warning on line 1044 in src/Phinx/Db/Adapter/SQLiteAdapter.php

View check run for this annotation

Codecov / codecov/patch

src/Phinx/Db/Adapter/SQLiteAdapter.php#L1044

Added line #L1044 was not covered by tests
}

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

$stmt = $this->query(
sprintf('PRAGMA %sforeign_key_check(%s)', $schema, $this->quoteTableName($tableToCheck)),
);
$row = $stmt->fetch();
$stmt->closeCursor();
$otherTables = $this
->query(
"SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT IN (" . implode(',', array_fill(0, count($tableNames), '?')) . ')',
$tableNames,
)
->fetchAll();

if (is_array($row)) {
throw new RuntimeException(sprintf(
'Integrity constraint violation: FOREIGN KEY constraint on `%s` failed.',
$tableToCheck,
));
foreach ($otherTables as $otherTable) {
$foreignKeyList = $this->getTableInfo($otherTable['name'], 'foreign_key_list');
foreach ($foreignKeyList as $foreignKey) {
if (in_array(strtolower($foreignKey['table']), $tableNames)) {
$tablesToCheck[] = $otherTable['name'];
break;
}
}
}

return $state;
});
$tablesToCheck = array_unique(array_map('strtolower', $tablesToCheck));

return $instructions;
foreach ($tablesToCheck as $tableToCheck) {
$schema = $this->getSchemaName($tableToCheck, true)['schema'];

$stmt = $this->query(
sprintf('PRAGMA %sforeign_key_check(%s)', $schema, $this->quoteTableName($tableToCheck)),
);
$row = $stmt->fetch();
$stmt->closeCursor();

if (is_array($row)) {
throw new RuntimeException(sprintf(
'Integrity constraint violation: FOREIGN KEY constraint on `%s` failed.',
$tableToCheck,
));
}
}
}

/**
Expand Down Expand Up @@ -1230,16 +1226,13 @@
* @param ?string $renamedOrRemovedColumnName The name of the renamed or removed column when part of a column
* rename/drop operation.
* @param ?string $newColumnName The new column name when part of a column rename operation.
* @param bool $validateForeignKeys Whether to validate foreign keys after the copy and drop operations. Note that
* enabling this option only has an effect when the `foreign_keys` PRAGMA is set to `ON`!
* @return \Phinx\Db\Util\AlterInstructions
*/
protected function endAlterByCopyTable(
AlterInstructions $instructions,
string $tableName,
?string $renamedOrRemovedColumnName = null,
?string $newColumnName = null,
bool $validateForeignKeys = true,
): AlterInstructions {
$instructions = $this->bufferIndicesAndTriggers($instructions, $tableName);

Expand All @@ -1251,26 +1244,9 @@
}
}

$foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'];

if ($foreignKeysEnabled) {
$instructions->addPostStep('PRAGMA foreign_keys = OFF');
}

$instructions = $this->copyAndDropTmpTable($instructions, $tableName);
$instructions = $this->recreateIndicesAndTriggers($instructions);

if ($foreignKeysEnabled) {
$instructions->addPostStep('PRAGMA foreign_keys = ON');
}

if (
$foreignKeysEnabled &&
$validateForeignKeys
) {
$instructions = $this->validateForeignKeys($instructions, $tableName);
}

return $instructions;
}

Expand Down Expand Up @@ -1661,7 +1637,7 @@
return $newState + $state;
});

return $this->endAlterByCopyTable($instructions, $tableName, null, null, false);
return $this->endAlterByCopyTable($instructions, $tableName, null, null);
}

/**
Expand All @@ -1673,7 +1649,6 @@

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

//Delete indexes from original table and recreate them in temporary table
Expand Down Expand Up @@ -2013,4 +1988,44 @@

return $this->decoratedConnection = $this->buildConnection(SqliteDriver::class, $options);
}

/**
* {@inheritDoc}
*/
public function preExecuteActions(array $updateSequences): array
{
$foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'];

if (!$foreignKeysEnabled) {
foreach ($updateSequences as $updates) {
foreach ($updates as $update) {
foreach ($update->getActions() as $action) {
if ($action instanceof AddForeignKey) {
$foreignKeysEnabled = true;
break 3;
}
}
}
}
}

if ($foreignKeysEnabled) {
$this->execute('PRAGMA foreign_keys = OFF');
}

return [
'foreignKeysEnabled' => $foreignKeysEnabled,
];
}

/**
* {@inheritDoc}
*/
public function postExecuteActions(array $tableNames, array $preOptions): void
{
if ($preOptions['foreignKeysEnabled']) {
$this->execute('PRAGMA foreign_keys = ON');
$this->validateForeignKeys($tableNames);
}
}
}
15 changes: 14 additions & 1 deletion src/Phinx/Db/Plan/Plan.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,22 @@ protected function inverseUpdatesSequence(): array
*/
public function execute(AdapterInterface $executor): void
{
$updatesSequence = $this->updatesSequence();
$preOptions = $executor->preExecuteActions($updatesSequence);

foreach ($this->tableCreates as $newTable) {
$executor->createTable($newTable->getTable(), $newTable->getColumns(), $newTable->getIndexes());
}

foreach ($this->updatesSequence() as $updates) {
$tables = [];
foreach ($updatesSequence as $updates) {
foreach ($updates as $update) {
$tables[] = $update->getTable()->getName();
$executor->executeActions($update->getTable(), $update->getActions());
}
}

$executor->postExecuteActions(array_unique($tables), $preOptions);
}

/**
Expand All @@ -162,15 +169,21 @@ public function execute(AdapterInterface $executor): void
*/
public function executeInverse(AdapterInterface $executor): void
{
$preOptions = $executor->preExecuteActions($this->inverseUpdatesSequence());
$tables = [];

foreach ($this->inverseUpdatesSequence() as $updates) {
foreach ($updates as $update) {
$tables[] = $update->getTable()->getName();
$executor->executeActions($update->getTable(), $update->getActions());
}
}

foreach ($this->tableCreates as $newTable) {
$executor->createTable($newTable->getTable(), $newTable->getColumns(), $newTable->getIndexes());
}

$executor->postExecuteActions(array_unique($tables), $preOptions);
}

/**
Expand Down
14 changes: 10 additions & 4 deletions tests/Phinx/Db/Adapter/SQLiteAdapterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2387,6 +2387,7 @@ public function testAlterTableDoesViolateForeignKeyConstraintOnTargetTableChange
*/
public function testAlterTableDoesViolateForeignKeyConstraintOnSourceTableChange()
{
/** @var \Phinx\Db\Adapter\AdapterInterface&\PHPUnit\Framework\MockObject\MockObject $adapter */
$adapter = $this
->getMockBuilder(SQLiteAdapter::class)
->setConstructorArgs([SQLITE_DB_CONFIG, new ArrayInput([]), new NullOutput()])
Expand All @@ -2396,14 +2397,19 @@ public function testAlterTableDoesViolateForeignKeyConstraintOnSourceTableChange
$adapterReflection = new ReflectionObject($adapter);
$queryReflection = $adapterReflection->getParentClass()->getMethod('query');

$count = 0;
$adapter
->expects($this->atLeastOnce())
->method('query')
->willReturnCallback(function (string $sql, array $params = []) use ($adapter, $queryReflection) {
->willReturnCallback(function (string $sql, array $params = []) use ($adapter, &$count, $queryReflection) {
if ($sql === 'PRAGMA foreign_key_check(`comments`)') {
$adapter->execute('PRAGMA foreign_keys = OFF');
$adapter->execute('DELETE FROM articles');
$adapter->execute('PRAGMA foreign_keys = ON');
$count++;

if ($count > 1) {
$adapter->execute('PRAGMA foreign_keys = OFF');
$adapter->execute('DELETE FROM articles');
$adapter->execute('PRAGMA foreign_keys = ON');
}
}

return $queryReflection->invoke($adapter, $sql, $params);
Expand Down
Loading