Opened 14 years ago
Closed 12 years ago
#14970 closed New feature (fixed)
Inconsistency in handling of managed/unmanaged transactions
| Reported by: | mwrobel | Owned by: | Aymeric Augustin |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 1.2 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
I think that there are some bugs in the way the 'managed' flag is handled in enter_transaction_management() and leave_transaction_management() in django/db/transaction.py.
- enter_transaction_management() has a 'managed' parameter. I guess it should set the new transaction in managed mode depending on the value of this parameter, but it takes the mode from the surrounding transaction or from the settings (as stated in a comment). Surprisingly, connection._enter_transaction_management() is called with the value of the 'managed' parameter instead of the mode that is set for the new transaction.
- leave_transaction_management() calls connection._leave_transaction_management() passing the mode of the current transaction (the transaction that is being left) as a parameter. From what I can see in django/db/backends/postgresql_psycopg2/base.py, it expects to get the mode of the transaction that will be active after leave_transaction_management() finishes.
Now I will describe the use case in which I encountered the problems: I use the psycopg2 backend and I want to have the connection in database-level autocommit mode by default. Sometimes I want to begin a managed transaction and after some operations commit or rollback it and return to the database-level autocommit mode.
Currently I have to enter the transaction as follows:
enter_transaction_management(managed=True) managed(True)
and leave:
managed(False) leave_transaction_management()
I think that the managed() calls should be redundant, but the first/second one is needed to circumvent the first/second problem I mentioned.
My proposal is to:
- change enter_transaction_management() to set mode depending on the parameter (and when parameter is not given, take the mode from the surrounding transaction or the settings).
- change leave_transaction_management() to pass the mode of the transaction that becomes active to connection._leave_transaction_management() (I don't know what was the intended semantics, but it is what pycopg2 backend seems to expect, and other backends don't override _leave_transaction_management()).
I see that the proposed changes can break backward compatibility, but I think that they don't change any documented behavior.
I can create patches that fix the problem, but I would like to hear from Django developers if the fix is acceptable before I write the patches.
Change History (6)
comment:1 by , 14 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 14 years ago
| Severity: | → Normal |
|---|---|
| Type: | → New feature |
marking as new feature due to backward-compatibility concerns.
comment:5 by , 12 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:6 by , 12 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Accepting, because what you describe makes sense.
However, backwards compatibility can't be ignored here. Transactions are one of those areas where very subtle changes in behavior have the potential to wreak havoc in live systems. I haven't sat down to fully work out the potential impact of what you're proposing, but before anything is committed, I'll need to see a very detailed analysis of any potential impact. The interaction with Postgres is particularly important, because the _enter_transaction_managmeent call on the backend exists for the benefit of Postgres.