Merge pull request #862 from mpoiriert/patch-2

Quote table and constraint names in drop foreign key / drop constraint SQL

fixes #1186

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -1450,14 +1450,17 @@ abstract class AbstractPlatform
      */
     public function getDropConstraintSQL($constraint, $table)
     {
-        if ($constraint instanceof Constraint) {
-            $constraint = $constraint->getQuotedName($this);
+        if (! $constraint instanceof Constraint) {
+            $constraint = new Identifier($constraint);
         }
 
-        if ($table instanceof Table) {
-            $table = $table->getQuotedName($this);
+        if (! $table instanceof Table) {
+            $table = new Identifier($table);
         }
 
+        $constraint = $constraint->getQuotedName($this);
+        $table = $table->getQuotedName($this);
+
         return 'ALTER TABLE ' . $table . ' DROP CONSTRAINT ' . $constraint;
     }
 
@@ -1471,14 +1474,17 @@ abstract class AbstractPlatform
      */
     public function getDropForeignKeySQL($foreignKey, $table)
     {
-        if ($foreignKey instanceof ForeignKeyConstraint) {
-            $foreignKey = $foreignKey->getQuotedName($this);
+        if (! $foreignKey instanceof ForeignKeyConstraint) {
+            $foreignKey = new Identifier($foreignKey);
         }
 
-        if ($table instanceof Table) {
-            $table = $table->getQuotedName($this);
+        if (! $table instanceof Table) {
+            $table = new Identifier($table);
         }
 
+        $foreignKey = $foreignKey->getQuotedName($this);
+        $table = $table->getQuotedName($this);
+
         return 'ALTER TABLE ' . $table . ' DROP FOREIGN KEY ' . $foreignKey;
     }
 

lib/Doctrine/DBAL/Platforms/OraclePlatform.php

@@ -691,14 +691,17 @@ END;';
      */
     public function getDropForeignKeySQL($foreignKey, $table)
     {
-        if ($foreignKey instanceof ForeignKeyConstraint) {
-            $foreignKey = $foreignKey->getQuotedName($this);
+        if (! $foreignKey instanceof ForeignKeyConstraint) {
+            $foreignKey = new Identifier($foreignKey);
         }
 
-        if ($table instanceof Table) {
-            $table = $table->getQuotedName($this);
+        if (! $table instanceof Table) {
+            $table = new Identifier($table);
         }
 
+        $foreignKey = $foreignKey->getQuotedName($this);
+        $table = $table->getQuotedName($this);
+
         return 'ALTER TABLE ' . $table . ' DROP CONSTRAINT ' . $foreignKey;
     }
 

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php

@@ -160,14 +160,17 @@ class SQLServerPlatform extends AbstractPlatform
      */
     public function getDropForeignKeySQL($foreignKey, $table)
     {
-        if ($foreignKey instanceof ForeignKeyConstraint) {
-            $foreignKey = $foreignKey->getQuotedName($this);
+        if (! $foreignKey instanceof ForeignKeyConstraint) {
+            $foreignKey = new Identifier($foreignKey);
         }
 
-        if ($table instanceof Table) {
-            $table = $table->getQuotedName($this);
+        if (! $table instanceof Table) {
+            $table = new Identifier($table);
         }
 
+        $foreignKey = $foreignKey->getQuotedName($this);
+        $table = $table->getQuotedName($this);
+
         return 'ALTER TABLE ' . $table . ' DROP CONSTRAINT ' . $foreignKey;
     }
 

lib/Doctrine/DBAL/Schema/SchemaDiff.php

@@ -154,7 +154,7 @@ class SchemaDiff
 
         if ($platform->supportsForeignKeyConstraints() && $saveMode == false) {
             foreach ($this->orphanedForeignKeys as $orphanedForeignKey) {
-                $sql[] = $platform->getDropForeignKeySQL($orphanedForeignKey, $orphanedForeignKey->getLocalTableName());
+                $sql[] = $platform->getDropForeignKeySQL($orphanedForeignKey, $orphanedForeignKey->getLocalTable());
             }
         }
 

tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php

@@ -571,6 +571,16 @@ abstract class AbstractMySQLPlatformTestCase extends AbstractPlatformTestCase
         );
     }
 
+    protected function getQuotesDropForeignKeySQL()
+    {
+        return 'ALTER TABLE `table` DROP FOREIGN KEY `select`';
+    }
+
+    protected function getQuotesDropConstraintSQL()
+    {
+        return 'ALTER TABLE `table` DROP CONSTRAINT `select`';
+    }
+
     public function testDoesNotPropagateDefaultValuesForUnsupportedColumnTypes()
     {
         $table = new Table("text_blob_default_value");

tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php

@@ -967,6 +967,52 @@ abstract class AbstractPlatformTestCase extends \Doctrine\Tests\DbalTestCase
         );
     }
 
+    /**
+     * @group DBAL-1237
+     */
+    public function testQuotesDropForeignKeySQL()
+    {
+        if (! $this->_platform->supportsForeignKeyConstraints()) {
+            $this->markTestSkipped(
+                sprintf('%s does not support foreign key constraints.', get_class($this->_platform))
+            );
+        }
+
+        $tableName = 'table';
+        $table = new Table($tableName);
+        $foreignKeyName = 'select';
+        $foreignKey = new ForeignKeyConstraint(array(), 'foo', array(), 'select');
+        $expectedSql = $this->getQuotesDropForeignKeySQL();
+
+        $this->assertSame($expectedSql, $this->_platform->getDropForeignKeySQL($foreignKeyName, $tableName));
+        $this->assertSame($expectedSql, $this->_platform->getDropForeignKeySQL($foreignKey, $table));
+    }
+
+    protected function getQuotesDropForeignKeySQL()
+    {
+        return 'ALTER TABLE "table" DROP FOREIGN KEY "select"';
+    }
+
+    /**
+     * @group DBAL-1237
+     */
+    public function testQuotesDropConstraintSQL()
+    {
+        $tableName = 'table';
+        $table = new Table($tableName);
+        $constraintName = 'select';
+        $constraint = new ForeignKeyConstraint(array(), 'foo', array(), 'select');
+        $expectedSql = $this->getQuotesDropConstraintSQL();
+
+        $this->assertSame($expectedSql, $this->_platform->getDropConstraintSQL($constraintName, $tableName));
+        $this->assertSame($expectedSql, $this->_platform->getDropConstraintSQL($constraint, $table));
+    }
+
+    protected function getQuotesDropConstraintSQL()
+    {
+        return 'ALTER TABLE "table" DROP CONSTRAINT "select"';
+    }
+
     protected function getStringLiteralQuoteCharacter()
     {
         return "'";

tests/Doctrine/Tests/DBAL/Platforms/AbstractPostgreSqlPlatformTestCase.php

@@ -669,6 +669,11 @@ abstract class AbstractPostgreSqlPlatformTestCase extends AbstractPlatformTestCa
         );
     }
 
+    protected function getQuotesDropForeignKeySQL()
+    {
+        return 'ALTER TABLE "table" DROP CONSTRAINT "select"';
+    }
+
     public function testGetNullCommentOnColumnSQL()
     {
         $this->assertEquals(

tests/Doctrine/Tests/DBAL/Platforms/AbstractSQLServerPlatformTestCase.php

@@ -1035,6 +1035,16 @@ abstract class AbstractSQLServerPlatformTestCase extends AbstractPlatformTestCas
         );
     }
 
+    protected function getQuotesDropForeignKeySQL()
+    {
+        return 'ALTER TABLE [table] DROP CONSTRAINT [select]';
+    }
+
+    protected function getQuotesDropConstraintSQL()
+    {
+        return 'ALTER TABLE [table] DROP CONSTRAINT [select]';
+    }
+
     /**
      * @dataProvider getGeneratesIdentifierNamesInDefaultConstraintDeclarationSQL
      * @group DBAL-830

tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php

@@ -543,6 +543,11 @@ class OraclePlatformTest extends AbstractPlatformTestCase
         );
     }
 
+    protected function getQuotesDropForeignKeySQL()
+    {
+        return 'ALTER TABLE "table" DROP CONSTRAINT "select"';
+    }
+
     /**
      * @group DBAL-423
      */

tests/Doctrine/Tests/DBAL/Schema/SchemaDiffTest.php

@@ -75,7 +75,10 @@ class SchemaDiffTest extends \PHPUnit_Framework_TestCase
         if ($unsafe) {
             $platform->expects($this->exactly(1))
                      ->method('getDropForeignKeySql')
-                     ->with($this->isInstanceof('Doctrine\DBAL\Schema\ForeignKeyConstraint'), $this->equalTo('local_table'))
+                     ->with(
+                         $this->isInstanceof('Doctrine\DBAL\Schema\ForeignKeyConstraint'),
+                         $this->isInstanceOf('Doctrine\DBAL\Schema\Table')
+                     )
                      ->will($this->returnValue('drop_orphan_fk'));
         }
         $platform->expects($this->exactly(1))