Skip to content

Commit

Permalink
fix foreign table name quotation in schema table and Sqlite platform …
Browse files Browse the repository at this point in the history
…and add more robust tests
  • Loading branch information
deeky666 committed Jun 18, 2013
1 parent 94f80b9 commit 1af95fc
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 29 deletions.
6 changes: 3 additions & 3 deletions lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -281,9 +281,9 @@ protected function _getCommonIntegerTypeDeclarationSQL(array $columnDef)
public function getForeignKeyDeclarationSQL(ForeignKeyConstraint $foreignKey) public function getForeignKeyDeclarationSQL(ForeignKeyConstraint $foreignKey)
{ {
return parent::getForeignKeyDeclarationSQL(new ForeignKeyConstraint( return parent::getForeignKeyDeclarationSQL(new ForeignKeyConstraint(
$foreignKey->getLocalColumns(), $foreignKey->getQuotedLocalColumns($this),
str_replace('.', '__', $foreignKey->getForeignTableName()), str_replace('.', '__', $foreignKey->getQuotedForeignTableName($this)),
$foreignKey->getForeignColumns(), $foreignKey->getQuotedForeignColumns($this),
$foreignKey->getName(), $foreignKey->getName(),
$foreignKey->getOptions() $foreignKey->getOptions()
)); ));
Expand Down
22 changes: 15 additions & 7 deletions lib/Doctrine/DBAL/Schema/ForeignKeyConstraint.php
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ class ForeignKeyConstraint extends AbstractAsset implements Constraint
protected $_localColumnNames; 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; protected $_foreignTableName;


Expand All @@ -65,11 +67,11 @@ class ForeignKeyConstraint extends AbstractAsset implements Constraint
/** /**
* Initializes the foreign key constraint. * Initializes the foreign key constraint.
* *
* @param array $localColumnNames Names of the referencing table columns. * @param array $localColumnNames Names of the referencing table columns.
* @param string $foreignTableName Name of the referenced table. * @param Table|string $foreignTableName Referenced table.
* @param array $foreignColumnNames Names of the referenced table columns. * @param array $foreignColumnNames Names of the referenced table columns.
* @param string|null $name Name of the foreign key constraint. * @param string|null $name Name of the foreign key constraint.
* @param array $options Options associated with 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()) public function __construct(array $localColumnNames, $foreignTableName, array $foreignColumnNames, $name = null, array $options = array())
{ {
Expand All @@ -80,7 +82,13 @@ public function __construct(array $localColumnNames, $foreignTableName, array $f
$this->_localColumnNames = $localColumnNames $this->_localColumnNames = $localColumnNames
? array_combine($localColumnNames, array_map($identifierConstructorCallback, $localColumnNames)) ? array_combine($localColumnNames, array_map($identifierConstructorCallback, $localColumnNames))
: array(); : array();
$this->_foreignTableName = new Identifier($foreignTableName);
if ($foreignTableName instanceof Table) {
$this->_foreignTableName = $foreignTableName;
} else {
$this->_foreignTableName = new Identifier($foreignTableName);
}

$this->_foreignColumnNames = $foreignColumnNames $this->_foreignColumnNames = $foreignColumnNames
? array_combine($foreignColumnNames, array_map($identifierConstructorCallback, $foreignColumnNames)) ? array_combine($foreignColumnNames, array_map($identifierConstructorCallback, $foreignColumnNames))
: array(); : array();
Expand Down
8 changes: 2 additions & 6 deletions lib/Doctrine/DBAL/Schema/Table.php
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
/** /**
* Object Representation of a table * Object Representation of a table
* *
* *
* @link www.doctrine-project.org * @link www.doctrine-project.org
* @since 2.0 * @since 2.0
* @version $Revision$ * @version $Revision$
Expand Down Expand Up @@ -355,15 +355,11 @@ public function addUnnamedForeignKeyConstraint($foreignTable, array $localColumn
public function addNamedForeignKeyConstraint($name, $foreignTable, array $localColumnNames, array $foreignColumnNames, array $options=array()) public function addNamedForeignKeyConstraint($name, $foreignTable, array $localColumnNames, array $foreignColumnNames, array $options=array())
{ {
if ($foreignTable instanceof Table) { if ($foreignTable instanceof Table) {
$foreignTableName = $foreignTable->getName();

foreach ($foreignColumnNames as $columnName) { foreach ($foreignColumnNames as $columnName) {
if ( ! $foreignTable->hasColumn($columnName)) { if ( ! $foreignTable->hasColumn($columnName)) {
throw SchemaException::columnDoesNotExist($columnName, $foreignTable->getName()); throw SchemaException::columnDoesNotExist($columnName, $foreignTable->getName());
} }
} }
} else {
$foreignTableName = $foreignTable;
} }


foreach ($localColumnNames as $columnName) { foreach ($localColumnNames as $columnName) {
Expand All @@ -373,7 +369,7 @@ public function addNamedForeignKeyConstraint($name, $foreignTable, array $localC
} }


$constraint = new ForeignKeyConstraint( $constraint = new ForeignKeyConstraint(
$localColumnNames, $foreignTableName, $foreignColumnNames, $name, $options $localColumnNames, $foreignTable, $foreignColumnNames, $name, $options
); );
$this->_addForeignKeyConstraint($constraint); $this->_addForeignKeyConstraint($constraint);


Expand Down
25 changes: 22 additions & 3 deletions tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -465,12 +465,31 @@ public function testQuotedColumnInForeignKeyPropagation()
$table = new Table('`quoted`'); $table = new Table('`quoted`');
$table->addColumn('create', 'string'); $table->addColumn('create', 'string');
$table->addColumn('foo', 'string'); $table->addColumn('foo', 'string');
$table->addColumn('`bar`', 'string');


// Foreign table with reserved keyword as name (needs quotation).
$foreignTable = new Table('foreign'); $foreignTable = new Table('foreign');
$foreignTable->addColumn('create', 'string'); $foreignTable->addColumn('create', 'string'); // Foreign column with reserved keyword as name (needs quotation).
$foreignTable->addColumn('bar', 'string'); $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); $sql = $this->_platform->getCreateTableSQL($table, AbstractPlatform::CREATE_FOREIGNKEYS);
$this->assertEquals($this->getQuotedColumnInForeignKeySQL(), $sql); $this->assertEquals($this->getQuotedColumnInForeignKeySQL(), $sql);
Expand Down
6 changes: 4 additions & 2 deletions tests/Doctrine/Tests/DBAL/Platforms/MySqlPlatformTest.php
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -251,8 +251,10 @@ protected function getQuotedColumnInIndexSQL()
protected function getQuotedColumnInForeignKeySQL() protected function getQuotedColumnInForeignKeySQL()
{ {
return array( 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', '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_22660D028FD6E0FB8C736521 FOREIGN KEY (`create`, foo) REFERENCES `foreign` (`create`, bar)', '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`)',
); );
} }


Expand Down
6 changes: 4 additions & 2 deletions tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -302,8 +302,10 @@ protected function getQuotedColumnInIndexSQL()
protected function getQuotedColumnInForeignKeySQL() protected function getQuotedColumnInForeignKeySQL()
{ {
return array( return array(
'CREATE TABLE "quoted" ("create" VARCHAR2(255) NOT NULL, foo VARCHAR2(255) NOT NULL)', 'CREATE TABLE "quoted" ("create" VARCHAR2(255) NOT NULL, foo VARCHAR2(255) NOT NULL, "bar" VARCHAR2(255) NOT NULL)',
'ALTER TABLE "quoted" ADD CONSTRAINT FK_22660D028FD6E0FB8C736521 FOREIGN KEY ("create", foo) REFERENCES foreign ("create", bar)', '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")',
); );
} }
} }
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -285,8 +285,10 @@ protected function getQuotedColumnInIndexSQL()
protected function getQuotedColumnInForeignKeySQL() protected function getQuotedColumnInForeignKeySQL()
{ {
return array( return array(
'CREATE TABLE "quoted" ("create" VARCHAR(255) NOT NULL, foo VARCHAR(255) NOT NULL)', 'CREATE TABLE "quoted" ("create" VARCHAR(255) NOT NULL, foo VARCHAR(255) NOT NULL, "bar" VARCHAR(255) NOT NULL)',
'ALTER TABLE "quoted" ADD CONSTRAINT FK_22660D028FD6E0FB8C736521 FOREIGN KEY ("create", foo) REFERENCES "foreign" ("create", bar) NOT DEFERRABLE INITIALLY IMMEDIATE', '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',
); );
} }


Expand Down
8 changes: 5 additions & 3 deletions tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public function testModifyLimitQueryWithFromColumnNames()
$sql = $this->_platform->modifyLimitQuery('SELECT a.fromFoo, fromBar FROM foo', 10); $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); $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 * @group DDC-2470
*/ */
Expand Down Expand Up @@ -311,8 +311,10 @@ protected function getQuotedColumnInIndexSQL()
protected function getQuotedColumnInForeignKeySQL() protected function getQuotedColumnInForeignKeySQL()
{ {
return array( return array(
'CREATE TABLE [quoted] ([create] NVARCHAR(255) NOT NULL, foo NVARCHAR(255) NOT NULL)', 'CREATE TABLE [quoted] ([create] NVARCHAR(255) NOT NULL, foo NVARCHAR(255) NOT NULL, [bar] NVARCHAR(255) NOT NULL)',
'ALTER TABLE [quoted] ADD CONSTRAINT FK_22660D028FD6E0FB8C736521 FOREIGN KEY ([create], foo) REFERENCES [foreign] ([create], bar)', '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])',
); );
} }
} }
6 changes: 5 additions & 1 deletion tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -295,7 +295,11 @@ protected function getQuotedColumnInIndexSQL()
protected function getQuotedColumnInForeignKeySQL() protected function getQuotedColumnInForeignKeySQL()
{ {
return array( 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)',
); );
} }
} }

0 comments on commit 1af95fc

Please sign in to comment.