• 
      

    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)