Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix unwanted SQLite schema emulation in SqliteSchemaManager #6337

Open
wants to merge 1 commit into
base: 3.8.x
Choose a base branch
from
+66 −4
Open

Fix unwanted SQLite schema emulation in SqliteSchemaManager #6337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions psalm.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,9 @@

<!-- TODO: remove in 4.0.0 -->
<referencedMethod name="Doctrine\DBAL\Platforms\DB2Platform::getForUpdateSQL"/>

<!-- TODO: remove in 4.0.0 -->
<referencedMethod name="Doctrine\DBAL\Platforms\SqlitePlatform::canEmulateSchemas"/>
</errorLevel>
</DeprecatedMethod>
<DeprecatedProperty>
Expand Down
2 changes: 1 addition & 1 deletion src/Platforms/SqlitePlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@
*/
public function canEmulateSchemas()
{
Deprecation::trigger(
Deprecation::triggerIfCalledFromOutside(

Check warning on line 911 in src/Platforms/SqlitePlatform.php

View check run for this annotation

Codecov / codecov/patch

src/Platforms/SqlitePlatform.php#L911 

Added line #L911 was not covered by tests
'doctrine/dbal',
'https://github.com/doctrine/dbal/pull/4805',
'SqlitePlatform::canEmulateSchemas() is deprecated.',
Expand Down
12 changes: 9 additions & 3 deletions src/Schema/SqliteSchemaManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,9 @@

if ($tableName !== null) {
$conditions[] = 't.name = ?';
$params[] = str_replace('.', '__', $tableName);
$params[] = $this->_platform->canEmulateSchemas()
? str_replace('.', '__', $tableName)
: $tableName;

Check warning on line 709 in src/Schema/SqliteSchemaManager.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/SqliteSchemaManager.php#L707-L709 

Added lines #L707 - L709 were not covered by tests
}

$sql .= ' WHERE ' . implode(' AND ', $conditions) . ' ORDER BY t.name, c.cid';
Expand All @@ -729,7 +731,9 @@

if ($tableName !== null) {
$conditions[] = 't.name = ?';
$params[] = str_replace('.', '__', $tableName);
$params[] = $this->_platform->canEmulateSchemas()
? str_replace('.', '__', $tableName)
: $tableName;

Check warning on line 736 in src/Schema/SqliteSchemaManager.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/SqliteSchemaManager.php#L734-L736 

Added lines #L734 - L736 were not covered by tests
}

$sql .= ' WHERE ' . implode(' AND ', $conditions) . ' ORDER BY t.name, i.seq';
Expand All @@ -755,7 +759,9 @@

if ($tableName !== null) {
$conditions[] = 't.name = ?';
$params[] = str_replace('.', '__', $tableName);
$params[] = $this->_platform->canEmulateSchemas()
? str_replace('.', '__', $tableName)
: $tableName;

Check warning on line 764 in src/Schema/SqliteSchemaManager.php

View check run for this annotation

Codecov / codecov/patch

src/Schema/SqliteSchemaManager.php#L762-L764 

Added lines #L762 - L764 were not covered by tests
}

$sql .= ' WHERE ' . implode(' AND ', $conditions) . ' ORDER BY t.name, p.id DESC, p.seq';
Expand Down
53 changes: 53 additions & 0 deletions tests/Functional/Schema/SqliteSchemaManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
use Doctrine\DBAL\Types\BlobType;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Types\Types;
use ReflectionMethod;

use function array_keys;
use function array_map;
use function array_shift;
use function assert;
use function dirname;
Expand Down Expand Up @@ -397,4 +399,55 @@ public function testShorthandInForeignKeyReferenceWithMultipleColumns(): void
$createTableTrackSql,
);
}

public function testListTableNoSchemaEmulation(): void
{
$databasePlatform = $this->connection->getDatabasePlatform();
assert($databasePlatform instanceof SqlitePlatform);
$databasePlatform->disableSchemaEmulation();

$this->dropTableIfExists('`list_table_no_schema_emulation.test`');

$this->connection->executeStatement(<<<'DDL'
CREATE TABLE `list_table_no_schema_emulation.test` (
id INTEGER,
parent_id INTEGER,
PRIMARY KEY (id),
FOREIGN KEY (parent_id) REFERENCES `list_table_no_schema_emulation.test` (id)
);
DDL);

$this->connection->executeStatement(<<<'DDL'
CREATE INDEX i ON `list_table_no_schema_emulation.test` (parent_id);
DDL);

$schemaManager = $this->schemaManager;
$refl = new ReflectionMethod($schemaManager, 'selectTableColumns');
$refl->setAccessible(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing method accessibility in tests is a smell. Why do you need to do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because full table introspection with schema is not supported (yet).

Therefore, I test functional behaviour using protected methods.

$res = $refl->invoke($schemaManager, 'main', 'list_table_no_schema_emulation.test')
->fetchAllAssociative();

self::assertSame([
['list_table_no_schema_emulation.test', 'id'],
['list_table_no_schema_emulation.test', 'parent_id'],
], array_map(static fn (array $row) => [$row['table_name'], $row['name']], $res));

$refl = new ReflectionMethod($schemaManager, 'selectIndexColumns');
$refl->setAccessible(true);
$res = $refl->invoke($schemaManager, 'main', 'list_table_no_schema_emulation.test')
->fetchAllAssociative();

self::assertSame([
['list_table_no_schema_emulation.test', 'i'],
], array_map(static fn (array $row) => [$row['table_name'], $row['name']], $res));

$refl = new ReflectionMethod($schemaManager, 'selectForeignKeyColumns');
$refl->setAccessible(true);
$res = $refl->invoke($schemaManager, 'main', 'list_table_no_schema_emulation.test')
->fetchAllAssociative();

self::assertSame([
['list_table_no_schema_emulation.test', 'parent_id', 'id'],
], array_map(static fn (array $row) => [$row['table_name'], $row['from'], $row['to']], $res));
}
}