-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: quote namespace names #6652
base: 4.2.x
Are you sure you want to change the base?
Conversation
src/Platforms/AbstractPlatform.php
Outdated
if (preg_match('/^\d/', $schemaName)) { | ||
return 'CREATE SCHEMA ' . $this->quoteIdentifier($schemaName); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at first, I was thinking of create an asset Doctrine\DBAL\Schema\Namespace
but namespace are handled in a different way than other assets, and it seemed easier to do it like this, but I still can create the new class
9c540bc
to
eaa408a
Compare
eaa408a
to
f8a686d
Compare
@@ -1162,6 +1167,10 @@ public function getCreateSchemaSQL(string $schemaName): string | |||
throw NotSupported::new(__METHOD__); | |||
} | |||
|
|||
if (preg_match('/^\d/', $schemaName) > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though it's not consistently documented, all get[...]SQL()
methods of this class accept SQL fragments as string
parameters and just concatenate them with other fragments of SQL. Treating the schema name as a literal is against this implied contract. It's the caller of the method who should quote the names that need to be quoted.
Depending on how you manage your schema via DBAL, you may try explicitly quoting names in the schema definition similar to this:
dbal/tests/Functional/Schema/OracleSchemaManagerTest.php
Lines 251 to 255 in 95d093a
public function testQuotedTableNameRemainsQuotedInSchema(): void | |
{ | |
$table = new Table('"tester"'); | |
$table->addColumn('"id"', Types::INTEGER); | |
$table->addColumn('"name"', Types::STRING, ['length' => 32]); |
This won't be necessary in DBAL 5.0 (see #6589).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Within the current API, a proper solution would be to assume all object names obtained by introspecting the database quoted, but schema names are returned as strings, so there's no way to indicate that besides wrapping all names in quotes. Doing that will fix this bug but will be a breaking change, so we can do that only in a major release.
Follow up of this discussion
ping @morozov