Skip to content

fix(migrations): create index for fields added with db_index=True - #2384

Open
nk-batman21 wants to merge 1 commit into
tortoise:developfrom
nk-batman21:fix-addfield-db-index
Open

nk-batman21 wants to merge 1 commit into
tortoise:developfrom
nk-batman21:fix-addfield-db-index

Conversation

@nk-batman21

Copy link
Copy Markdown

Description

AddField only runs ALTER TABLE ... ADD COLUMN, so a field added to an existing model with db_index=True never gets its index. CreateModel and AlterField both create it.

To hit it, add email = fields.CharField(max_length=200, null=True, db_index=True) to an existing User model and run makemigrations and migrate. sqlmigrate shows only ALTER TABLE "user" ADD COLUMN "email" VARCHAR(200);, the table has no index afterwards, and the next makemigrations reports no changes because the migration state already has db_index=True.

add_field in the base and SQLite schema editors now calls add_index after adding the column, as _alter_field does. The index gets the same idx_... name as in CreateModel, so a later AlterField to db_index=False drops it. For a ForeignKeyField the index goes on its column (e.g. writer_id).

Some existing migrations also add an unnamed single-column Index on such a column: from listing it in Meta.indexes as a workaround, or the AddIndex the autodetector used to emit when recreating a generated field (no longer emitted). That index has the same name, so AddIndex and RemoveIndex now skip it, also on downgrade, the way CreateModel already merges the two. Without this those migrations would fail on a fresh database with index ... already exists. Named, typed, partial and multi-column indexes are unchanged. Two limits: a database that applied the AddField before this change keeps missing the index, and on MSSQL rolling back such an AddField now hits the existing DROP COLUMN limit for indexed columns, as with CreateModel.

Motivation and Context

Adding an indexed column to an existing table is a routine change. Today the migration applies cleanly and the index is silently missing, on every backend.

How Has This Been Tested?

New tests for the base editor (plain field and ForeignKeyField with source_field), the SQLite editor, and AddField followed by AddIndex/RemoveIndex on the same column. The generated-field recreate test now expects only the GIN AddIndex. All four fail without the change.

Ran: pytest tests/migrations and the SQLite test suite
Ran: the CLI steps above on SQLite, plus db_index=True together with Meta.indexes = [("email",)] in the same migration and in a later one: one index each time, downgrades and removing the Meta.indexes entry work
Ran: the comprehensive migrations example (migrate, downgrade erp)
Checked: ruff format --check, ruff check, mypy

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added the changelog accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

AddField only emitted ALTER TABLE ... ADD COLUMN, so a field added to an
existing model with db_index=True never got its index, while CreateModel
and AlterField both create it. add_field now calls add_index after adding
the column, as _alter_field does.

A plain single-column index on a db_index field has the same name as the
field's own index, so AddIndex and RemoveIndex now skip it, the way
CreateModel already merges the two. Existing migrations that pair such an
AddField with an AddIndex on the same column keep applying on a fresh
database.

The generated-field recreate path no longer emits its own AddIndex for the
field, since AddField now creates that index.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant