Fix ordering and conflict issues with adding and dropping indexes.

Review Request #12280 — Created May 14, 2022 and submitted

Information

Django Evolution
release-2.x

Reviewers

When switching between db_index and unique states, or when dropping
indexes due to db_index=False while performing other changes to the
model, we'd run into a couple issues. We could end up trying to create
duplicate indexes, drop indexes too late, or create standard and
unique indexes in one go.

There were two reasons for these problems, one common to all databases,
and one SQLite-specific.

The common issue was that we handled db_index and unique changes
separately. If enabling db_index and unique in the same batch of
changes, we could get indexes for both. If disabling both, we could get
redundant drops. If switching from one to the other, we could get
ordering issues or accidental drops or adds of an index.

This is resolved by considering db_index and unique changes
together, and ensuring that we never add or drop two types of indexes
at once or put them in the wrong order. The logic here is common to all
databases.

SQLite had a specialized issue caused by the fact that we have to
rebuild tables and copy data in order to modify field state. Since this
process is expensive, we batch together the changes we need to make and
then generate the appropriate SQL at execution time. This process ends
up causing us to drop indexes that are no longer needed and to rebuild
the indexes we do want in the resulting table.

The common logic for adding or deleting standard indexes didn't work
well with this. We'd just output the SQL, which could result in an index
being dropped after a table rebuild (when the index no longer exists
anyway) or being added after the rebuild (when it already was added as
part of generation).

This issue only applies to db_index changes, since unique already
triggered a table rebuild.

The fix here is to have db_index changes trigger a possible rebuild as
well, instead of immediately outputting SQL. The new generation ops for
this (ADD DB INDEX and DROP DB INDEX) don't themselves trigger a
rebuild. If nothing else is being rebuilt, the appropriate SQL will be
added as normal, but in the right order. If there is a table rebuild,
any drops will take place before the rebuild, and any adds after
(technically, they'll already be added, so we just stuff the information
back into the DatabaseState for index caching).

New unit tests for the various combinations of states have been added.

Unit tests pass for all versions of Django on all supported types of
databases.

Tested along with the Review Board 4 -> 5 database upgrade, which was
hitting this bug.

Summary ID
Fix ordering and conflict issues with adding and dropping indexes.
When switching between `db_index` and `unique` states, or when dropping indexes due to `db_index=False` while performing other changes to the model, we'd run into a couple issues. We could end up trying to create duplicate indexes, drop indexes too late, or create standard *and* unique indexes in one go. There were two reasons for these problems, one common to all databases, and one SQLite-specific. The common issue was that we handled `db_index` and `unique` changes separately. If enabling `db_index` and `unique` in the same batch of changes, we could get indexes for both. If disabling both, we could get redundant drops. If switching from one to the other, we could get ordering issues or accidental drops or adds of an index. This is resolved by considering `db_index` and `unique` changes together, and ensuring that we never add or drop two types of indexes at once or put them in the wrong order. The logic here is common to all databases. SQLite had a specialized issue caused by the fact that we have to rebuild tables and copy data in order to modify field state. Since this process is expensive, we batch together the changes we need to make and then generate the appropriate SQL at execution time. This process ends up causing us to drop indexes that are no longer needed and to rebuild the indexes we do want in the resulting table. The common logic for adding or deleting standard indexes didn't work well with this. We'd just output the SQL, which could result in an index being dropped after a table rebuild (when the index no longer exists anyway) or being added after the rebuild (when it already was added as part of generation). This issue only applies to `db_index` changes, since `unique` already triggered a table rebuild. The fix here is to have `db_index` changes trigger a possible rebuild as well, instead of immediately outputting SQL. The new generation ops for this (`ADD DB INDEX` and `DROP DB INDEX`) don't themselves trigger a rebuild. If nothing else is being rebuilt, the appropriate SQL will be added as normal, but in the right order. If there is a table rebuild, any drops will take place before the rebuild, and any adds after (technically, they'll already be added, so we just stuff the information back into the `DatabaseState` for index caching). New unit tests for the various combinations of states have been added.
7778bda58df36ff38c37779e8909303e348c6baf
Description From Last Updated

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E128 continuation line under-indented for visual indent

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

flake8

chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.x (23811e5)