Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#26323 closed Bug (duplicate)

TransactionManagementError is raised when autocommit is false

Reported by: Tore Lundqvist Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

TransactionManagementError: "An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block." is raised when you try to used a database connection after a database exception even those autocommit was set to false from the start. It should be up to the user how to handle the database exception and the transaction as autocommit was set to false.

Change History (11)

comment:1 by Tore Lundqvist, 8 years ago

My suggestion is that "needs_rollback" is set to false in the end of exit in Atomic when connection.commit_on_exit is false. commit_on_exit == false seems to be there to indicate that autocommit was false before the atomic block. in_atomic_block is also set to false here for the same reason. See pull requset: https://github.com/django/django/pull/6240

comment:2 by Aymeric Augustin, 8 years ago

When you say "autocommit was set to false from the start", are you referring to settings.DATABASES['default']['AUTOCOMMIT'] = False? Can you provide a test case or sample code demonstrating the problem?

in reply to:  2 comment:4 by Tore Lundqvist, 8 years ago

Replying to aaugustin:

No, I run a management command and start it with set_autocommit(False) but that might give the same end result in this case. I looked for tests for similar things in Django to have something to start from but could not find any. I need to fake a database exception to be able to make a good test case for this I think.

comment:5 by Aymeric Augustin, 8 years ago

Can you check the note titled Avoid catching exceptions inside atomic?

Can you share a code sample?

in reply to:  5 comment:6 by Tore Lundqvist, 8 years ago

Replying to aaugustin:

Can you check the note titled Avoid catching exceptions inside atomic?

I'm not in a atomic block when handling the exception, no atomic blocks are used in the code. But deep down in Django atomic blocks are used anyway and thats where need_rollback is set. e.g. in save_base() /Users/torel/work/djangoproject/django/django/db/models/base.py:725

Can you share a code sample?

I can't share the actual code and it would not be very useful anyway I think. I have to write a test somehow, could you point me to a test I could use to get started?

comment:7 by Tim Graham, 8 years ago

Component: UncategorizedDatabase layer (models, ORM)

tests/transactions seems like a natural place.

comment:8 by Tore Lundqvist, 8 years ago

Thanks,
I added a test that shows the problem to the PR. If you comment out the fix and run the test with mysql you get the exception.

comment:9 by Aymeric Augustin, 8 years ago

Resolution: wontfix
Status: newclosed

Thanks for providing this example. Now I can see what you're talking about.

Django's transaction management is specifically designed to prevent the following pattern, which appears in your test:

  1. start a transaction
  2. run a statement that fails, typically because it breaks an integrity constraint
  3. run another statement without rolling back

That pattern isn't portable across databases. It doesn't work on PostgreSQL.

Also, in my opinion, it's an illegal behavior for a transactional database. A transaction is supposed to guarantee that either all statements have executed or none of them.

In my example, the first statement fails, the second succeeds, and MySQL happily commits the transaction, even though only one of the two statements executed.

To sum up, while MySQL doesn't care about transactional integrity, Django does. It's a design decision I made when I wrote the current transaction management.

If you want to discuss this decision, please write to the DevelopersMailingList.

comment:10 by Aymeric Augustin, 8 years ago

In case that isn't clear — you're hitting this issue because you aren't following the documentation I linked to in my earlier comment. Just add a with transaction.atomic inside your try/except and things should work.

Last edited 8 years ago by Aymeric Augustin (previous) (diff)

comment:11 by Aymeric Augustin, 8 years ago

I may have managed to reproduce this exception: https://code.djangoproject.com/ticket/26340

comment:12 by Aymeric Augustin, 8 years ago

Resolution: wontfixduplicate

The OP confirmed that #26340 is the same issue. Let's continue the discussion over there.

Note: See TracTickets for help on using tickets.
Back to Top