Opened 7 years ago
Closed 7 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
sqlmigrateassignment ofself.output_transactionto considerconnection.features.can_rollback_ddlas well. - Adding a test in
tests/migrations/test_commands.pybased on an existing test for non-atomic migrations that mocksconnection.features.can_rollback_ddltoFalseinstead of overdiddingMIGRATION_MODULESto 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 , 7 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 7 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 7 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:4 by , 7 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 , 7 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 🙂)