Opened 6 years ago
Closed 6 years ago
#30189 closed Bug (fixed)
sqlmigrate wraps it's outpout in BEGIN/COMMIT even if the database doesn't support transactional DDL
Reported by: | Simon Charette | Owned by: | Parth Patil |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
The migration executor only adds the outer BEGIN
/COMMIT
if the migration is atomic and the schema editor can rollback DDL but the current sqlmigrate
logic only takes migration.atomic
into consideration.
The issue can be addressed by
- Changing
sqlmigrate
assignment ofself.output_transaction
to considerconnection.features.can_rollback_ddl
as well. - Adding a test in
tests/migrations/test_commands.py
based on an existing test for non-atomic migrations that mocksconnection.features.can_rollback_ddl
toFalse
instead of overdiddingMIGRATION_MODULES
to point to a non-atomic migration.
I marked the ticket as easy picking because I included the above guidelines but feel free to uncheck it if you deem it inappropriate.
Change History (7)
comment:1 by , 6 years ago
Description: | modified (diff) |
---|
comment:2 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 6 years ago
Hey, I'm working on this ticket, I would like you to know as this is my first ticket it may take little longer to complete :).
Here is a | link to the working branch
You may feel free to post references or elaborate more on the topic.
comment:5 by , 6 years ago
Hi Parth. No problem. If you need help please reach out to e.g. django-core-mentorship citing this issue, and where you've got to/got stuck.
Welcome aboard, and have fun! ✨
Super. We don't have enough Easy Pickings tickets for the demand, so this kind of thing is great. (IMO 🙂)