Opened 8 months ago

Closed 7 months 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: master
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 Simon Charette)

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

  1. Changing sqlmigrate assignment of self.output_transaction to consider connection.features.can_rollback_ddl as well.
  2. Adding a test in tests/migrations/test_commands.py based on an existing test for non-atomic migrations that mocks connection.features.can_rollback_ddl to False instead of overdidding MIGRATION_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 Changed 8 months ago by Simon Charette

Description: modified (diff)

comment:2 Changed 8 months ago by Parth Patil

Owner: changed from nobody to Parth Patil
Status: newassigned

comment:3 Changed 8 months ago by Carlton Gibson

Triage Stage: UnreviewedAccepted

I marked the ticket as easy picking because I included the above guidelines but feel free to uncheck it if you deem it inappropriate.

Super. We don't have enough Easy Pickings tickets for the demand, so this kind of thing is great. (IMO 🙂)

comment:4 Changed 8 months ago by Parth Patil

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 Changed 8 months ago by Carlton Gibson

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! ✨

comment:6 Changed 8 months ago by Parth Patil

Has patch: set
Needs documentation: set
Last edited 8 months ago by Parth Patil (previous) (diff)

comment:7 Changed 7 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In acc04198:

Fixed #30189 -- Removed transaction from sqlmigrate output if database doesn't use one.

Note: See TracTickets for help on using tickets.
Back to Top