Skip to content

Conversation

@Chris53897
Copy link
Contributor

#298

Maybe the only problem is this change.
https://github.com/doctrine/dbal/blob/4.0.x/UPGRADE.md#bc-break-renamed-sqlite-platform-classes

TODO: I know there must be a bc layer for dbal < 4

Co-authored-by: Alexis Lefebvre <[email protected]>
@Chris53897
Copy link
Contributor Author

Solving #300 would help development.

@alexislefebvre
Copy link
Collaborator

alexislefebvre commented Apr 14, 2024

On the 3.x branch, with doctrine/dbal 4, this line is not working:

$databaseTool = $this->items[$registry->getName()][$driverName] ?? $this->items[$registry->getName()]['default'];

$this->items[$registry->getName()][$driverName] is evaluated as false, null or whatever and it switch to the statement after ??.

Update: you found the issue: DatabaseToolCollection registers an item with key Doctrine\DBAL\Platforms\SqlitePlatform then later the key Doctrine\DBAL\Platforms\SQLitePlatform is searched.

@alexislefebvre
Copy link
Collaborator

Please check this:

Feel free to backport it to the 2.x branch.

@alexislefebvre
Copy link
Collaborator

alexislefebvre commented May 4, 2024

Tests were OK on local environment but not on CI, because the versions of databases used on GitHub Actions were outdated: bf29b7d

if ($platform instanceof AbstractMySQLPlatform || $platform instanceof MySqlPlatform) {
return 'mysql';
} elseif ($platform instanceof SqlitePlatform) {
} elseif ($platform instanceof SQLitePlatform) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
} elseif ($platform instanceof SQLitePlatform) {
} elseif ($platform instanceof SqlitePlatform || $platform instanceof SQLitePlatform) {

Is this needed to keep compatibility?

private function isSqlite(): bool
{
return $this->connection->getDatabasePlatform() instanceof SqlitePlatform;
return $this->connection->getDatabasePlatform() instanceof SQLitePlatform;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return $this->connection->getDatabasePlatform() instanceof SQLitePlatform;
return $this->connection->getDatabasePlatform() instanceof SqlitePlatform || $this->connection->getDatabasePlatform() instanceof SQLitePlatform;

Is this needed to keep compatibility?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants