Fix and improve column renaming on SQLite.

Review Request #11090 — Created July 21, 2020 and submitted

Information

Django Evolution
master

Reviewers

Modern versions of SQLite (3.26+) don't require a full table rebuild in
order to rename a column, which is good because we weren't even handling
that correctly before, leaving bad foreign key references from other
tables pointing to the old column names. This noticeably breaks on
Django 2.2, which also had some fixes for bad references, and performs a
reference check after altering database schema to ensure they're caught,
breaking us (and rightfully so).

On the modern versions, we can just run ALTER TABLE ... RENAME COLUMN
to perform the rename. This will take care of updating any references
from other tables. This is performed as a standalone statement, run
independently of any queued-up table rebuilds.

For older versions of SQLite, we now update references from other
tables. The way this is done is really wonky, but is the method
recommended by the documentation.

First, we have to run all this at the time in which we want to perform
the rename, as it depends on some database state affected by other
schema modifications. To do this, we make use of the new callable
entries for SQL statements allowed in our execution runner.

Performing a reference update requires the following steps:

  1. Fetching the current SQLite schema version and storing it
    (querying PRAGMA schema_version)
  2. Enabling schema writing (PRAGMA writable_schema = 1)
  3. Updating the SQL for all tables, replacing the references
    (UPDATE sqlite_master SET sql = replace(sql, old_refs, new_refs),
    with old_refs and new_refs being a string like
    REFERENCES "<tablename>" ("<colname>"))
  4. Setting a new schema version (PRAGMA schema_version = ...)
  5. Disabling schema writing (PRAGMA writable_schema = 0)
  6. Committing the transaction and performing a VACUUM (which rebuilds
    the database)

This all needs to be done in its own transaction, as well. This is where
we cheat. We have no way at this time of telling Django Evolution (or
Django) that we want a new transaction, since we're already going to be
in one when this executes (and may not be able to have much control over
this, depending on when things are called).

So we ignore Django's transaction management and just send along the SQL
statements to commit the existing transaction, begin a new one, send the
above, commit that, and then begin a new one for the remainder of
Django's transaction management. It's pulling the rug out from under
Django a bit, but if our job is done right, Django will never notice.

The end result is that we have proper column renaming support for a
range of SQLite versions, and all unit tests pass on Django 2.2.

Unit tests pass for all supported versions of Django and Python.

Tested with SQLite 3.24, 3.26, and 3.28.

Summary ID
Fix and improve column renaming on SQLite.
Modern versions of SQLite (3.26+) don't require a full table rebuild in order to rename a column, which is good because we weren't even handling that correctly before, leaving bad foreign key references from other tables pointing to the old column names. This noticeably breaks on Django 2.2, which also had some fixes for bad references, and performs a reference check after altering database schema to ensure they're caught, breaking us (and rightfully so). On the modern versions, we can just run `ALTER TABLE ... RENAME COLUMN` to perform the rename. This will take care of updating any references from other tables. This is performed as a standalone statement, run independently of any queued-up table rebuilds. For older versions of SQLite, we now update references from other tables. The way this is done is really wonky, but is the method recommended by the documentation. First, we have to run all this at the time in which we want to perform the rename, as it depends on some database state affected by other schema modifications. To do this, we make use of the new callable entries for SQL statements allowed in our execution runner. Performing a reference update requires the following steps: 1. Fetching the current SQLite schema version and storing it (querying `PRAGMA schema_version`) 2. Enabling schema writing (`PRAGMA writable_schema = 1`) 3. Updating the SQL for all tables, replacing the references (`UPDATE sqlite_master SET sql = replace(sql, old_refs, new_refs)`, with `old_refs` and `new_refs` being a string like ` REFERENCES "<tablename>" ("<colname>") `) 4. Setting a new schema version (`PRAGMA schema_version = ...`) 5. Disabling schema writing (`PRAGMA writable_schema = 0`) 6. Committing the transaction and performing a `VACUUM` (which rebuilds the database) This all needs to be done in its own transaction, as well. This is where we cheat. We have no way at this time of telling Django Evolution (or Django) that we want a new transaction, since we're already going to be in one when this executes (and may not be able to have much control over this, depending on when things are called). So we ignore Django's transaction management and just send along the SQL statements to commit the existing transaction, begin a new one, send the above, commit that, and then begin a new one for the remainder of Django's transaction management. It's pulling the rug out from under Django a bit, but if our job is done right, Django will never notice. The end result is that we have proper column renaming support for a range of SQLite versions, and all unit tests pass on Django 2.2.
d7a5c67f6d68684db4df9c5761fec88fe507b452
Description From Last Updated

E131 continuation line unaligned for hanging indent

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (b2243a6)
Loading...