From d316a207352ee3a2ca555a0dd9bafce791a1429b Mon Sep 17 00:00:00 2001
From: Mark Story <mark@mark-story.com>
Date: Wed, 4 Sep 2024 23:06:49 -0400
Subject: [PATCH 1/2] Disable foreign key checks when removing temp tables

Attempt to reproduce the problem reported in #741 but added a test that
covers the scenario based on current information. Perhaps the problem
can be reproduced in CI. There is also a chance that the test suite
harness makes this challenging to reproduce because of wrapping
transactions.
---
 src/Db/Adapter/SqliteAdapter.php              |  7 +++++
 src/Db/Table/Table.php                        |  1 +
 .../TestCase/Db/Adapter/SqliteAdapterTest.php | 26 +++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php
index 7a9df3e7..a2a43d3b 100644
--- a/src/Db/Adapter/SqliteAdapter.php
+++ b/src/Db/Adapter/SqliteAdapter.php
@@ -1046,7 +1046,14 @@ protected function copyAndDropTmpTable(AlterInstructions $instructions, string $
                 $state['selectColumns']
             );
 
+            $foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'] ?? false;
+            if ($foreignKeysEnabled) {
+                // $this->execute('PRAGMA foreign_keys = OFF');
+            }
             $this->execute(sprintf('DROP TABLE %s', $this->quoteTableName($tableName)));
+            if ($foreignKeysEnabled) {
+                // $this->execute('PRAGMA foreign_keys = ON');
+            }
             $this->execute(sprintf(
                 'ALTER TABLE %s RENAME TO %s',
                 $this->quoteTableName($state['tmpTableName']),
diff --git a/src/Db/Table/Table.php b/src/Db/Table/Table.php
index 9b2685b1..70f270f6 100644
--- a/src/Db/Table/Table.php
+++ b/src/Db/Table/Table.php
@@ -12,6 +12,7 @@
 
 /**
  * @internal
+ * @TODO rename this to `TableMetadata` having two classes with very similar names is confusing for me.
  */
 class Table
 {
diff --git a/tests/TestCase/Db/Adapter/SqliteAdapterTest.php b/tests/TestCase/Db/Adapter/SqliteAdapterTest.php
index aa9e7a5a..9b34858c 100644
--- a/tests/TestCase/Db/Adapter/SqliteAdapterTest.php
+++ b/tests/TestCase/Db/Adapter/SqliteAdapterTest.php
@@ -1601,6 +1601,32 @@ public function testAddColumnWithComment()
         $this->assertMatchesRegularExpression('/\/\* Comments from "column1" \*\//', $sql);
     }
 
+    public function testAddColumnTableWithConstraint()
+    {
+        $this->adapter->execute('PRAGMA foreign_keys = ON');
+        $roles = new Table('constraint_roles', [], $this->adapter);
+        $roles->addColumn('name', 'string')
+            ->save();
+        $users = new Table('constraint_users', [], $this->adapter);
+        $users->addColumn('username', 'string')
+            ->addColumn('role_id', 'integer', ['null' => false])
+            ->addForeignKey(['role_id'], $roles->getTable(), ['id'])
+            ->save();
+
+        $this->adapter->insert($roles->getTable(), ['name' => 'admin']);
+        $this->adapter->insert($users->getTable(), ['username' => 'test', 'role_id' => 1]);
+
+        $updatedRoles = new Table($roles->getName(), [], $this->adapter);
+        // This should fail, but passes locally :(
+        $updatedRoles
+            ->addColumn('description', 'string', ['default' => 'short desc'])
+            ->update();
+        $res = $this->adapter->fetchAll('select * from sqlite_master where type = \'table\'');
+        $res = $this->adapter->fetchRow('select * from constraint_roles LIMIT 1');
+        $this->assertArrayHasKey('description', $res, 'Should have new column in output');
+        $this->assertEquals('short desc', $res['description']);
+    }
+
     public function testPhinxTypeLiteral()
     {
         $this->assertEquals(

From d8ac1b0d1cb580719ce9f3c3e6dc90715e49fd85 Mon Sep 17 00:00:00 2001
From: Mark Story <mark@mark-story.com>
Date: Mon, 9 Sep 2024 10:29:08 -0400
Subject: [PATCH 2/2] Fix up stan errors

While these foreign key toggles appear to work in tests. When testing
the same migration operations within an application the contraint error
is reproducible and not fixed still.
---
 src/Db/Adapter/SqliteAdapter.php | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php
index a2a43d3b..98f3cd82 100644
--- a/src/Db/Adapter/SqliteAdapter.php
+++ b/src/Db/Adapter/SqliteAdapter.php
@@ -1046,13 +1046,14 @@ protected function copyAndDropTmpTable(AlterInstructions $instructions, string $
                 $state['selectColumns']
             );
 
-            $foreignKeysEnabled = (bool)$this->fetchRow('PRAGMA foreign_keys')['foreign_keys'] ?? false;
+            $result = $this->fetchRow('PRAGMA foreign_keys');
+            $foreignKeysEnabled = $result ? (bool)$result['foreign_keys'] : false;
             if ($foreignKeysEnabled) {
-                // $this->execute('PRAGMA foreign_keys = OFF');
+                $this->execute('PRAGMA foreign_keys = OFF');
             }
             $this->execute(sprintf('DROP TABLE %s', $this->quoteTableName($tableName)));
             if ($foreignKeysEnabled) {
-                // $this->execute('PRAGMA foreign_keys = ON');
+                $this->execute('PRAGMA foreign_keys = ON');
             }
             $this->execute(sprintf(
                 'ALTER TABLE %s RENAME TO %s',