Implement foreign keys (#61)

* Add/remove foreign keys * Add option to disable foreign key checks * Add foreign key tests * Don't drop keys that don't exist
graze · Aug 4, 2019 · 96b0295 · 96b0295
1 parent c341cad
commit 96b0295
Show file tree

Hide file tree

Showing 5 changed files with 172 additions and 14 deletions.
diff --git a/src/Command/Diff.php b/src/Command/Diff.php
@@ -42,6 +42,7 @@ class Diff extends Command
     const OPTION_NO_ALTER_ENGINE    = 'no-alter-engine';
     const OPTION_LOG_SKIPPED        = 'log-skipped';
     const OPTION_NO_LOG_SKIPPED     = 'no-log-skipped';
+    const OPTION_NO_FOREIGN_KEY_CHECKS = 'no-foreign-key-checks';
 
     /** @var string */
     private $engine = 'InnoDB';
@@ -65,6 +66,8 @@ class Diff extends Command
     private $logDir = null;
     /** @var bool */
     private $logSkipped = true;
+    /** @var bool */
+    private $disableForeignKeyChecks = false;
 
     protected function configure()
     {
@@ -154,6 +157,7 @@ protected function configure()
 
         $this->addOption(self::OPTION_LOG_SKIPPED);
         $this->addOption(self::OPTION_NO_LOG_SKIPPED);
+        $this->addOption(self::OPTION_NO_FOREIGN_KEY_CHECKS);
     }
 
     /**
@@ -177,6 +181,9 @@ private function getCurrentSchema(Connection $connection, $dbName)
 
         $dump = new MysqlDump();
         $dump->setDefaultDatabase($dbName);
+        // Disable adding indexes for foreign keys on the current schema, if the new schema doesn't have the foreign
+        // key then both the foreign key and the index would be dropped however the index won't exist.
+        $dump->setAddIndexForForeignKey(false);
         $dump->parse($stream);
 
         return $dump;
@@ -212,6 +219,10 @@ private function applyChanges(Connection $connection, $connectionName, array $di
         if ($this->applyChanges == 'no') {
             return;
         }
+
+        if ($this->disableForeignKeyChecks) {
+            $connection->executeQuery('SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0');
+        }
 
         $confirm = $this->applyChanges == 'confirm';
         $defaultResponse = 'y';
@@ -284,6 +295,10 @@ private function applyChanges(Connection $connection, $connectionName, array $di
                 );
             }
         }
+
+        if ($this->disableForeignKeyChecks) {
+            $connection->executeQuery('SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS');
+        }
     }
 
     /**
@@ -321,6 +336,10 @@ protected function execute(InputInterface $input, OutputInterface $output)
             $this->logSkipped = false;
         }
 
+        if ($input->getOption(self::OPTION_NO_FOREIGN_KEY_CHECKS)) {
+            $this->disableForeignKeyChecks = true;
+        }
+
         try {
             $config = new Config($this->configFile);
             $config->parse();

diff --git a/src/Parse/CreateTable.php b/src/Parse/CreateTable.php
@@ -25,6 +25,9 @@ class CreateTable
 
     /** @var array */
     private $covers = [];
+
+    /** @var bool */
+    private $addIndexForForeignKey = true;
 
     /**
      * Constructor.
@@ -57,6 +60,16 @@ public function setDefaultEngine($engine)
         $this->options->setDefaultEngine($engine);
     }
 
+    /**
+     * Sets whether to add an index for each foreign key if one isn't defined, this is the default behaviour of MySQL.
+     *
+     * @param bool $addIndexForForeignKey
+     */
+    public function setAddIndexForForeignKey($addIndexForForeignKey)
+    {
+        $this->addIndexForForeignKey = $addIndexForForeignKey;
+    }
+
     /**
      * Parses a table definition from $stream.
      *
@@ -288,19 +301,22 @@ private function processIndexes()
 
         foreach ($this->indexes as $index) {
             if ($index->type === 'FOREIGN KEY') {
-                // TODO - doesn't correctly deal with indexes like foo(10)
-                $lookup = implode('\0', $index->getColumns());
-                if (!array_key_exists($lookup, $this->covers)) {
-                    $newIndex = new IndexDefinition();
-                    $newIndex->type = 'KEY';
-                    $newIndex->columns = $index->columns;
-                    if (!is_null($index->constraint)) {
-                        $newIndex->name = $index->constraint;
-                    } elseif (!is_null($index->name)) {
-                        $newIndex->name = $index->name;
+                if ($this->addIndexForForeignKey) {
+                    // TODO - doesn't correctly deal with indexes like foo(10)
+                    $lookup = implode('\0', $index->getColumns());
+                    if (!array_key_exists($lookup, $this->covers)) {
+                        $newIndex = new IndexDefinition();
+                        $newIndex->type = 'KEY';
+                        $newIndex->columns = $index->columns;
+                        if (!is_null($index->constraint)) {
+                            $newIndex->name = $index->constraint;
+                        } elseif (!is_null($index->name)) {
+                            $newIndex->name = $index->name;
+                        }
+                        $indexes[] = $newIndex;
                     }
-                    $indexes[] = $newIndex;
                 }
+
                 $foreign = new IndexDefinition();
                 if (is_null($index->constraint)) {
                     $foreign->constraint = $this->name . '_ibfk_' . ++$ibfkCounter;
@@ -369,7 +385,7 @@ private function processIndexes()
             $this->indexes[$index->name] = $index;
         }
         foreach ($foreigns as $foreign) {
-            $this->foreigns[] = $foreign;
+            $this->foreigns[$foreign->constraint] = $foreign;
         }
     }
 
@@ -417,6 +433,7 @@ public function diff(CreateTable $that, array $flags = [])
         $alters = array_merge(
             $this->diffColumns($that),
             $this->diffIndexes($that),
+            $this->diffForeigns($that),
             $this->diffOptions($that, [
                 'alterEngine' => $flags['alterEngine']
             ])
@@ -520,8 +537,6 @@ private function diffIndexes(CreateTable $that)
                         $alter = "DROP PRIMARY KEY";
                         break;
 
-                // TODO - foreign keys???
-
                     default:
                         $alter = "DROP KEY " . Token::escapeIdentifier($indexName);
                         break;
@@ -541,6 +556,33 @@ private function diffIndexes(CreateTable $that)
         return $alters;
     }
 
+    /**
+     * @param CreateTable $that
+     * @return array
+     */
+    private function diffForeigns(CreateTable $that)
+    {
+        $alters = [];
+
+        foreach ($this->foreigns as $foreignName => $foreign) {
+            if (!array_key_exists($foreignName, $that->foreigns) ||
+                $foreign->toString() !== $that->foreigns[$foreignName]->toString()
+            ) {
+                $alters[] = "DROP FOREIGN KEY " . Token::escapeIdentifier($foreignName);
+            }
+        }
+
+        foreach ($that->foreigns as $foreignName => $foreign) {
+            if (!array_key_exists($foreignName, $this->foreigns) ||
+                $foreign->toString() !== $this->foreigns[$foreignName]->toString()
+            ) {
+                $alters[] = "ADD " . $foreign->toString();
+            }
+        }
+
+        return $alters;
+    }
+
     /**
      * @param CreateTable $that
      * @param array $flags

diff --git a/src/Parse/MysqlDump.php b/src/Parse/MysqlDump.php
@@ -25,6 +25,8 @@ class MysqlDump
     private $defaultEngine = 'InnoDB';
     /** @var CollationInfo */
     private $defaultCollation = null;
+    /** @var bool */
+    private $addIndexForForeignKey = true;
 
     /**
      * Constructor
@@ -104,6 +106,16 @@ public function setDefaultEngine($engine)
         $this->defaultEngine = $engine;
     }
 
+    /**
+     * Sets whether to add an index for each foreign key if one isn't defined, this is the default behaviour of MySQL.
+     *
+     * @param bool $addIndexForForeignKey
+     */
+    public function setAddIndexForForeignKey($addIndexForForeignKey)
+    {
+        $this->addIndexForForeignKey = $addIndexForForeignKey;
+    }
+
     /**
      * Sets the default collation to assume when no collation or charset
      * is specified in CREATE DATABASE or CREATE TABLE.
@@ -151,6 +163,7 @@ public function parse(TokenStream $stream, array $flags = [])
                 }
                 $table = new CreateTable($this->database->getCollation());
                 $table->setDefaultEngine($this->defaultEngine);
+                $table->setAddIndexForForeignKey($this->addIndexForForeignKey);
                 $table->parse($stream);
                 $stream->expect(Token::SYMBOL, ';');
 

diff --git a/tests/Parse/CreateTableTest.php b/tests/Parse/CreateTableTest.php
@@ -115,6 +115,7 @@ public function testDiff($firstTableText, $secondTableText, $expected)
         $firstStream = $this->makeStream($firstTableText);
         $firstTable = new CreateTable($collation);
         $firstTable->setDefaultEngine('InnoDB');
+        $firstTable->setAddIndexForForeignKey(false);
         $firstTable->parse($firstStream);
 
         $secondStream = $this->makeStream($secondTableText);
@@ -136,6 +137,7 @@ public function diffProvider()
 
         foreach ([
                     'columns.sql',
+                    'foreignKeys.sql',
                     'indexes.sql',
                     'simpleDiff.sql'
                  ] as $file) {

diff --git a/tests/Parse/sql/diff/foreignKeys.sql b/tests/Parse/sql/diff/foreignKeys.sql
@@ -0,0 +1,82 @@
+-- test - Add unnamed foreign key
+create table t (
+    `ux` int(11) DEFAULT NULL
+);
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+ALTER TABLE `t`
+ADD KEY `ux` (`ux`),
+ADD CONSTRAINT `t_ibfk_1` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+
+-- test - Add named foreign key
+create table t (
+    `ux` int(11) DEFAULT NULL
+);
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    CONSTRAINT `forKey1` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+ALTER TABLE `t`
+ADD KEY `forKey1` (`ux`),
+ADD CONSTRAINT `forKey1` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+
+-- test - Key automatically gets added for foreign key as per MySQL
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    CONSTRAINT `forKey1` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    CONSTRAINT `forKey1` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+ALTER TABLE `t`
+ADD KEY `forKey1` (`ux`)
+
+-- test - Automatically added key already exists in the current table so no change is required
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    KEY `forKey1` (`ux`),
+    CONSTRAINT `forKey1` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    CONSTRAINT `forKey1` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+
+-- test - Drop unnamed foreign key
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+create table t (
+    `ux` int(11) DEFAULT NULL
+);
+ALTER TABLE `t`
+DROP FOREIGN KEY `t_ibfk_1`
+
+-- test - Drop named foreign key
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    CONSTRAINT `forKey1` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+create table t (
+    `ux` int(11) DEFAULT NULL
+);
+ALTER TABLE `t`
+DROP FOREIGN KEY `forKey1`
+
+-- test - Rename foreign key
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    CONSTRAINT `foo` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+create table t (
+    `ux` int(11) DEFAULT NULL,
+    CONSTRAINT `bar` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)
+);
+ALTER TABLE `t`
+ADD KEY `bar` (`ux`),
+DROP FOREIGN KEY `foo`,
+ADD CONSTRAINT `bar` FOREIGN KEY (`ux`) REFERENCES `u` (`x`)