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.

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

CreateSavepoint method does not connect automatically to the database #3437

Closed
tezvi opened this issue Jan 23, 2019 · 8 comments
Closed

CreateSavepoint method does not connect automatically to the database #3437

tezvi opened this issue Jan 23, 2019 · 8 comments

Comments

@tezvi
Copy link

tezvi commented Jan 23, 2019

Bug Report

Q A
BC Break no
Version 2.9.1

Summary

When executing createSavepoint method on Connection instance PHP halts with null reference exception. The connection was not previously established.

Current behaviour

When executing Doctrine\DBAL\Connection::createSavepoint without previously issued queries or opened transaction an error occurs: "Call to a member function exec() on null".

$this->_conn->exec($this->platform->createSavePoint($savepoint));

How to reproduce

Make sure to call Doctrine\DBAL\Connection::createSavepoint when no connection was previously established.

Expected behaviour

The method should explicitly call Doctrine\DBAL\Connection::connect to create connection before issuing a SAVEPOINT query to the database.
The same behavior is already implemented for Doctrine\DBAL\Connection::beginTransaction and Doctrine\DBAL\Connection::rollBack methods.
The same approach could be also applied to Doctrine\DBAL\Connection::releaseSavepoint and Doctrine\DBAL\Connection::rollbackSavepoint.

@Ocramius
Copy link
Member

Would you be able to send in a test and a fix? Adding a $this->connect() call seems trivial, but we need a test case to prevent a regression.

@tezvi
Copy link
Author

tezvi commented Jan 23, 2019

Sure, I'll give it a go.

What do you think should we also log SAVEPOINT related queries like here:

if ($logger) {
$logger->startQuery('"SAVEPOINT"');
}

@Ocramius
Copy link
Member

That would make a lot of sense, yes

@tezvi
Copy link
Author

tezvi commented Feb 21, 2019

@Ocramius it seems it has already been fixed thx to @morozov #3443

@morozov
Copy link
Member

morozov commented Feb 21, 2019

@tezvi yes. It was detected by PHPStan as calling a method on a potentially null value.

@tezvi
Copy link
Author

tezvi commented Feb 21, 2019

Nice, case closed then 😁
The only thing that might be missing is proper SQL logging. It is provided by rollback and commit methods (hardcoded), but could be reused from platform instance createSavePoint and releaseSavepoints. What do you think?

Example:

$logger->startQuery('"ROLLBACK TO SAVEPOINT"');

@morozov
Copy link
Member

morozov commented Feb 21, 2019

@tezvi not sure what this is about. Is something not logged when it's supposed to be? Please feel free to file a new issue.

@morozov morozov closed this as completed Feb 21, 2019
@morozov morozov self-assigned this Jun 7, 2019
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants