[Django] #22496: Data migrations are not transactional (except on Postgres)
Django
-------------------------------------+-------------------------------------
Reporter: shai | Owner: nobody
Type: Bug | Status: new
Component: | Version: master
Migrations | Keywords: oracle mysql sqlite
Severity: Release | Has patch: 0
blocker | Needs tests: 1
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 1 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
In e74d2183c28467aefc0b87e3fa6d405dbfdea82c (master) and
9bf890f6f9d137d5b86fd9a6a38fb11c5d21b1af (1.7.x) schema editor was made
not to open transactions for backends that cannot roll back DDL. However,
there is nothing else that starts transactions in migrations, even if they
are data migrations -- so now, of the backends in core, migrations are
only transactional on PostgreSQL.
This is a spin-off from #22483.
--
Ticket URL: <https://code.djangoproject.com/ticket/22496>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: oracle mysql sqlite | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by aaugustin):
First, we should set `can_rollback_ddl = True` for SQLite. SQLite fully
supports transactional DDL, even thouh the Python sqlite3 module goes to
[http://bugs.python.org/issue10740 great lengths] to prevent developers
from using that capability. With the new transaction model introduced in
Django 1.6, I believe this change is safe.
----
This leaves MySQL and Oracle. Let's consider
[https://docs.djangoproject.com/en/dev/ref/migration-operations/#special-
operations special operations]:
- RunPython should generally run in a transaction, but I can imagine use
cases where it would run DDL statements through `cursor.execute`, among
other things.
- RunSQL is a problem: what if it runs some custom DDL statements?
- I'm not sure about SeparateDatabaseAndState.
In fact the real question is the failure mode when running DDL in a
transaction. If the transaction simply gets committed, it isn't great
(because of the mismatch between the actual transaction state and what
Django believes the state is) but it's acceptable. If Django crashes, it's
bad :-) We may get bitten by "Oracle needs to close and reopen the
connection after DDL" again...
If we decide to make these operations transactional, perhaps that could be
controlled by an `atomic` flag or keyword argument to the `Operation`
class. (Looking at the code, it would require a refactoring.) A simpler
solution would be to decorate `database_forwards/backwards` with `@atomic`
but that would make subclassing more difficult to get right.
The alternative is to leave transaction management entirely up to the
user. But users aren't good at transaction management in general.
If I had to choose, I would go for an `atomic` flag that's enabled by
default for the three special operations but can be turned off by users
who're running into one of the edge cases where the transaction prevents
the operation from executing.
--
Ticket URL: <https://code.djangoproject.com/ticket/22496#comment:1>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: oracle mysql sqlite | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"e368912902add8accf08d290ea77df49bd488744"]:
{{{
#!CommitTicketReference repository=""
revision="e368912902add8accf08d290ea77df49bd488744"
Set some transaction-related feature flags on SQLite.
Refs #22496.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22496#comment:2>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: oracle mysql sqlite | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"782fa14db4b855b4c5034413d1065ea0915f6491"]:
{{{
#!CommitTicketReference repository=""
revision="782fa14db4b855b4c5034413d1065ea0915f6491"
[1.7.x] Set some transaction-related feature flags on SQLite.
Refs #22496.
Backport of e368912 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22496#comment:3>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: oracle mysql sqlite | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by andrewgodwin):
My vote is that we definitely leave RunSQL as non-transactional on MySQL
and Oracle (SeparateDatabaseAndState is a combiner, it should be left
alone too).
I also agree that we should have RunPython in a transaction, but have an
argument to turn it off if you're doing schema changes on Oracle (the only
know crash case I know of). I'll commit that now and mark this as fixed.
--
Ticket URL: <https://code.djangoproject.com/ticket/22496#comment:4>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: oracle mysql sqlite | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"5a917cfef319df33ca30a3b27bd0b0533b8e63bb"]:
{{{
#!CommitTicketReference repository=""
revision="5a917cfef319df33ca30a3b27bd0b0533b8e63bb"
Fixed #22496: Data migrations get transactions again!
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22496#comment:5>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: oracle mysql sqlite | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------
Comment (by Andrew Godwin <andrew@…>):
In [changeset:"7f63ac5a9f0ed76d2e372a8c0fda95cce67e7170"]:
{{{
#!CommitTicketReference repository=""
revision="7f63ac5a9f0ed76d2e372a8c0fda95cce67e7170"
[1.7.x] Fixed #22496: Data migrations get transactions again!
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22496#comment:6>