diff --git a/UPGRADE.md b/UPGRADE.md index b3632793ae..5559b7a932 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,28 @@ awareness about deprecated code. # Upgrade to 4.3 +## Deprecated relying on the current implementation of the database object name parser + +The current object name parser implicitly quotes identifiers in the following cases: + +1. If the object name is a reserved keyword (e.g., `select`). +2. If an unquoted identifier is preceded by a quoted identifier (e.g., `"inventory".product`). + +As a result, the original case of such identifiers is preserved on platforms that respect the SQL-92 standard (i.e., +identifiers are not upper-cased on Oracle and IBM DB2, and not lower-cased on PostgreSQL). This behavior is deprecated. + +If preserving the original case of an identifier is required, please explicitly quote it (e.g., `select` → `"select"`). + +Additionally, the current parser exhibits the following defects: + +1. It ignores a missing closing quote in a quoted identifier (e.g., `"inventory`). +2. It allows names with more than two identifiers (e.g., `warehouse.inventory.product`) but only uses the first two, + ignoring the remaining ones. +3. If a quoted identifier contains a dot, it incorrectly treats the part before the dot as a qualifier, despite the + identifier being quoted. + +Relying on the above behaviors is deprecated. + ## Deprecated `AbstractPlatform::quoteIdentifier()` and `Connection::quoteIdentifier()` The `AbstractPlatform::quoteIdentifier()` and `Connection::quoteIdentifier()` methods have been deprecated. diff --git a/src/Platforms/AbstractMySQLPlatform.php b/src/Platforms/AbstractMySQLPlatform.php index 6a1d6326f0..ee7c10d9dc 100644 --- a/src/Platforms/AbstractMySQLPlatform.php +++ b/src/Platforms/AbstractMySQLPlatform.php @@ -875,4 +875,9 @@ public function fetchTableOptionsByTable(bool $includeTableName): string return $sql . ' WHERE ' . implode(' AND ', $conditions); } + + public function normalizeUnquotedIdentifier(string $identifier): string + { + return $identifier; + } } diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 1014f8f52e..66b249abb2 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -2293,6 +2293,18 @@ public function getUnionDistinctSQL(): string return 'UNION'; } + /** + * Changes the case of unquoted identifier in the same way as the given platform would change it if it was specified + * in an SQL statement. + * + * Even though the default behavior is not the most common across supported platforms, it is part of the SQL92 + * standard. + */ + public function normalizeUnquotedIdentifier(string $identifier): string + { + return strtoupper($identifier); + } + /** * Creates the schema manager that can be used to inspect and change the underlying * database schema according to the dialect of the platform. diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php index 0af29f674a..edbe374de9 100644 --- a/src/Platforms/PostgreSQLPlatform.php +++ b/src/Platforms/PostgreSQLPlatform.php @@ -786,4 +786,9 @@ public function createSchemaManager(Connection $connection): PostgreSQLSchemaMan { return new PostgreSQLSchemaManager($connection, $this); } + + public function normalizeUnquotedIdentifier(string $identifier): string + { + return strtolower($identifier); + } } diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php index c03cf6efc3..bb5adaf4dd 100644 --- a/src/Platforms/SQLServerPlatform.php +++ b/src/Platforms/SQLServerPlatform.php @@ -1241,4 +1241,9 @@ public function createSchemaManager(Connection $connection): SQLServerSchemaMana { return new SQLServerSchemaManager($connection, $this); } + + public function normalizeUnquotedIdentifier(string $identifier): string + { + return $identifier; + } } diff --git a/src/Platforms/SQLitePlatform.php b/src/Platforms/SQLitePlatform.php index 32f28f28af..e1dafec4aa 100644 --- a/src/Platforms/SQLitePlatform.php +++ b/src/Platforms/SQLitePlatform.php @@ -973,4 +973,9 @@ public function getUnionSelectPartSQL(string $subQuery): string { return $subQuery; } + + public function normalizeUnquotedIdentifier(string $identifier): string + { + return $identifier; + } } diff --git a/src/Schema/AbstractAsset.php b/src/Schema/AbstractAsset.php index 671bddfe80..9e6f273800 100644 --- a/src/Schema/AbstractAsset.php +++ b/src/Schema/AbstractAsset.php @@ -5,12 +5,17 @@ namespace Doctrine\DBAL\Schema; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Schema\Name\Parser; +use Doctrine\DBAL\Schema\Name\Parser\Identifier; +use Doctrine\Deprecations\Deprecation; use function array_map; +use function count; use function crc32; use function dechex; use function explode; use function implode; +use function sprintf; use function str_contains; use function str_replace; use function strtolower; @@ -35,11 +40,18 @@ abstract class AbstractAsset protected bool $_quoted = false; + /** @var list */ + private array $identifiers = []; + + private bool $validateFuture = false; + /** * Sets the name of this asset. */ protected function _setName(string $name): void { + $input = $name; + if ($this->isIdentifierQuoted($name)) { $this->_quoted = true; $name = $this->trimQuotes($name); @@ -52,6 +64,81 @@ protected function _setName(string $name): void } $this->_name = $name; + + $this->validateFuture = false; + + if ($input !== '') { + $parser = new Parser(); + + try { + $identifiers = $parser->parse($input); + } catch (Parser\Exception $e) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6592', + 'Unable to parse object name: %s.', + $e->getMessage(), + ); + + return; + } + } else { + $identifiers = []; + } + + switch (count($identifiers)) { + case 0: + $this->identifiers = []; + + return; + case 1: + $namespace = null; + $name = $identifiers[0]; + break; + + case 2: + /** @psalm-suppress PossiblyUndefinedArrayOffset */ + [$namespace, $name] = $identifiers; + break; + + default: + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6592', + 'An object name may consist of at most 2 identifiers (.), %d given.', + count($identifiers), + ); + + return; + } + + $this->identifiers = $identifiers; + $this->validateFuture = true; + + $futureName = $name->getValue(); + $futureNamespace = $namespace?->getValue(); + + if ($this->_name !== $futureName) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6592', + 'Instead of "%s", this name will be interpreted as "%s" in 5.0', + $this->_name, + $futureName, + ); + } + + if ($this->_namespace === $futureNamespace) { + return; + } + + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6592', + 'Instead of %s, the namespace in this name will be interpreted as %s in 5.0.', + $this->_namespace !== null ? sprintf('"%s"', $this->_namespace) : 'null', + $futureNamespace !== null ? sprintf('"%s"', $futureNamespace) : 'null', + ); } /** @@ -129,12 +216,47 @@ public function getName(): string 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->quoteSingleIdentifier($v) : $v; + $parts = $normalizedParts = []; + + foreach (explode('.', $this->getName()) as $identifier) { + $isQuoted = $this->_quoted || $keywords->isKeyword($identifier); + + if (! $isQuoted) { + $parts[] = $identifier; + $normalizedParts[] = $platform->normalizeUnquotedIdentifier($identifier); + } else { + $parts[] = $platform->quoteSingleIdentifier($identifier); + $normalizedParts[] = $identifier; + } + } + + $name = implode('.', $parts); + + if ($this->validateFuture) { + $futureParts = array_map(static function (Identifier $identifier) use ($platform): string { + $value = $identifier->getValue(); + + if (! $identifier->isQuoted()) { + $value = $platform->normalizeUnquotedIdentifier($value); + } + + return $value; + }, $this->identifiers); + + if ($normalizedParts !== $futureParts) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/6592', + 'Relying on implicitly quoted identifiers preserving their original case is deprecated. ' + . 'The current name %s will become %s in 5.0. ' + . 'Please quote the name if the case needs to be preserved.', + $name, + implode('.', array_map([$platform, 'quoteSingleIdentifier'], $futureParts)), + ); + } } - return implode('.', $parts); + return $name; } /** diff --git a/src/Schema/Name/Parser.php b/src/Schema/Name/Parser.php new file mode 100644 index 0000000000..d2817f30d3 --- /dev/null +++ b/src/Schema/Name/Parser.php @@ -0,0 +1,103 @@ +[^"]*(?:""[^"]*)*)" # ANSI SQL double-quoted + | `(?[^`]*(?:``[^`]*)*)` # MySQL-style backtick-quoted + | \[(?[^]]*(?:]][^]]*)*)] # SQL Server-style square-bracket-quoted + | (?[^\s."`\[\]]+) # Unquoted + ) + /x + PATTERN; + + /** + * @return list + * + * @throws Exception + */ + public function parse(string $input): array + { + if ($input === '') { + return []; + } + + $offset = 0; + $identifiers = []; + $length = strlen($input); + + while (true) { + if ($offset >= $length) { + throw ExpectedNextIdentifier::new(); + } + + if (preg_match(self::IDENTIFIER_PATTERN, $input, $matches, 0, $offset) === 0) { + throw UnableToParseIdentifier::new($offset); + } + + if (isset($matches['ansi']) && strlen($matches['ansi']) > 0) { + $identifier = Identifier::quoted(str_replace('""', '"', $matches['ansi'])); + } elseif (isset($matches['mysql']) && strlen($matches['mysql']) > 0) { + $identifier = Identifier::quoted(str_replace('``', '`', $matches['mysql'])); + } elseif (isset($matches['sqlserver']) && strlen($matches['sqlserver']) > 0) { + $identifier = Identifier::quoted(str_replace(']]', ']', $matches['sqlserver'])); + } else { + assert(isset($matches['unquoted']) && strlen($matches['unquoted']) > 0); + $identifier = Identifier::unquoted($matches['unquoted']); + } + + $identifiers[] = $identifier; + + $offset += strlen($matches[0]); + + if ($offset >= $length) { + break; + } + + $character = $input[$offset]; + + if ($character !== '.') { + throw ExpectedDot::new($offset, $character); + } + + $offset++; + } + + return $identifiers; + } +} diff --git a/src/Schema/Name/Parser/Exception.php b/src/Schema/Name/Parser/Exception.php new file mode 100644 index 0000000000..5c4db04bdf --- /dev/null +++ b/src/Schema/Name/Parser/Exception.php @@ -0,0 +1,11 @@ +value; + } + + public function isQuoted(): bool + { + return $this->isQuoted; + } + + /** + * Creates a quoted identifier. + */ + public static function quoted(string $value): self + { + return new self($value, true); + } + + /** + * Creates an unquoted identifier. + */ + public static function unquoted(string $value): self + { + return new self($value, false); + } +} diff --git a/tests/Schema/AbstractAssetTest.php b/tests/Schema/AbstractAssetTest.php new file mode 100644 index 0000000000..09a405a725 --- /dev/null +++ b/tests/Schema/AbstractAssetTest.php @@ -0,0 +1,93 @@ +expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6592'); + + $identifier = new Identifier($name); + $identifier->getQuotedName($platform); + } + + /** @return iterable */ + public static function nameParsingDeprecationProvider(): iterable + { + return [ + // unquoted keywords not in normal case + ['select', new OraclePlatform()], + ['SELECT', new PostgreSQLPlatform()], + + // unquoted name not in normal case qualified by quoted name + ['"_".id', new OraclePlatform()], + ['"_".ID', new PostgreSQLPlatform()], + + // name with more than one qualifier + ['i.am.overqualified', new MySQLPlatform()], + + // parse error + ['table.', new MySQLPlatform()], + ['"table', new MySQLPlatform()], + ['table"', new MySQLPlatform()], + [' ', new MySQLPlatform()], + + // incompatible parser behavior + ['"example.com"', new MySQLPlatform()], + ]; + } + + #[DataProvider('noNameParsingDeprecationProvider')] + public function testNoNameParsingDeprecation(string $name, AbstractPlatform $platform): void + { + $identifier = new Identifier($name); + + $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/XXXX'); + $identifier->getQuotedName($platform); + } + + /** @return iterable */ + public static function noNameParsingDeprecationProvider(): iterable + { + return [ + // empty name + ['', new MySQLPlatform()], + + // name with one qualifier + ['schema.table', new MySQLPlatform()], + + // quoted keywords + ['"select"', new OraclePlatform()], + ['"SELECT"', new PostgreSQLPlatform()], + + // unquoted keywords in normal case + ['SELECT', new OraclePlatform()], + ['select', new PostgreSQLPlatform()], + + // unquoted keywords in any case on a platform that does not force a case + ['SELECT', new MySQLPlatform()], + ['select', new MySQLPlatform()], + + // non-keywords in any case + ['id', new OraclePlatform()], + ['ID', new OraclePlatform()], + ['id', new PostgreSQLPlatform()], + ['ID', new PostgreSQLPlatform()], + ]; + } +} diff --git a/tests/Schema/Name/ParserTest.php b/tests/Schema/Name/ParserTest.php new file mode 100644 index 0000000000..66e33fa921 --- /dev/null +++ b/tests/Schema/Name/ParserTest.php @@ -0,0 +1,141 @@ +parser = new Parser(); + } + + /** + * @param list $expected + * + * @throws Exception + */ + #[DataProvider('validInputProvider')] + public function testValidInput(string $input, array $expected): void + { + $name = $this->parser->parse($input); + self::assertEquals($expected, $name); + } + + /** @return iterable}> */ + public static function validInputProvider(): iterable + { + yield ['', []]; + yield ['table', [Identifier::unquoted('table')]]; + yield ['schema.table', [Identifier::unquoted('schema'), Identifier::unquoted('table')]]; + + yield ['"example.com"', [Identifier::quoted('example.com')]]; + yield ['`example.com`', [Identifier::quoted('example.com')]]; + yield ['[example.com]', [Identifier::quoted('example.com')]]; + + yield [ + 'a."b".c.`d`.e.[f].g', + [ + Identifier::unquoted('a'), + Identifier::quoted('b'), + Identifier::unquoted('c'), + Identifier::quoted('d'), + Identifier::unquoted('e'), + Identifier::quoted('f'), + Identifier::unquoted('g'), + ], + ]; + + yield ['"schema"."table"', [Identifier::quoted('schema'), Identifier::quoted('table')]]; + yield ['`schema`.`table`', [Identifier::quoted('schema'), Identifier::quoted('table')]]; + yield ['[schema].[table]', [Identifier::quoted('schema'), Identifier::quoted('table')]]; + + yield [ + 'schema."example.com"', + [ + Identifier::unquoted('schema'), + Identifier::quoted('example.com'), + ], + ]; + + yield [ + '"a""b".`c``d`.[e]]f]', + [ + Identifier::quoted('a"b'), + Identifier::quoted('c`d'), + Identifier::quoted('e]f'), + ], + ]; + + yield [ + 'schéma."übermäßigkeit".`àçcênt`.[éxtrême].çhâràctér', + [ + Identifier::unquoted('schéma'), + Identifier::quoted('übermäßigkeit'), + Identifier::quoted('àçcênt'), + Identifier::quoted('éxtrême'), + Identifier::unquoted('çhâràctér'), + ], + ]; + + yield [ + '" spaced identifier ".more', + [ + Identifier::quoted(' spaced identifier '), + Identifier::unquoted('more'), + ], + ]; + + yield [ + '0."0".`0`.[0]', + [ + Identifier::unquoted('0'), + Identifier::quoted('0'), + Identifier::quoted('0'), + Identifier::quoted('0'), + ], + ]; + } + + /** + * @param class-string $expectedException + * + * @throws Exception + */ + #[DataProvider('invalidInputProvider')] + public function testInvalidInput(string $input, string $expectedException): void + { + $this->expectException($expectedException); + $this->parser->parse($input); + } + + /** @return iterable}> */ + public static function invalidInputProvider(): iterable + { + yield ['"example.com', UnableToParseIdentifier::class]; + yield ['`example.com', UnableToParseIdentifier::class]; + yield ['[example.com', UnableToParseIdentifier::class]; + yield ['schema."example.com', UnableToParseIdentifier::class]; + yield ['schema.[example.com', UnableToParseIdentifier::class]; + yield ['schema.`example.com', UnableToParseIdentifier::class]; + + yield ['schema.', ExpectedNextIdentifier::class]; + yield ['schema..', UnableToParseIdentifier::class]; + yield ['.table', UnableToParseIdentifier::class]; + + yield ['schema.table name', ExpectedDot::class]; + yield ['"schema.[example.com]', UnableToParseIdentifier::class]; + } +}