Merge pull request #338 from deeky666/fix-asset-identfier-quotation

Fix foreign table name quotation in schema table and Sqlite platform

lib/Doctrine/DBAL/Platforms/SqlitePlatform.php

@@ -281,9 +281,9 @@ class SqlitePlatform extends AbstractPlatform
     public function getForeignKeyDeclarationSQL(ForeignKeyConstraint $foreignKey)
     {
         return parent::getForeignKeyDeclarationSQL(new ForeignKeyConstraint(
-            $foreignKey->getLocalColumns(),
-            str_replace('.', '__', $foreignKey->getForeignTableName()),
-            $foreignKey->getForeignColumns(),
+            $foreignKey->getQuotedLocalColumns($this),
+            str_replace('.', '__', $foreignKey->getQuotedForeignTableName($this)),
+            $foreignKey->getQuotedForeignColumns($this),
             $foreignKey->getName(),
             $foreignKey->getOptions()
         ));

lib/Doctrine/DBAL/Schema/ForeignKeyConstraint.php

@@ -45,7 +45,9 @@ class ForeignKeyConstraint extends AbstractAsset implements Constraint
     protected $_localColumnNames;
 
     /**
-     * @var Identifier Asset identifier instance of the referenced table name the foreign key constraint is associated with.
+     * Table or asset identifier instance of the referenced table name the foreign key constraint is associated with.
+     *
+     * @var Table|Identifier
      */
     protected $_foreignTableName;
 
@@ -65,11 +67,11 @@ class ForeignKeyConstraint extends AbstractAsset implements Constraint
     /**
      * Initializes the foreign key constraint.
      *
-     * @param array       $localColumnNames   Names of the referencing table columns.
-     * @param string      $foreignTableName   Name of the referenced table.
-     * @param array       $foreignColumnNames Names of the referenced table columns.
-     * @param string|null $name               Name of the foreign key constraint.
-     * @param array       $options            Options associated with the foreign key constraint.
+     * @param array        $localColumnNames   Names of the referencing table columns.
+     * @param Table|string $foreignTableName   Referenced table.
+     * @param array        $foreignColumnNames Names of the referenced table columns.
+     * @param string|null  $name               Name of the foreign key constraint.
+     * @param array        $options            Options associated with the foreign key constraint.
      */
     public function __construct(array $localColumnNames, $foreignTableName, array $foreignColumnNames, $name = null, array $options = array())
     {
@@ -80,7 +82,13 @@ class ForeignKeyConstraint extends AbstractAsset implements Constraint
         $this->_localColumnNames = $localColumnNames
             ? array_combine($localColumnNames, array_map($identifierConstructorCallback, $localColumnNames))
             : array();
-        $this->_foreignTableName = new Identifier($foreignTableName);
+
+        if ($foreignTableName instanceof Table) {
+            $this->_foreignTableName = $foreignTableName;
+        } else {
+            $this->_foreignTableName = new Identifier($foreignTableName);
+        }
+
         $this->_foreignColumnNames = $foreignColumnNames
             ? array_combine($foreignColumnNames, array_map($identifierConstructorCallback, $foreignColumnNames))
             : array();

lib/Doctrine/DBAL/Schema/Table.php

@@ -28,7 +28,7 @@ use Doctrine\DBAL\DBALException;
 /**
  * Object Representation of a table
  *
- * 
+ *
  * @link    www.doctrine-project.org
  * @since   2.0
  * @version $Revision$
@@ -355,15 +355,11 @@ class Table extends AbstractAsset
     public function addNamedForeignKeyConstraint($name, $foreignTable, array $localColumnNames, array $foreignColumnNames, array $options=array())
     {
         if ($foreignTable instanceof Table) {
-            $foreignTableName = $foreignTable->getName();
-
             foreach ($foreignColumnNames as $columnName) {
                 if ( ! $foreignTable->hasColumn($columnName)) {
                     throw SchemaException::columnDoesNotExist($columnName, $foreignTable->getName());
                 }
             }
-        } else {
-            $foreignTableName = $foreignTable;
         }
 
         foreach ($localColumnNames as $columnName) {
@@ -373,7 +369,7 @@ class Table extends AbstractAsset
         }
 
         $constraint = new ForeignKeyConstraint(
-            $localColumnNames, $foreignTableName, $foreignColumnNames, $name, $options
+            $localColumnNames, $foreignTable, $foreignColumnNames, $name, $options
         );
         $this->_addForeignKeyConstraint($constraint);
 

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

@@ -465,12 +465,31 @@ abstract class AbstractPlatformTestCase extends \Doctrine\Tests\DbalTestCase
         $table = new Table('`quoted`');
         $table->addColumn('create', 'string');
         $table->addColumn('foo', 'string');
+        $table->addColumn('`bar`', 'string');
 
+        // Foreign table with reserved keyword as name (needs quotation).
         $foreignTable = new Table('foreign');
-        $foreignTable->addColumn('create', 'string');
-        $foreignTable->addColumn('bar', 'string');
+        $foreignTable->addColumn('create', 'string');    // Foreign column with reserved keyword as name (needs quotation).
+        $foreignTable->addColumn('bar', 'string');       // Foreign column with non-reserved keyword as name (does not need quotation).
+        $foreignTable->addColumn('`foo-bar`', 'string'); // Foreign table with special character in name (needs quotation on some platforms, e.g. Sqlite).
 
-        $table->addForeignKeyConstraint($foreignTable, array('create', 'foo'), array('create', 'bar'));
+        $table->addForeignKeyConstraint($foreignTable, array('create', 'foo', '`bar`'), array('create', 'bar', '`foo-bar`'), array(), 'FK_WITH_RESERVED_KEYWORD');
+
+        // Foreign table with non-reserved keyword as name (does not need quotation).
+        $foreignTable = new Table('foo');
+        $foreignTable->addColumn('create', 'string');    // Foreign column with reserved keyword as name (needs quotation).
+        $foreignTable->addColumn('bar', 'string');       // Foreign column with non-reserved keyword as name (does not need quotation).
+        $foreignTable->addColumn('`foo-bar`', 'string'); // Foreign table with special character in name (needs quotation on some platforms, e.g. Sqlite).
+
+        $table->addForeignKeyConstraint($foreignTable, array('create', 'foo', '`bar`'), array('create', 'bar', '`foo-bar`'), array(), 'FK_WITH_NON_RESERVED_KEYWORD');
+
+        // Foreign table with special character in name (needs quotation on some platforms, e.g. Sqlite).
+        $foreignTable = new Table('`foo-bar`');
+        $foreignTable->addColumn('create', 'string');    // Foreign column with reserved keyword as name (needs quotation).
+        $foreignTable->addColumn('bar', 'string');       // Foreign column with non-reserved keyword as name (does not need quotation).
+        $foreignTable->addColumn('`foo-bar`', 'string'); // Foreign table with special character in name (needs quotation on some platforms, e.g. Sqlite).
+
+        $table->addForeignKeyConstraint($foreignTable, array('create', 'foo', '`bar`'), array('create', 'bar', '`foo-bar`'), array(), 'FK_WITH_INTENDED_QUOTATION');
 
         $sql = $this->_platform->getCreateTableSQL($table, AbstractPlatform::CREATE_FOREIGNKEYS);
         $this->assertEquals($this->getQuotedColumnInForeignKeySQL(), $sql);

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

@@ -251,8 +251,10 @@ class MySqlPlatformTest extends AbstractPlatformTestCase
     protected function getQuotedColumnInForeignKeySQL()
     {
         return array(
-            'CREATE TABLE `quoted` (`create` VARCHAR(255) NOT NULL, foo VARCHAR(255) NOT NULL) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB',
-            'ALTER TABLE `quoted` ADD CONSTRAINT FK_22660D028FD6E0FB8C736521 FOREIGN KEY (`create`, foo) REFERENCES `foreign` (`create`, bar)',
+            'CREATE TABLE `quoted` (`create` VARCHAR(255) NOT NULL, foo VARCHAR(255) NOT NULL, `bar` VARCHAR(255) NOT NULL) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB',
+            'ALTER TABLE `quoted` ADD CONSTRAINT FK_WITH_RESERVED_KEYWORD FOREIGN KEY (`create`, foo, `bar`) REFERENCES `foreign` (`create`, bar, `foo-bar`)',
+            'ALTER TABLE `quoted` ADD CONSTRAINT FK_WITH_NON_RESERVED_KEYWORD FOREIGN KEY (`create`, foo, `bar`) REFERENCES foo (`create`, bar, `foo-bar`)',
+            'ALTER TABLE `quoted` ADD CONSTRAINT FK_WITH_INTENDED_QUOTATION FOREIGN KEY (`create`, foo, `bar`) REFERENCES `foo-bar` (`create`, bar, `foo-bar`)',
         );
     }
 

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

@@ -302,8 +302,10 @@ class OraclePlatformTest extends AbstractPlatformTestCase
     protected function getQuotedColumnInForeignKeySQL()
     {
         return array(
-            'CREATE TABLE "quoted" ("create" VARCHAR2(255) NOT NULL, foo VARCHAR2(255) NOT NULL)',
-            'ALTER TABLE "quoted" ADD CONSTRAINT FK_22660D028FD6E0FB8C736521 FOREIGN KEY ("create", foo) REFERENCES foreign ("create", bar)',
+            'CREATE TABLE "quoted" ("create" VARCHAR2(255) NOT NULL, foo VARCHAR2(255) NOT NULL, "bar" VARCHAR2(255) NOT NULL)',
+            'ALTER TABLE "quoted" ADD CONSTRAINT FK_WITH_RESERVED_KEYWORD FOREIGN KEY ("create", foo, "bar") REFERENCES foreign ("create", bar, "foo-bar")',
+            'ALTER TABLE "quoted" ADD CONSTRAINT FK_WITH_NON_RESERVED_KEYWORD FOREIGN KEY ("create", foo, "bar") REFERENCES foo ("create", bar, "foo-bar")',
+            'ALTER TABLE "quoted" ADD CONSTRAINT FK_WITH_INTENDED_QUOTATION FOREIGN KEY ("create", foo, "bar") REFERENCES "foo-bar" ("create", bar, "foo-bar")',
         );
     }
 }

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

@@ -285,8 +285,10 @@ class PostgreSqlPlatformTest extends AbstractPlatformTestCase
     protected function getQuotedColumnInForeignKeySQL()
     {
         return array(
-            'CREATE TABLE "quoted" ("create" VARCHAR(255) NOT NULL, foo VARCHAR(255) NOT NULL)',
-            'ALTER TABLE "quoted" ADD CONSTRAINT FK_22660D028FD6E0FB8C736521 FOREIGN KEY ("create", foo) REFERENCES "foreign" ("create", bar) NOT DEFERRABLE INITIALLY IMMEDIATE',
+            'CREATE TABLE "quoted" ("create" VARCHAR(255) NOT NULL, foo VARCHAR(255) NOT NULL, "bar" VARCHAR(255) NOT NULL)',
+            'ALTER TABLE "quoted" ADD CONSTRAINT FK_WITH_RESERVED_KEYWORD FOREIGN KEY ("create", foo, "bar") REFERENCES "foreign" ("create", bar, "foo-bar") NOT DEFERRABLE INITIALLY IMMEDIATE',
+            'ALTER TABLE "quoted" ADD CONSTRAINT FK_WITH_NON_RESERVED_KEYWORD FOREIGN KEY ("create", foo, "bar") REFERENCES foo ("create", bar, "foo-bar") NOT DEFERRABLE INITIALLY IMMEDIATE',
+            'ALTER TABLE "quoted" ADD CONSTRAINT FK_WITH_INTENDED_QUOTATION FOREIGN KEY ("create", foo, "bar") REFERENCES "foo-bar" ("create", bar, "foo-bar") NOT DEFERRABLE INITIALLY IMMEDIATE',
         );
     }
 

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

@@ -221,7 +221,7 @@ class SQLServerPlatformTest extends AbstractPlatformTestCase
         $sql = $this->_platform->modifyLimitQuery('SELECT a.fromFoo, fromBar FROM foo', 10);
         $this->assertEquals('SELECT * FROM (SELECT a.fromFoo, fromBar, ROW_NUMBER() OVER (ORDER BY (SELECT 0)) AS doctrine_rownum FROM foo) AS doctrine_tbl WHERE doctrine_rownum BETWEEN 1 AND 10', $sql);
     }
-    
+
     /**
      * @group DDC-2470
      */
@@ -311,8 +311,10 @@ class SQLServerPlatformTest extends AbstractPlatformTestCase
     protected function getQuotedColumnInForeignKeySQL()
     {
         return array(
-            'CREATE TABLE [quoted] ([create] NVARCHAR(255) NOT NULL, foo NVARCHAR(255) NOT NULL)',
-            'ALTER TABLE [quoted] ADD CONSTRAINT FK_22660D028FD6E0FB8C736521 FOREIGN KEY ([create], foo) REFERENCES [foreign] ([create], bar)',
+            'CREATE TABLE [quoted] ([create] NVARCHAR(255) NOT NULL, foo NVARCHAR(255) NOT NULL, [bar] NVARCHAR(255) NOT NULL)',
+            'ALTER TABLE [quoted] ADD CONSTRAINT FK_WITH_RESERVED_KEYWORD FOREIGN KEY ([create], foo, [bar]) REFERENCES [foreign] ([create], bar, [foo-bar])',
+            'ALTER TABLE [quoted] ADD CONSTRAINT FK_WITH_NON_RESERVED_KEYWORD FOREIGN KEY ([create], foo, [bar]) REFERENCES foo ([create], bar, [foo-bar])',
+            'ALTER TABLE [quoted] ADD CONSTRAINT FK_WITH_INTENDED_QUOTATION FOREIGN KEY ([create], foo, [bar]) REFERENCES [foo-bar] ([create], bar, [foo-bar])',
         );
     }
 }

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

@@ -295,7 +295,11 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
     protected function getQuotedColumnInForeignKeySQL()
     {
         return array(
-            'CREATE TABLE "quoted" ("create" VARCHAR(255) NOT NULL, foo VARCHAR(255) NOT NULL, CONSTRAINT FK_22660D028FD6E0FB8C736521 FOREIGN KEY ("create", foo) REFERENCES "foreign" ("create", bar) NOT DEFERRABLE INITIALLY IMMEDIATE)',
+            'CREATE TABLE "quoted" (' .
+            '"create" VARCHAR(255) NOT NULL, foo VARCHAR(255) NOT NULL, "bar" VARCHAR(255) NOT NULL, ' .
+            'CONSTRAINT FK_WITH_RESERVED_KEYWORD FOREIGN KEY ("create", foo, "bar") REFERENCES "foreign" ("create", bar, "foo-bar") NOT DEFERRABLE INITIALLY IMMEDIATE, ' .
+            'CONSTRAINT FK_WITH_NON_RESERVED_KEYWORD FOREIGN KEY ("create", foo, "bar") REFERENCES foo ("create", bar, "foo-bar") NOT DEFERRABLE INITIALLY IMMEDIATE, ' .
+            'CONSTRAINT FK_WITH_INTENDED_QUOTATION FOREIGN KEY ("create", foo, "bar") REFERENCES "foo-bar" ("create", bar, "foo-bar") NOT DEFERRABLE INITIALLY IMMEDIATE)',
         );
     }
 }