Skip to content

Commit 0fc75bb

Browse files
morozovmorozov
committed
Add deprecations for current parser behavior
1 parent 0bf32c5 commit 0fc75bb

File tree

3 files changed

+241
-4
lines changed

3 files changed

+241
-4
lines changed

‎UPGRADE.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,28 @@ awareness about deprecated code.
88

99
# Upgrade to 4.3
1010

11+
## Deprecated relying on the current implementation of the database object name parser
12+
13+
The current object name parser implicitly quotes identifiers in the following cases:
14+
15+
1. If the object name is a reserved keyword (e.g., `select`).
16+
2. If an unquoted identifier is preceded by a quoted identifier (e.g., `"inventory".product`).
17+
18+
As a result, the original case of such identifiers is preserved on platforms that respect the SQL-92 standard (i.e.,
19+
identifiers are not upper-cased on Oracle and IBM DB2, and not lower-cased on PostgreSQL). This behavior is deprecated.
20+
21+
If preserving the original case of an identifier is required, please explicitly quote it (e.g., `select` → `"select"`).
22+
23+
Additionally, the current parser exhibits the following defects:
24+
25+
1. It ignores a missing closing quote in a quoted identifier (e.g., `"inventory`).
26+
2. It allows names with more than two identifiers (e.g., `warehouse.inventory.product`) but only uses the first two,
27+
ignoring the remaining ones.
28+
3. If a quoted identifier contains a dot, it incorrectly treats the part before the dot as a qualifier, despite the
29+
identifier being quoted.
30+
31+
Relying on the above behaviors is deprecated.
32+
1133
## Deprecated `AbstractPlatform::quoteIdentifier()` and `Connection::quoteIdentifier()`
1234

1335
The `AbstractPlatform::quoteIdentifier()` and `Connection::quoteIdentifier()` methods have been deprecated.

‎src/Schema/AbstractAsset.php

Lines changed: 126 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,17 @@
55
namespace Doctrine\DBAL\Schema;
66

77
use Doctrine\DBAL\Platforms\AbstractPlatform;
8+
use Doctrine\DBAL\Schema\Name\Parser;
9+
use Doctrine\DBAL\Schema\Name\Parser\Identifier;
10+
use Doctrine\Deprecations\Deprecation;
811

912
use function array_map;
13+
use function count;
1014
use function crc32;
1115
use function dechex;
1216
use function explode;
1317
use function implode;
18+
use function sprintf;
1419
use function str_contains;
1520
use function str_replace;
1621
use function strtolower;
@@ -35,11 +40,18 @@ abstract class AbstractAsset
3540

3641
protected bool $_quoted = false;
3742

43+
/** @var list<Identifier> */
44+
private array $identifiers = [];
45+
46+
private bool $validateFuture = false;
47+
3848
/**
3949
* Sets the name of this asset.
4050
*/
4151
protected function _setName(string $name): void
4252
{
53+
$input = $name;
54+
4355
if ($this->isIdentifierQuoted($name)) {
4456
$this->_quoted = true;
4557
$name = $this->trimQuotes($name);
@@ -52,6 +64,81 @@ protected function _setName(string $name): void
5264
}
5365

5466
$this->_name = $name;
67+
68+
$this->validateFuture = false;
69+
70+
if ($input !== '') {
71+
$parser = new Parser();
72+
73+
try {
74+
$identifiers = $parser->parse($input);
75+
} catch (Parser\Exception $e) {
76+
Deprecation::trigger(
77+
'doctrine/dbal',
78+
'https://github.com/doctrine/dbal/pull/6592',
79+
'Unable to parse object name: %s.',
80+
$e->getMessage(),
81+
);
82+
83+
return;
84+
}
85+
} else {
86+
$identifiers = [];
87+
}
88+
89+
switch (count($identifiers)) {
90+
case 0:
91+
$this->identifiers = [];
92+
93+
return;
94+
case 1:
95+
$namespace = null;
96+
$name = $identifiers[0];
97+
break;
98+
99+
case 2:
100+
/** @psalm-suppress PossiblyUndefinedArrayOffset */
101+
[$namespace, $name] = $identifiers;
102+
break;
103+
104+
default:
105+
Deprecation::trigger(
106+
'doctrine/dbal',
107+
'https://github.com/doctrine/dbal/pull/6592',
108+
'An object name may consist of at most 2 identifiers (<namespace>.<name>), %d given.',
109+
count($identifiers),
110+
);
111+
112+
return;
113+
}
114+
115+
$this->identifiers = $identifiers;
116+
$this->validateFuture = true;
117+
118+
$futureName = $name->getValue();
119+
$futureNamespace = $namespace?->getValue();
120+
121+
if ($this->_name !== $futureName) {
122+
Deprecation::trigger(
123+
'doctrine/dbal',
124+
'https://github.com/doctrine/dbal/pull/6592',
125+
'Instead of "%s", this name will be interpreted as "%s" in 5.0',
126+
$this->_name,
127+
$futureName,
128+
);
129+
}
130+
131+
if ($this->_namespace === $futureNamespace) {
132+
return;
133+
}
134+
135+
Deprecation::trigger(
136+
'doctrine/dbal',
137+
'https://github.com/doctrine/dbal/pull/6592',
138+
'Instead of %s, the namespace in this name will be interpreted as %s in 5.0.',
139+
$this->_namespace !== null ? sprintf('"%s"', $this->_namespace) : 'null',
140+
$futureNamespace !== null ? sprintf('"%s"', $futureNamespace) : 'null',
141+
);
55142
}
56143

57144
/**
@@ -129,12 +216,47 @@ public function getName(): string
129216
public function getQuotedName(AbstractPlatform $platform): string
130217
{
131218
$keywords = $platform->getReservedKeywordsList();
132-
$parts = explode('.', $this->getName());
133-
foreach ($parts as $k => $v) {
134-
$parts[$k] = $this->_quoted || $keywords->isKeyword($v) ? $platform->quoteSingleIdentifier($v) : $v;
219+
$parts = $normalizedParts = [];
220+
221+
foreach (explode('.', $this->getName()) as $identifier) {
222+
$isQuoted = $this->_quoted || $keywords->isKeyword($identifier);
223+
224+
if (! $isQuoted) {
225+
$parts[] = $identifier;
226+
$normalizedParts[] = $platform->normalizeUnquotedIdentifier($identifier);
227+
} else {
228+
$parts[] = $platform->quoteSingleIdentifier($identifier);
229+
$normalizedParts[] = $identifier;
230+
}
231+
}
232+
233+
$name = implode('.', $parts);
234+
235+
if ($this->validateFuture) {
236+
$futureParts = array_map(static function (Identifier $identifier) use ($platform): string {
237+
$value = $identifier->getValue();
238+
239+
if (! $identifier->isQuoted()) {
240+
$value = $platform->normalizeUnquotedIdentifier($value);
241+
}
242+
243+
return $value;
244+
}, $this->identifiers);
245+
246+
if ($normalizedParts !== $futureParts) {
247+
Deprecation::trigger(
248+
'doctrine/dbal',
249+
'https://github.com/doctrine/dbal/pull/6592',
250+
'Relying on implicitly quoted identifiers preserving their original case is deprecated. '
251+
. 'The current name %s will become %s in 5.0. '
252+
. 'Please quote the name if the case needs to be preserved.',
253+
$name,
254+
implode('.', array_map([$platform, 'quoteSingleIdentifier'], $futureParts)),
255+
);
256+
}
135257
}
136258

137-
return implode('.', $parts);
259+
return $name;
138260
}
139261

140262
/**

‎tests/Schema/AbstractAssetTest.php

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\DBAL\Tests\Schema;
6+
7+
use Doctrine\DBAL\Platforms\AbstractPlatform;
8+
use Doctrine\DBAL\Platforms\MySQLPlatform;
9+
use Doctrine\DBAL\Platforms\OraclePlatform;
10+
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
11+
use Doctrine\DBAL\Schema\Identifier;
12+
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
13+
use PHPUnit\Framework\Attributes\DataProvider;
14+
use PHPUnit\Framework\TestCase;
15+
16+
class AbstractAssetTest extends TestCase
17+
{
18+
use VerifyDeprecations;
19+
20+
#[DataProvider('nameParsingDeprecationProvider')]
21+
public function testNameParsingDeprecation(string $name, AbstractPlatform $platform): void
22+
{
23+
$this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/XXXX');
24+
25+
$identifier = new Identifier($name);
26+
$identifier->getQuotedName($platform);
27+
}
28+
29+
/** @return iterable<array{string, AbstractPlatform}> */
30+
public static function nameParsingDeprecationProvider(): iterable
31+
{
32+
return [
33+
// unquoted keywords not in normal case
34+
['select', new OraclePlatform()],
35+
['SELECT', new PostgreSQLPlatform()],
36+
37+
// unquoted name not in normal case qualified by quoted name
38+
['"_".id', new OraclePlatform()],
39+
['"_".ID', new PostgreSQLPlatform()],
40+
41+
// name with more than one qualifier
42+
['i.am.overqualified', new MySQLPlatform()],
43+
44+
// parse error
45+
['table.', new MySQLPlatform()],
46+
['"table', new MySQLPlatform()],
47+
['table"', new MySQLPlatform()],
48+
[' ', new MySQLPlatform()],
49+
50+
// incompatible parser behavior
51+
['"example.com"', new MySQLPlatform()],
52+
];
53+
}
54+
55+
#[DataProvider('noNameParsingDeprecationProvider')]
56+
public function testNoNameParsingDeprecation(string $name, AbstractPlatform $platform): void
57+
{
58+
$identifier = new Identifier($name);
59+
60+
$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/XXXX');
61+
$identifier->getQuotedName($platform);
62+
}
63+
64+
/** @return iterable<array{string, AbstractPlatform}> */
65+
public static function noNameParsingDeprecationProvider(): iterable
66+
{
67+
return [
68+
// empty name
69+
['', new MySQLPlatform()],
70+
71+
// name with one qualifier
72+
['schema.table', new MySQLPlatform()],
73+
74+
// quoted keywords
75+
['"select"', new OraclePlatform()],
76+
['"SELECT"', new PostgreSQLPlatform()],
77+
78+
// unquoted keywords in normal case
79+
['SELECT', new OraclePlatform()],
80+
['select', new PostgreSQLPlatform()],
81+
82+
// unquoted keywords in any case on a platform that does not force a case
83+
['SELECT', new MySQLPlatform()],
84+
['select', new MySQLPlatform()],
85+
86+
// non-keywords in any case
87+
['id', new OraclePlatform()],
88+
['ID', new OraclePlatform()],
89+
['id', new PostgreSQLPlatform()],
90+
['ID', new PostgreSQLPlatform()],
91+
];
92+
}
93+
}

0 commit comments

Comments
 (0)