Is support for (releasing) savepoints required?

[ddeboer]

Since #232, using this bundle with a database that supports savepoints has become required. While I understand there are good reasons for this, I’m wondering if it’s strictly necessary.

My use case: I use this bundle in a project with integration tests against MS SQL Server. I don’t particularly like SQL Server but it happens to be run in production. SQL Server doesn’t support releasing savepoints.

[dmaicher]

Unfortunately I don't see a way how this can work for the upcoming DBAL 4 without making use of savepoints. So for now I'm afraid version 8 won't be usable for you then :confused:

Feel free to take a look yourself to see if you can make it work deprecation free on DBAL 3 (being compatible with DBAL 4) without using savepoints though.

[garak]

By the way, that change is not mentioned in any release, nor in an UPGRADE file in this repo. The only reference I can find about savepoints is in the README (and I'm not even sure it still applies to version 8)

[dmaicher]

@garak for now I mentioned it on the first beta release. I will update the readme for the final 8.0 release.

[garak]

@garak for now I mentioned it on the first beta release. I will update the readme for the final 8.0 release.

Thanks for the reply, but the mention in the first beta is not explaining how to upgrade. Moreover, it explicitly says "no BC breaks", which seems no true to me. I think you can already provide an upgrade path in the README, as soon as you mention the versions (to avoid the possibility that someone can misunderstand it and apply it now). In this way you can encourage people to try version 8, and as soon as you'll release a stable version you'll be already prepared.

[dmaicher]

Indeed I now added (as long as your database platform supports savepoints).

Well the upgrade path for platforms without full savepoint support would be to change the database platform I guess :yum:

I will take a look to add something to the readme soon.

[ddeboer]

To clarify: all Doctrine drivers support savepoints, including MS SQL Server. It’s releasing savepoints which is unsupported and triggers the error message:

https://github.com/dmaicher/doctrine-test-bundle/blob/a9ff7d7d1ba888374ca3e583cddc3fa056db2220/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticDriver.php#L54-L56

My project has two connections: a regular (Postgres) one, that works fine with this bundle. The other connection is SQL Server (not my choice), which no longer works with version 8 of this bundle. I worked around this by managing the transaction myself and disabling this bundle for the SQL Server connection:

# config/packages/test/dama_doctrine_test_bundle.yaml
dama_doctrine_test:
    enable_static_connection:
        default: true
        mssql: false

use Doctrine\DBAL\Connection;

final class MyTest extends KernelTestCase
{
    private Connection $connection;

    protected function setUp(): void
    {
        $this->connection = self::getContainer()->get('doctrine.dbal.mssql_connection');
        $this->connection->beginTransaction();
    }

    protected function tearDown(): void
    {
        $this->connection->rollBack();
    }

    public function test(): void 
    {
        // my test case
    }
}

@dmaicher Does this bundle need to rely on releasing savepoints? Or could a simple rollBack() suffice as a fallback for platforms that don’t support it?

[dmaicher]

Maybe there is a way indeed :thinking: Let me look into it

[dmaicher]

@ddeboer could you test this? https://github.com/dmaicher/doctrine-test-bundle/pull/257

I think we actually don't need to release the savepoint. From what I can see RDBMS allow overwriting an existing savepoint with the same name anyway.

[ddeboer]

I can confirm #257 works and allowed me to revert my local changes described above.