Skip to content

Commit

Permalink
Merge pull request #6756 from morozov/auto-quote-postgres-identifiers
Browse files Browse the repository at this point in the history 
Auto-quote introspected PostgreSQL identifiers
  • Loading branch information
@morozov
morozov authored Feb 2, 2025
2 parents ec16c82 + 62ae320 commit da19422
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 121 deletions.
17 changes: 12 additions & 5 deletions src/Schema/PostgreSQLSchemaManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,16 @@ protected function _getPortableTableIndexesList($tableIndexes, $tableName = null
foreach ($tableIndexes as $row) {
$colNumbers = array_map('intval', explode(' ', $row['indkey']));
$columnNameSql = sprintf(
'SELECT attnum, attname FROM pg_attribute WHERE attrelid=%d AND attnum IN (%s) ORDER BY attnum ASC',
<<<'SQL'
SELECT attnum,
quote_ident(attname) AS attname
FROM pg_attribute
WHERE attrelid = %d
AND attnum IN (%s)
ORDER BY attnum
SQL,
$row['indrelid'],
implode(' ,', $colNumbers),
implode(', ', $colNumbers),
);

$indexColumns = $this->_conn->fetchAllAssociative($columnNameSql);
Expand Down Expand Up @@ -612,7 +619,7 @@ protected function selectTableColumns(string $databaseName, ?string $tableName =
$sql = 'SELECT';

if ($tableName === null) {
$sql .= ' c.relname AS table_name, n.nspname AS schema_name,';
$sql .= ' quote_ident(c.relname) AS table_name, quote_ident(n.nspname) AS schema_name,';
}

$sql .= sprintf(<<<'SQL'
Expand Down Expand Up @@ -664,7 +671,7 @@ protected function selectIndexColumns(string $databaseName, ?string $tableName =
$sql = 'SELECT';

if ($tableName === null) {
$sql .= ' tc.relname AS table_name, tn.nspname AS schema_name,';
$sql .= ' quote_ident(tc.relname) AS table_name, quote_ident(tn.nspname) AS schema_name,';
}

$sql .= <<<'SQL'
Expand Down Expand Up @@ -698,7 +705,7 @@ protected function selectForeignKeyColumns(string $databaseName, ?string $tableN
$sql = 'SELECT';

if ($tableName === null) {
$sql .= ' tc.relname AS table_name, tn.nspname AS schema_name,';
$sql .= ' quote_ident(tc.relname) AS table_name, quote_ident(tn.nspname) AS schema_name,';
}

$sql .= <<<'SQL'
Expand Down
116 changes: 0 additions & 116 deletions tests/Functional/Schema/OracleSchemaManagerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,122 +71,6 @@ public function testAlterTableColumnNotNull(callable $comparatorFactory): void
self::assertTrue($columns['bar']->getNotnull());
}

public function testListTableDetailsWithDifferentIdentifierQuotingRequirements(): void
{
$primaryTableName = '"Primary_Table"';
$offlinePrimaryTable = new Table($primaryTableName);
$offlinePrimaryTable->addColumn(
'"Id"',
Types::INTEGER,
['autoincrement' => true, 'comment' => 'Explicit casing.'],
);
$offlinePrimaryTable->addColumn('select', Types::INTEGER, ['comment' => 'Reserved keyword.']);
$offlinePrimaryTable->addColumn('foo', Types::INTEGER, ['comment' => 'Implicit uppercasing.']);
$offlinePrimaryTable->addColumn('BAR', Types::INTEGER);
$offlinePrimaryTable->addColumn('"BAZ"', Types::INTEGER);
$offlinePrimaryTable->addIndex(['select'], 'from');
$offlinePrimaryTable->addIndex(['foo'], 'foo_index');
$offlinePrimaryTable->addIndex(['BAR'], 'BAR_INDEX');
$offlinePrimaryTable->addIndex(['"BAZ"'], 'BAZ_INDEX');
$offlinePrimaryTable->setPrimaryKey(['"Id"']);

$foreignTableName = 'foreign';
$offlineForeignTable = new Table($foreignTableName);
$offlineForeignTable->addColumn('id', Types::INTEGER, ['autoincrement' => true]);
$offlineForeignTable->addColumn('"Fk"', Types::INTEGER);
$offlineForeignTable->addIndex(['"Fk"'], '"Fk_index"');
$offlineForeignTable->addForeignKeyConstraint(
$primaryTableName,
['"Fk"'],
['"Id"'],
[],
'"Primary_Table_Fk"',
);
$offlineForeignTable->setPrimaryKey(['id']);

$this->dropTableIfExists($foreignTableName);
$this->dropTableIfExists($primaryTableName);

$this->schemaManager->createTable($offlinePrimaryTable);
$this->schemaManager->createTable($offlineForeignTable);

$onlinePrimaryTable = $this->schemaManager->introspectTable($primaryTableName);
$onlineForeignTable = $this->schemaManager->introspectTable($foreignTableName);

$platform = $this->connection->getDatabasePlatform();

// Primary table assertions
self::assertSame($primaryTableName, $onlinePrimaryTable->getQuotedName($platform));

self::assertTrue($onlinePrimaryTable->hasColumn('"Id"'));
self::assertSame('"Id"', $onlinePrimaryTable->getColumn('"Id"')->getQuotedName($platform));
self::assertTrue($onlinePrimaryTable->hasPrimaryKey());

$primaryKey = $onlinePrimaryTable->getPrimaryKey();

self::assertNotNull($primaryKey);
self::assertSame(['"Id"'], $primaryKey->getQuotedColumns($platform));

self::assertTrue($onlinePrimaryTable->hasColumn('select'));
self::assertSame('"select"', $onlinePrimaryTable->getColumn('select')->getQuotedName($platform));

self::assertTrue($onlinePrimaryTable->hasColumn('foo'));
self::assertSame('FOO', $onlinePrimaryTable->getColumn('foo')->getQuotedName($platform));

self::assertTrue($onlinePrimaryTable->hasColumn('BAR'));
self::assertSame('BAR', $onlinePrimaryTable->getColumn('BAR')->getQuotedName($platform));

self::assertTrue($onlinePrimaryTable->hasColumn('"BAZ"'));
self::assertSame('BAZ', $onlinePrimaryTable->getColumn('"BAZ"')->getQuotedName($platform));

self::assertTrue($onlinePrimaryTable->hasIndex('from'));
self::assertTrue($onlinePrimaryTable->getIndex('from')->hasColumnAtPosition('"select"'));
self::assertSame(['"select"'], $onlinePrimaryTable->getIndex('from')->getQuotedColumns($platform));

self::assertTrue($onlinePrimaryTable->hasIndex('foo_index'));
self::assertTrue($onlinePrimaryTable->getIndex('foo_index')->hasColumnAtPosition('foo'));
self::assertSame(['FOO'], $onlinePrimaryTable->getIndex('foo_index')->getQuotedColumns($platform));

self::assertTrue($onlinePrimaryTable->hasIndex('BAR_INDEX'));
self::assertTrue($onlinePrimaryTable->getIndex('BAR_INDEX')->hasColumnAtPosition('BAR'));
self::assertSame(['BAR'], $onlinePrimaryTable->getIndex('BAR_INDEX')->getQuotedColumns($platform));

self::assertTrue($onlinePrimaryTable->hasIndex('BAZ_INDEX'));
self::assertTrue($onlinePrimaryTable->getIndex('BAZ_INDEX')->hasColumnAtPosition('"BAZ"'));
self::assertSame(['BAZ'], $onlinePrimaryTable->getIndex('BAZ_INDEX')->getQuotedColumns($platform));

// Foreign table assertions
self::assertTrue($onlineForeignTable->hasColumn('id'));
self::assertSame('ID', $onlineForeignTable->getColumn('id')->getQuotedName($platform));
self::assertTrue($onlineForeignTable->hasPrimaryKey());

$primaryKey = $onlineForeignTable->getPrimaryKey();

self::assertNotNull($primaryKey);
self::assertSame(['ID'], $primaryKey->getQuotedColumns($platform));

self::assertTrue($onlineForeignTable->hasColumn('"Fk"'));
self::assertSame('"Fk"', $onlineForeignTable->getColumn('"Fk"')->getQuotedName($platform));

self::assertTrue($onlineForeignTable->hasIndex('"Fk_index"'));
self::assertTrue($onlineForeignTable->getIndex('"Fk_index"')->hasColumnAtPosition('"Fk"'));
self::assertSame(['"Fk"'], $onlineForeignTable->getIndex('"Fk_index"')->getQuotedColumns($platform));

self::assertTrue($onlineForeignTable->hasForeignKey('"Primary_Table_Fk"'));
self::assertSame(
$primaryTableName,
$onlineForeignTable->getForeignKey('"Primary_Table_Fk"')->getQuotedForeignTableName($platform),
);
self::assertSame(
['"Fk"'],
$onlineForeignTable->getForeignKey('"Primary_Table_Fk"')->getQuotedLocalColumns($platform),
);
self::assertSame(
['"Id"'],
$onlineForeignTable->getForeignKey('"Primary_Table_Fk"')->getQuotedForeignColumns($platform),
);
}

public function testListTableColumnsSameTableNamesInDifferentSchemas(): void
{
$table = $this->createListTableColumns();
Expand Down
92 changes: 92 additions & 0 deletions tests/Functional/Schema/SchemaManagerFunctionalTestCase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Exception\DatabaseObjectNotFoundException;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\DB2Platform;
use Doctrine\DBAL\Platforms\OraclePlatform;
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
use Doctrine\DBAL\Platforms\SqlitePlatform;
use Doctrine\DBAL\Platforms\SQLServerPlatform;
use Doctrine\DBAL\Schema\AbstractAsset;
Expand Down Expand Up @@ -1716,6 +1718,96 @@ private function createReservedKeywordTables(): void
$schemaManager->createSchemaObjects($schema);
}

/** @throws Exception */
public function testQuotedIdentifiers(): void
{
$platform = $this->connection->getDatabasePlatform();

if ($platform instanceof DB2Platform) {
self::markTestIncomplete(
'Introspection of lower-case identifiers as quoted is currently not implemented on IBM DB2.',
);
}

if (! $platform instanceof OraclePlatform && ! $platform instanceof PostgreSQLPlatform) {
self::markTestSkipped('The current platform does not auto-quote introspected identifiers.');
}

$artists = new Table('"Artists"');
$artists->addColumn('"Id"', Types::INTEGER);
$artists->addColumn('"Name"', Types::INTEGER);
$artists->addIndex(['"Name"'], '"Idx_Name"');
$artists->setPrimaryKey(['"Id"']);

$tracks = new Table('"Tracks"');
$tracks->addColumn('"Id"', Types::INTEGER);
$tracks->addColumn('"Artist_Id"', Types::INTEGER);
$tracks->addIndex(['"Artist_Id"'], '"Idx_Artist_Id"');
$tracks->addForeignKeyConstraint(
'"Artists"',
['"Artist_Id"'],
['"Id"'],
[],
'"Artists_Fk"',
);
$tracks->setPrimaryKey(['"Id"']);

$this->dropTableIfExists('"Tracks"');
$this->dropTableIfExists('"Artists"');

$this->schemaManager->createTable($artists);
$this->schemaManager->createTable($tracks);

$artists = $this->schemaManager->introspectTable('"Artists"');
$tracks = $this->schemaManager->introspectTable('"Tracks"');

$platform = $this->connection->getDatabasePlatform();

// Primary table assertions
self::assertSame('"Artists"', $artists->getQuotedName($platform));
self::assertSame('"Id"', $artists->getColumn('"Id"')->getQuotedName($platform));
self::assertSame('"Name"', $artists->getColumn('"Name"')->getQuotedName($platform));
self::assertSame(['"Name"'], $artists->getIndex('"Idx_Name"')->getQuotedColumns($platform));

$primaryKey = $artists->getPrimaryKey();
self::assertNotNull($primaryKey);
self::assertSame(['"Id"'], $primaryKey->getQuotedColumns($platform));

// Foreign table assertions
self::assertTrue($tracks->hasColumn('"Id"'));
self::assertSame('"Id"', $tracks->getColumn('"Id"')->getQuotedName($platform));

$primaryKey = $tracks->getPrimaryKey();
self::assertNotNull($primaryKey);
self::assertSame(['"Id"'], $primaryKey->getQuotedColumns($platform));

self::assertTrue($tracks->hasColumn('"Artist_Id"'));
self::assertSame(
'"Artist_Id"',
$tracks->getColumn('"Artist_Id"')->getQuotedName($platform),
);

self::assertTrue($tracks->hasIndex('"Idx_Artist_Id"'));
self::assertSame(
['"Artist_Id"'],
$tracks->getIndex('"Idx_Artist_Id"')->getQuotedColumns($platform),
);

self::assertTrue($tracks->hasForeignKey('"Artists_Fk"'));
self::assertSame(
'"Artists"',
$tracks->getForeignKey('"Artists_Fk"')->getQuotedForeignTableName($platform),
);
self::assertSame(
['"Artist_Id"'],
$tracks->getForeignKey('"Artists_Fk"')->getQuotedLocalColumns($platform),
);
self::assertSame(
['"Id"'],
$tracks->getForeignKey('"Artists_Fk"')->getQuotedForeignColumns($platform),
);
}

public function testChangeIndexWithForeignKeys(): void
{
$this->dropTableIfExists('child');
Expand Down

0 comments on commit da19422

Please sign in to comment.