From 0fc75bb0cdc0488756d55808b31cfa06e4ab401e Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Mon, 11 Nov 2024 14:17:14 -0800 Subject: [PATCH] Add deprecations for current parser behavior --- UPGRADE.md | 22 +++++ src/Schema/AbstractAsset.php | 130 ++++++++++++++++++++++++++++- tests/Schema/AbstractAssetTest.php | 93 +++++++++++++++++++++ 3 files changed, 241 insertions(+), 4 deletions(-) create mode 100644 tests/Schema/AbstractAssetTest.php 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/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/tests/Schema/AbstractAssetTest.php b/tests/Schema/AbstractAssetTest.php new file mode 100644 index 0000000000..5c8ba5290d --- /dev/null +++ b/tests/Schema/AbstractAssetTest.php @@ -0,0 +1,93 @@ +expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/XXXX'); + + $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()], + ]; + } +}