[Django] #26323: TransactionManagementError is raised when autocommit is false
Django
-------------------------------+--------------------
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26323>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by tltx):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:1>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:2>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tltx):
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 that 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:3>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tltx):
Replying to [comment:2 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:4>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Can you check the note titled
[https://docs.djangoproject.com/en/1.9/topics/db/transactions
/#controlling-transactions-explicitly Avoid catching exceptions inside
atomic]?
Can you share a code sample?
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:5>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tltx):
Replying to [comment:5 aaugustin]:
> Can you check the note titled
[https://docs.djangoproject.com/en/1.9/topics/db/transactions
/#controlling-transactions-explicitly 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:6>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* component: Uncategorized => Database layer (models, ORM)
Comment:
`tests/transactions` seems like a natural place.
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:7>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tltx):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:8>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* status: new => closed
* resolution: => wontfix
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:9>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:10>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
I may have managed to reproduce this exception:
https://code.djangoproject.com/ticket/26340
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:11>
Django
Reporter: tltx | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* resolution: wontfix => duplicate
Comment:
The OP confirmed that #26340 is the same issue. Let's continue the
discussion over there.
--
Ticket URL: <https://code.djangoproject.com/ticket/26323#comment:12>