Skip to content

Commit

Permalink
fix: quote namespace names
Browse files Browse the repository at this point in the history
  • Loading branch information
nikophil committed Dec 18, 2024
1 parent c91166a commit 9c540bc
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 5 deletions.
13 changes: 13 additions & 0 deletions src/Platforms/AbstractPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
use function key;
use function max;
use function mb_strlen;
use function preg_match;
use function preg_quote;
use function preg_replace;
use function sprintf;
Expand Down Expand Up @@ -1038,6 +1039,10 @@ public function getAlterSchemaSQL(SchemaDiff $diff): array
foreach ($diff->getCreatedSchemas() as $schema) {
$sql[] = $this->getCreateSchemaSQL($schema);
}

foreach ($diff->getDroppedSchemas() as $schema) {
$sql[] = $this->getDropSchemaSQL($schema);
}
}

if ($this->supportsSequences()) {
Expand Down Expand Up @@ -1162,6 +1167,10 @@ public function getCreateSchemaSQL(string $schemaName): string
throw NotSupported::new(__METHOD__);
}

if (preg_match('/^\d/', $schemaName)) {

Check failure on line 1170 in src/Platforms/AbstractPlatform.php

View workflow job for this annotation

GitHub Actions / Static Analysis with PHPStan (8.3)

Only booleans are allowed in an if condition, int|false given.
return 'CREATE SCHEMA ' . $this->quoteIdentifier($schemaName);
}

return 'CREATE SCHEMA ' . $schemaName;
}

Expand All @@ -1183,6 +1192,10 @@ public function getDropSchemaSQL(string $schemaName): string
throw NotSupported::new(__METHOD__);
}

if (preg_match('/^\d/', $schemaName)) {

Check failure on line 1195 in src/Platforms/AbstractPlatform.php

View workflow job for this annotation

GitHub Actions / Static Analysis with PHPStan (8.3)

Only booleans are allowed in an if condition, int|false given.
return 'DROP SCHEMA ' . $this->quoteIdentifier($schemaName);
}

return 'DROP SCHEMA ' . $schemaName;
}

Expand Down
5 changes: 4 additions & 1 deletion src/Schema/AbstractAsset.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use function dechex;
use function explode;
use function implode;
use function preg_match;
use function str_contains;
use function str_replace;
use function strtolower;
Expand Down Expand Up @@ -131,7 +132,9 @@ public function getQuotedName(AbstractPlatform $platform): string
$keywords = $platform->getReservedKeywordsList();
$parts = explode('.', $this->getName());
foreach ($parts as $k => $v) {
$parts[$k] = $this->_quoted || $keywords->isKeyword($v) ? $platform->quoteIdentifier($v) : $v;
$shouldBeQuoted = $this->_quoted || $keywords->isKeyword($v) || preg_match('/^\d/', $v);

Check failure on line 135 in src/Schema/AbstractAsset.php

View workflow job for this annotation

GitHub Actions / Static Analysis with PHPStan (8.3)

Only booleans are allowed in ||, int|false given on the right side.

$parts[$k] = $shouldBeQuoted ? $platform->quoteIdentifier($v) : $v;
}

return implode('.', $parts);
Expand Down
19 changes: 19 additions & 0 deletions src/Schema/Exception/NamespaceDoesNotExist.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Schema\Exception;

use Doctrine\DBAL\Schema\SchemaException;
use LogicException;

use function sprintf;

/** @psalm-immutable */
final class NamespaceDoesNotExist extends LogicException implements SchemaException
{
public static function new(string $namespaceName): self
{
return new self(sprintf('There is no namespace with name "%s" in the schema.', $namespaceName));
}
}
19 changes: 19 additions & 0 deletions src/Schema/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Schema\Exception\NamespaceAlreadyExists;
use Doctrine\DBAL\Schema\Exception\NamespaceDoesNotExist;
use Doctrine\DBAL\Schema\Exception\SequenceAlreadyExists;
use Doctrine\DBAL\Schema\Exception\SequenceDoesNotExist;
use Doctrine\DBAL\Schema\Exception\TableAlreadyExists;
Expand Down Expand Up @@ -267,6 +268,24 @@ public function createNamespace(string $name): self
return $this;
}

/**
* Drops a new namespace from the schema.
*
* @return $this
*/
public function dropNamespace(string $name): self
{
$unquotedName = strtolower($this->getUnquotedAssetName($name));

if (! isset($this->namespaces[$unquotedName])) {
throw NamespaceDoesNotExist::new($unquotedName);
}

unset($this->namespaces[$unquotedName]);

return $this;
}

/**
* Creates a new table.
*/
Expand Down
6 changes: 2 additions & 4 deletions tests/Functional/Schema/PostgreSQLSchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,10 @@ protected function assertVarBinaryColumnIsValid(Table $table, string $columnName
/**
* Although this test would pass in isolation on any platform, we keep it here for the following reasons:
*
* 1. The DBAL currently doesn't properly drop tables in the namespaces that need to be quoted
* (@see testListTableDetailsWhenCurrentSchemaNameQuoted()).
* 2. The schema returned by {@see AbstractSchemaManager::introspectSchema()} doesn't contain views, so
* 1. The schema returned by {@see AbstractSchemaManager::introspectSchema()} doesn't contain views, so
* {@see AbstractSchemaManager::dropSchemaObjects()} cannot drop the tables that have dependent views
* (@see testListTablesExcludesViews()).
* 3. In the case of inheritance, PHPUnit runs the tests declared immediately in the test class
* 2. In the case of inheritance, PHPUnit runs the tests declared immediately in the test class
* and then runs the tests declared in the parent.
*
* This test needs to be executed before the ones it conflicts with, so it has to be declared in the same class.
Expand Down
47 changes: 47 additions & 0 deletions tests/Functional/Schema/SchemaManagerFunctionalTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -1419,6 +1419,53 @@ public function testDropColumnWithDefault(): void
self::assertCount(1, $columns);
}

public function testCanCreateAndDropSchemaThatNeedToBeQuoted(): void
{
$platform = $this->connection->getDatabasePlatform();

if (! $platform->supportsSchemas()) {
self::markTestSkipped('The platform does not support schema/namespaces.');
}

$schemaManager = $this->connection->createSchemaManager();
$schema = $schemaManager->introspectSchema();

$schema->createNamespace('001_schema');

$schemaManager = $this->connection->createSchemaManager();
$schemaManager->migrateSchema($schema);
self::assertContains('001_schema', $schemaManager->listSchemaNames());

$schema->dropNamespace('001_schema');
$schemaManager->migrateSchema($schema);
self::assertNotContains('001_schema', $schemaManager->listSchemaNames());
}

public function testCanCreateAndDropTableFromNamespaceThatNeedToBeQuoted(): void
{
$platform = $this->connection->getDatabasePlatform();

if (! $platform->supportsSchemas()) {
self::markTestSkipped('The platform does not support schema/namespaces.');
}

$schemaManager = $this->connection->createSchemaManager();
$schema = $schemaManager->introspectSchema();

$table = $schema->createTable('001_schema.test_quoted_schema');
$table->addColumn('id', Types::INTEGER, ['notnull' => true]);
$table->setPrimaryKey(['id']);

$schemaManager = $this->connection->createSchemaManager();
$schemaManager->migrateSchema($schema);
self::assertContains('001_schema', $schemaManager->listSchemaNames());
self::assertContains('001_schema.test_quoted_schema', $schemaManager->listTableNames());

$schema->dropTable('001_schema.test_quoted_schema');
$schemaManager->migrateSchema($schema);
self::assertNotContains('001_schema.test_quoted_schema', $schemaManager->listTableNames());
}

/** @param list<Table> $tables */
protected function findTableByName(array $tables, string $name): ?Table
{
Expand Down

0 comments on commit 9c540bc

Please sign in to comment.