Merge pull request #66 from webrium/fix/sqlite-foreign-key-enforcement

fix(sqlite): enforce foreign key constraints on connect

Author: benkhalife

src/Connection/Connection.php

@@ -683,6 +683,10 @@ protected function createPdo(array $config): PDO
             $this->setMysqlCharset($pdo, $config);
         }
 
+        if ($driver === Config::SQLITE) {
+            $this->enableSqliteForeignKeys($pdo);
+        }
+
         return $pdo;
     }
 
@@ -737,4 +741,18 @@ protected function setMysqlCharset(PDO $pdo, array $config): void
 
         $pdo->exec("SET NAMES '{$charset}' COLLATE '{$collation}'");
     }
+
+    /**
+     * Enable foreign key constraint enforcement for SQLite.
+     *
+     * SQLite ignores FOREIGN KEY constraints by default; each connection
+     * must opt in explicitly.
+     *
+     * @param  PDO $pdo
+     * @return void
+     */
+    protected function enableSqliteForeignKeys(PDO $pdo): void
+    {
+        $pdo->exec('PRAGMA foreign_keys = ON');
+    }
 }

src/Schema.php

@@ -74,15 +74,8 @@ public static function create(string $table, callable $callback, ?string $connec
             $conn->statement($idx);
         }
 
-        // Foreign keys are already inlined as column-level CONSTRAINTs by
-        // compileCreate() above (see SchemaGrammar::compileCreate()), so
-        // they must NOT be re-emitted here via compileForeignKeys() — doing
-        // so duplicates every constraint as a separate ALTER TABLE ADD
-        // CONSTRAINT statement, which MySQL rejects (errno 121, duplicate
-        // constraint) and SQLite rejects outright (ALTER TABLE ADD
-        // CONSTRAINT isn't valid SQLite DDL). compileForeignKeys() is still
-        // used correctly by Schema::table() below, where the table (and
-        // its inline constraints) already exist.
+        // Foreign keys are already inlined into the CREATE TABLE statement
+        // by compileCreate(); do not re-emit them via compileForeignKeys().
 
         // PostgreSQL column comments (separate COMMENT ON COLUMN statements)
         if ($grammar instanceof PostgresSchemaGrammar) {

tests/Integration/SchemaForeignKeyTest.php

@@ -9,24 +9,8 @@
 
 /**
  * Regression coverage for Schema::create() double-emitting foreign key
- * constraints.
- *
- * Schema::create() compiles the CREATE TABLE statement (which already
- * inlines every `$table->foreign()` as a column-level CONSTRAINT, see
- * SchemaGrammar::compileCreate()) and then, unconditionally, ALSO runs
- * SchemaGrammar::compileForeignKeys() — which re-emits the very same
- * constraints as separate `ALTER TABLE ... ADD CONSTRAINT` statements.
- * No grammar overrides compileForeignKeys() to suppress this for the
- * "already inlined at CREATE TABLE time" case, so every driver executes
- * a duplicate constraint statement:
- *
- *  - MySQL:  rejects the duplicate constraint name (errno 121,
- *            "Duplicate key on write or update").
- *  - SQLite: has no `ALTER TABLE ... ADD CONSTRAINT` syntax at all, so
- *            the statement is a hard syntax error.
- *
- * A table defined with `$table->foreign()` therefore cannot be created
- * via Schema::create() on any driver.
+ * constraints (once inlined in CREATE TABLE, once more via a redundant
+ * ALTER TABLE ADD CONSTRAINT).
  */
 class SchemaForeignKeyTest extends IntegrationTestCase
 {
@@ -54,19 +38,8 @@ public function testCreateWithForeignKeyDoesNotThrow(): void
         }
     }
 
-    /**
-     * SQLite never enforces foreign keys unless the connection issues
-     * `PRAGMA foreign_keys = ON` — this library does not do so, which is a
-     * separate, pre-existing gap unrelated to the double-emission bug this
-     * file targets. Skipped here rather than silently asserting something
-     * false for that driver.
-     */
     public function testCreateWithForeignKeyActuallyEnforcesTheConstraint(): void
     {
-        if (strtolower((string) (getenv('DB_DRIVER') ?: 'sqlite')) === 'sqlite') {
-            $this->markTestSkipped('SQLite FK enforcement requires PRAGMA foreign_keys=ON, which this library does not set (separate gap).');
-        }
-
         $parent = 'sfk_test_parents2';
         $child  = 'sfk_test_children2';
 
@@ -81,9 +54,6 @@ public function testCreateWithForeignKeyActuallyEnforcesTheConstraint(): void
                 $t->foreign('parent_id')->references('id')->on($parent);
             });
 
-            // A reference to a non-existent parent row must be rejected —
-            // proof the constraint is really enforced by the DB, not just
-            // that CREATE TABLE happened to succeed some other way.
             $threw = false;
             try {
                 \Foxdb\DB::table($child)->insert(['parent_id' => 999999]);

tests/Integration/SqliteForeignKeyPragmaTest.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Foxdb\Tests\Integration;
+
+use Foxdb\DB;
+
+/**
+ * SQLite does not enforce FOREIGN KEY constraints unless a connection
+ * issues `PRAGMA foreign_keys = ON`. Connection::createPdo() must set
+ * this for every SQLite connection it opens.
+ *
+ * Run:
+ *   DB_DRIVER=sqlite vendor/bin/phpunit --testsuite=integration
+ *      --filter=SqliteForeignKeyPragmaTest
+ */
+class SqliteForeignKeyPragmaTest extends IntegrationTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        if (strtolower((string) (getenv('DB_DRIVER') ?: 'sqlite')) !== 'sqlite') {
+            $this->markTestSkipped('SQLite-specific.');
+        }
+    }
+
+    public function testForeignKeysPragmaIsEnabledOnConnect(): void
+    {
+        $row = DB::connection()->selectOne('PRAGMA foreign_keys');
+
+        $this->assertSame(1, (int) ((array) $row)['foreign_keys']);
+    }
+}