SQLite platform does not support alter foreign key

[VincentLanglet]

Bug Report

Q	A
Version	3.4.0

Summary

Test on the ci of https://github.com/sonata-project/EntityAuditBundle are not working.
I tried with dbal 3.3.8 and it works, with 3.4.0 I get

Doctrine\DBAL\Exception: Sqlite platform does not support alter foreign key

The regression was introduced by #5427 cc @morozov
Adding

public function supportsForeignKeyConstraints()
{
    return false;
}

fix the issue.

How to reproduce

You can clone https://github.com/sonata-project/EntityAuditBundle
run composer install
and then make test

Expected behaviour

No error

[morozov]

@VincentLanglet this behavior is expected. See #5409.

[VincentLanglet]

@VincentLanglet this behavior is expected. See #5409.

But then, the fact our tests were passing and now they dont doesn't mean it's a BC-break ?

Any idea how should it be fixed on my side then ?
Currently I'm getting the issue when I try to drop the Schema https://github.com/sonata-project/EntityAuditBundle/blob/1.x/tests/BaseTest.php#L192 (and then the next test is failing because some table already exists). Is there another method to call instead ?

[morozov]

But then, the fact our tests were passing and now they dont doesn't mean it's a BC-break ?

Technically, any change is a BC break:

This might be considered a minor breaking change (see xkcd #1172)

Practically, it depends on whether this behavior is covered by the API contract. The documentation on AbstractPlatform::supportsForeignKeyConstraints() doesn't describe what exactly the "support" mean, so it's up for interpretation. The lack of documented semantics renders this method pointless and even harmful.

Any idea how should it be fixed on my side then ?

I am sorry but I'm not familiar with your use case. What does it try to achieve by altering foreign keys on an existing table and how did it achieve that previously?

[VincentLanglet]

I am sorry but I'm not familiar with your use case. What does it try to achieve by altering foreign keys on an existing table and how did it achieve that previously?

I'm not trying to altering foreign keys, just creating/dropping the schema database.

I'm not familiar to these tests, they were written a long time ago by, I think, some doctrine maintainer.
And there are setting up

• An entityManager based on a sqlite connection
• A SchemaTool using the entityManager

Then test are doing things like

$this->em->persist($cat);
$this->em->flush();
static::assertCount(1, $this->em->getConnection()->fetchAllAssociative('SELECT id FROM revisions'));

And finally in the tearDown, the schema were drop with

$this->getSchemaTool()->dropSchema($classes);

I also tried

$this->getSchemaTool()->dropDatabase();

without more success.

All I want is a way to create and then clear my sqlite-memory schema/database.

Practically, it depends on whether this behavior is covered by the API contract. The documentation on AbstractPlatform::supportsForeignKeyConstraints() doesn't describe what exactly the "support" mean, so it's up for interpretation. The lack of documented semantics renders this method pointless and even harmful.

I feel like that the fact I currently cannot create/drop anymore the schema or the database of my sqlite database is more than just an interpretation.

[morozov]

It looks like the reason is that the ORM uses the deprecated DBAL APIs that don't properly drop SQLite schema if it contains foreign keys. It will be addressed in ORM 2.13 (doctrine/orm#9799).

Could you try running the tests using ORM ^2.13@dev?

[VincentLanglet]

It seems to work indeed with 2.13@dev. (Cf sonata-project/EntityAuditBundle#505)

[morozov]

The ORM 2.13.0 milestone is 94% complete.

@derrabus, would it be possible to expedite the release to address this issue for ORM users?

[morozov]

Just a few more observations:

1. The fact that the DBAL now supports foreign key constraints on SQLite doesn't mean that any code that uses it also supports them. For instance, the ORM up to 2.13 doesn't.
2. It looks like EntityAuditBundle has been successfully using foreign key constraints on SQLite despite the lack of the official support. If we consider supportsForeignKeyConstraints() part of the API contract, then the bundle is in violation of the contract: it uses foreign key constraints with SQLite regardless of the lack of their support.
3. The DBAL prior to 3.4.0 is also in violation of its own contract: it creates foreign keys on SQLite despite the lack of their support. The following two code fragments belong to the same revision of the same file:

   dbal/src/Platforms/SqlitePlatform.php

   Line 871 in f873a82

   $createFlags = $createFlags ?? self::CREATE_INDEXES | self::CREATE_FOREIGNKEYS;

   dbal/src/Platforms/SqlitePlatform.php

   Lines 825 to 828 in f873a82

   	public function supportsForeignKeyConstraints()
   	{
   	return false;
   	}

This API is impossible to deal with, it can be only broken. If this sort of BC break is not acceptable, then reverting supportsForeignKeyConstraints() is also a BC break.

[derrabus]

I have released ORM 2.13.0.

[morozov]

Thank you for the quick turnaround, @derrabus!