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

Why do you begin a transaction in your commit method?! #6258

Open
mica16 opened this issue Jan 5, 2024 · 11 comments
Open

Why do you begin a transaction in your commit method?! #6258

mica16 opened this issue Jan 5, 2024 · 11 comments
Labels

Comments

@mica16
Copy link

mica16 commented Jan 5, 2024

Bug Report

Using Doctrine 3.7.2 and Symfony 6.3.

Summary

In the EntityManager.php file, we have the wrapInTransactionmethod:

https://github.com/doctrine/orm/blob/e585a92763612455f591b44cf0482d9852cc5fc0/lib/Doctrine/ORM/EntityManager.php#L267-L284

Inside the code of $this->conn->commit() we can find that at the very end of the method:

dbal/src/Connection.php

Lines 1442 to 1448 in 6a793fb

if ($this->autoCommit !== false || $this->transactionNestingLevel !== 0) {
return $result;
}
$this->beginTransaction();
return $result;

Why to declare a nested transaction at the end without ever closing it?

Current behaviour

I noticed that when I run my integration tests, warning about "Nested Transactions..."
I was "forced" to declare "use_savepoints: true" in my doctrine.yaml.

How to reproduce

Don't declare use_savepoints: true in doctrine.yaml and run an integration test using wrapInTransaction.

Expected behaviour

Don't declare a useless nested transaction.

@mica16 mica16 closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2024
@greg0ire

This comment was marked as resolved.

@mica16

This comment was marked as resolved.

@greg0ire

This comment was marked as resolved.

@mica16 mica16 reopened this Jan 5, 2024
@mica16

This comment was marked as resolved.

@mica16 mica16 closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2024
@greg0ire

This comment was marked as resolved.

@greg0ire greg0ire reopened this Jan 5, 2024
@greg0ire greg0ire transferred this issue from doctrine/dbal Jan 5, 2024
@greg0ire
Copy link
Member

greg0ire commented Jan 5, 2024

Ah actually, the second code block is in doctrine/dbal, let's transfer the issue back!

@greg0ire greg0ire transferred this issue from doctrine/orm Jan 5, 2024
@greg0ire
Copy link
Member

greg0ire commented Jan 5, 2024

And to answer your question, this is done in auto-commit mode: https://www.doctrine-project.org/projects/doctrine-dbal/en/3.7/reference/transactions.html#auto-commit-mode

The new transaction is not nested, since we are inside commit(), as explained in the comment: // commits transaction and immediately starts a new one

@bassilil
Copy link

cod done?

@Guuzen
Copy link

Guuzen commented Jun 11, 2024

And to answer your question, this is done in auto-commit mode: https://www.doctrine-project.org/projects/doctrine-dbal/en/3.7/reference/transactions.html#auto-commit-mode

The new transaction is not nested, since we are inside commit(), as explained in the comment: // commits transaction and immediately starts a new one

But why do you begin transactions in the commit method? I haven't found an answer about the purpose in the doc.
I mean I tried to turn autocommit off and expected to manage transactions manually, but it turned out that dbal behaves differently to what I expected. In this case DBAL tries to handle transactions itself, but only partially because I need to call commit in order to finalize transactions. 🤔

@greg0ire
Copy link
Member

Sorry, I think I meant to wrote "this is done when not in auto-commit mode". When in auto-commit mode, there are no transactions at all => everything is committed to the database as soon as it sent.
When not in auto-commit mode, a transaction is automatically started to prevent that, but you have to close is yourself, and Doctrine cannot guess when you want to do that. At least that's my understanding with fresh eyes, and a cursory glance at the docs I mentioned earlier.

@Guuzen
Copy link

Guuzen commented Jun 11, 2024

When not in auto-commit mode, a transaction is automatically started to prevent that, but you have to close is yourself, and Doctrine cannot guess when you want to do that

But it can guess when I need to open a new one 🤔
Well, thank you. I think I understand why it works this way. Because somebody decided that autocommit = off in rdbmses should have two functions:

  • Avoid commiting every statement
  • Implicitly open transactions after every commit and after connection was established.

Current behaviour just mimics autocommit behaviour in databases, but consequently (in the case where autocommit = off) usage of transactional methods becomes pointless (because anyway, we will need to perform an additional commit and also there will probably be unnecessary nested transactions) and transactions demarcation instead of becoming explicit, stays implicit, but in a different way in comparison to autocommit = on 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants