Opened 3 months ago

Last modified 2 months ago

#29182 new Cleanup/optimization

SQLite database migration breaks ForeignKey constraint, leaving <table_name>__old in db schema

Reported by: ezaquarii Owned by: nobody
Component: Migrations Version: 2.0
Severity: Normal Keywords: sqlite migration
Cc: Simon Charette Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by ezaquarii)

SQLite table alteration uses rename and copy method. When running a database migration from Django command (via call_command('migrate')), database is left with references to <table_name>__old table that has been dropped after migration.

This happens only when running SQLite DB migrations from management command under transaction.
Running migrations via ./manage.py migrate does not cause any issues.

This is the demonstration:
https://github.com/ezaquarii/django-sqlite-migration-bug

Steps to reproduce the bug

  1. run ./bootstrap.sh
  2. from import app.models import *
  3. Bravo.objects.create()
  4. SQL cannot be executed due to missing table main.app_alpha__old
  • app/management/commands/bootstrap.py:9: run the migration
  • alpha table is created
  • bravo table is created; it uses foreign key to alpha
  • another alpha migration is run, adding dummy field
  • alpha table is renamed to alpha__old
  • bravo foreign key is re-linked to alpha__old
  • new table alpha is created and data from alpha__old is copied with applied alterations
  • alpha__old is dropped
  • bravo still references alpha__old - ForeignKey constraint is not updated and DB is left in unusable state
CREATE TABLE "app_bravo" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "dummy" varchar(10) NOT NULL, "alpha_id" integer NOT NULL REFERENCES "app_alpha__old" ("id") DEFERRABLE INITIALLY DEFERRED);

Tested with Django versions: 2.0.1 and 2.0.2

Attachments (2)

bug_repro.sh (990 bytes) - added by ezaquarii 3 months ago.
Bug reproduction steps as shell script
db_schema.sql (8.5 KB) - added by ezaquarii 3 months ago.
Database schema after running migrations - see references to temporary openvpn_serverold table

Download all attachments as: .zip

Change History (9)

Changed 3 months ago by ezaquarii

Attachment: bug_repro.sh added

Bug reproduction steps as shell script

Changed 3 months ago by ezaquarii

Attachment: db_schema.sql added

Database schema after running migrations - see references to temporary openvpn_serverold table

comment:1 Changed 3 months ago by ezaquarii

Description: modified (diff)

comment:2 Changed 3 months ago by Tim Graham

Could you provide a more minimal project that reproduces the issue? In particular, the steps to reproduce shouldn't require installing a bunch of NodeJS packages. Thanks!

comment:3 in reply to:  2 Changed 3 months ago by ezaquarii

Description: modified (diff)

Please check this repository:

https://github.com/ezaquarii/django-sqlite-migration-bug

Please check out bootstrap.py

        with transaction.atomic():
            call_command('migrate')

I think this is the issue. It works ok without transaction.

Last edited 3 months ago by ezaquarii (previous) (diff)

comment:4 Changed 3 months ago by ezaquarii

Description: modified (diff)

comment:5 Changed 3 months ago by ezaquarii

Description: modified (diff)

comment:6 Changed 3 months ago by Simon Charette

Cc: Simon Charette added
Triage Stage: UnreviewedAccepted

Hey ezaquarii, thank you for your great report.

You can get a bit of context about why all the SQLite constraint workarounds are required in #28849 but in this case I suspect odd things happen because, as you might know, savepoints are used in nested atomic() blocks.

The migration executor was not designed to be used within a transaction as it does some pretty intense transaction management itself so I'm accepting on the basis that we should do a better job at raising an error in this case. The fact that this doesn't happen when ./manage.py migrate is called directly is a good indicator that other weird things might be happening under the hood because this case is completely untested by the suite right now. For example, do the backends that we assume have transactional DDL support all behave the same way with regards to DDL in savepoints?

I guess you were trying to come up with some kind of deploying script that either fails or succeeds to apply multiple migrations?

I think the best place to raise the error from would be in sqlite3.DatebaseSchemaEditor.__enter__ when disable_constraint_checking returns False to signify the operation didn't succeed as database alteration methods the returned instance exposes all assume foreign key constraints are disabled. FWIW the reason why disable_constraint_checking returns false within a transaction is that PRAGMA foreign_keys = ON/OFF has absolutely no effect in a transaction which is one of the reason why the workarounds in #28849 had to be implemented.

Last edited 3 months ago by Simon Charette (previous) (diff)

comment:7 Changed 2 months ago by Tim Graham

Type: UncategorizedCleanup/optimization
Note: See TracTickets for help on using tickets.
Back to Top