Skip to content

Commit

Permalink
Merge pull request #6701 from morozov/deprecate-incomplete-sqlite-schema
Browse files Browse the repository at this point in the history
Deprecate introspection of incomplete SQLite schema
  • Loading branch information
morozov authored Jan 11, 2025
2 parents 13aaf2a + f07e369 commit 8c9efcf
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 16 deletions.
10 changes: 10 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ awareness about deprecated code.

# Upgrade to 4.3

## Deprecated introspection of SQLite foreign key constraints with omitted referenced column names in an incomplete schema

If the referenced column names are omitted in a foreign key constraint declaration, it implies that the constraint
references the primary key columns of the referenced table. If the referenced table is not present in the schema, the
constraint cannot be properly introspected, and the referenced column names are introspected as an empty list.
This behavior is deprecated.

In order to mitigate this issue, either ensure that the referenced table is present in the schema when introspecting
foreign constraints, or provide the referenced column names explicitly in the constraint declaration.

## Deprecated `UniqueConstraint` methods, property and behavior

The following `UniqueConstraint` methods and property have been deprecated:
Expand Down
30 changes: 22 additions & 8 deletions src/Schema/SQLiteSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Doctrine\DBAL\Types\StringType;
use Doctrine\DBAL\Types\TextType;
use Doctrine\DBAL\Types\Type;
use Doctrine\Deprecations\Deprecation;

use function array_change_key_case;
use function array_map;
Expand Down Expand Up @@ -322,20 +323,33 @@ protected function _getPortableTableForeignKeysList(array $tableForeignKeys): ar
$list[$id]['local'][] = $value['from'];

if ($value['to'] === null) {
// Inferring a shorthand form for the foreign key constraint, where the "to" field is empty.
// @see https://www.sqlite.org/foreignkeys.html#fk_indexes.
$foreignTableIndexes = $this->_getPortableTableIndexesList([], $value['table']);
continue;
}

if (! isset($foreignTableIndexes['primary'])) {
continue;
}
$list[$id]['foreign'][] = $value['to'];
}

$list[$id]['foreign'] = [...$list[$id]['foreign'], ...$foreignTableIndexes['primary']->getColumns()];
foreach ($list as $id => $value) {
if (count($value['foreign']) !== 0) {
continue;
}

// Inferring a shorthand form for the foreign key constraint, where the "to" field is empty.
// @see https://www.sqlite.org/foreignkeys.html#fk_indexes.
$foreignTableIndexes = $this->_getPortableTableIndexesList([], $value['foreignTable']);

if (! isset($foreignTableIndexes['primary'])) {
Deprecation::trigger(
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/6701',
'Introspection of SQLite foreign key constraints with omitted referenced column names'
. ' in an incomplete schema is deprecated.',
);

continue;
}

$list[$id]['foreign'][] = $value['to'];
$list[$id]['foreign'] = $foreignTableIndexes['primary']->getColumns();
}

return parent::_getPortableTableForeignKeysList($list);
Expand Down
38 changes: 30 additions & 8 deletions tests/Functional/Schema/SQLiteSchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@
use Doctrine\DBAL\Types\BlobType;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Types\Types;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;

use function array_keys;
use function array_shift;

class SQLiteSchemaManagerTest extends SchemaManagerFunctionalTestCase
{
use VerifyDeprecations;

protected function supportsPlatform(AbstractPlatform $platform): bool
{
return $platform instanceof SQLitePlatform;
Expand Down Expand Up @@ -51,9 +54,7 @@ public function testListForeignKeysFromExistingDatabase(): void
CREATE TABLE user (
id INTEGER PRIMARY KEY AUTOINCREMENT,
page INTEGER CONSTRAINT FK_1 REFERENCES page (key) DEFERRABLE INITIALLY DEFERRED,
parent INTEGER REFERENCES user(id) ON DELETE CASCADE,
log INTEGER,
CONSTRAINT FK_3 FOREIGN KEY (log) REFERENCES log ON UPDATE SET NULL NOT DEFERRABLE
parent INTEGER REFERENCES user(id) ON DELETE CASCADE
)
EOS);

Expand All @@ -72,16 +73,37 @@ public function testListForeignKeysFromExistingDatabase(): void
'',
['onUpdate' => 'NO ACTION', 'onDelete' => 'CASCADE', 'deferrable' => false, 'deferred' => false],
),
];

$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6701');

self::assertEquals($expected, $this->schemaManager->listTableForeignKeys('user'));
}

public function testListForeignKeysWithImplicitColumnsFromIncompleteSchema(): void
{
$this->connection->executeStatement('DROP TABLE IF EXISTS t1');
$this->connection->executeStatement(<<<'EOS'
CREATE TABLE t1 (
id INTEGER,
t2_id INTEGER,
FOREIGN KEY (t2_id) REFERENCES t2
)
EOS);

$this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/6701');

$expected = [
new ForeignKeyConstraint(
['log'],
'log',
['t2_id'],
't2',
[],
'FK_3',
['onUpdate' => 'SET NULL', 'onDelete' => 'NO ACTION', 'deferrable' => false, 'deferred' => false],
'',
['onUpdate' => 'NO ACTION', 'onDelete' => 'NO ACTION', 'deferrable' => false, 'deferred' => false],
),
];

self::assertEquals($expected, $this->schemaManager->listTableForeignKeys('user'));
self::assertEquals($expected, $this->schemaManager->listTableForeignKeys('t1'));
}

public function testColumnCollation(): void
Expand Down

0 comments on commit 8c9efcf

Please sign in to comment.