fix: migration with duplicate renaming of columns in some cases

[waketzheng]

Fixes #380

TODO:

• Add unittests
• Update changelog
• Request review

Description

1. Why change _rename_new/_rename_old to _rename_fields

aerich generate migration sql mainly by this two line:

        cls.diff_models(last_version, new_version_content) # Generate upgrade SQL
        cls.diff_models(new_version_content, last_version, False) # Generate downgrade SQL

When generating the upgrade SQL, if reply true for the click.prompt, the field_name will be added to class var _rename_new (list[str]).
Then while generating downgrade SQL, Line398 will check a field_name whether in the class var to decide if it's a rename or drop/add.
This can cause a bug that: if the different models have the same new field name, and one by rename with another by drop/add, will generate two rename sql lines in downgrade, for example:

• old_models.py

from tortoise import Model, fields

class User(Model):
    username = fields.CharField(max_length=20)

class Group(Model):
    group_name = fields.CharField(max_length=20)

• new_models.py

from tortoise import Model, fields

class User(Model):
    name = fields.CharField(max_length=20)

class Group(Model):
    name = fields.CharField(max_length=20)

$ aerich migrate

Rename group_name to name? [True]: false
Rename username to name? [True]: true

$ cat migrations/models/1*

from tortoise import BaseDBAsyncClient


async def upgrade(db: BaseDBAsyncClient) -> str:
    return """
        ALTER TABLE "group" ADD "name" VARCHAR(20) NOT NULL;
        ALTER TABLE "group" DROP COLUMN "group_name";
        ALTER TABLE "user" RENAME COLUMN "username" TO "name";"""


async def downgrade(db: BaseDBAsyncClient) -> str:
    return """
        ALTER TABLE "user" RENAME COLUMN "name" TO "username";
        ALTER TABLE "group" RENAME COLUMN "name" TO "group_name";"""

You can see there are two rename lines in the downgrade!

So I use a _rename_fields: Dict[str, Dict[str, str]] = {} # {'model': {'old_field': 'new_field'}} instead, to avoid this problem.

2. How to avoid asking 4 times for 2 fields rename

Skip fields that is confirmed to rename in previous for loop.

Verify

A complex case:

python files

• settings.py

DB_URL='sqlite://db.sqlite3'
print(f'{DB_URL=}')
TORTOISE_ORM = {
    "connections": {"default": DB_URL},
    "apps": {"models": {"models": ["models", "aerich.models"]}},
}

• models.py

from tortoise import Model, fields


class A(Model):
    a = fields.BooleanField()
    b = fields.CharField(max_length=20)
    c = fields.BooleanField()


class B(Model):
    a = fields.BooleanField()
    b = fields.BooleanField()
    c = fields.CharField(max_length=20)

• new_models.py

from tortoise import Model, fields


class A(Model):
    a_2 = fields.BooleanField()
    b_2 = fields.CharField(max_length=20)
    c_2 = fields.BooleanField()


class B(Model):
    a_2 = fields.BooleanField()
    b_2 = fields.BooleanField()
    c_2 = fields.CharField(max_length=20)

Shell command

rm -rf db.sqlite* migrations/
aerich init -t settings.TORTOISE_ORM
aerich init-db
cp new_models.py models.py

1. Run aerich migrate and type 6 times enter to rename all fields:

Got migration: cat migrations/models/1*

from tortoise import BaseDBAsyncClient


async def upgrade(db: BaseDBAsyncClient) -> str:
    return """
        ALTER TABLE "a" RENAME COLUMN "a" TO "a_2";
        ALTER TABLE "a" RENAME COLUMN "b" TO "b_2";
        ALTER TABLE "a" RENAME COLUMN "c" TO "c_2";
        ALTER TABLE "b" RENAME COLUMN "a" TO "a_2";
        ALTER TABLE "b" RENAME COLUMN "b" TO "b_2";
        ALTER TABLE "b" RENAME COLUMN "c" TO "c_2";"""


async def downgrade(db: BaseDBAsyncClient) -> str:
    return """
        ALTER TABLE "a" RENAME COLUMN "b_2" TO "b";
        ALTER TABLE "a" RENAME COLUMN "a_2" TO "a";
        ALTER TABLE "a" RENAME COLUMN "c_2" TO "c";
        ALTER TABLE "b" RENAME COLUMN "b_2" TO "b";
        ALTER TABLE "b" RENAME COLUMN "a_2" TO "a";
        ALTER TABLE "b" RENAME COLUMN "c_2" TO "c";"""

2. Run aerich migrate and type some f to only rename A.a/B.a/B.b

Got migration: cat migrations/models/1*

from tortoise import BaseDBAsyncClient


async def upgrade(db: BaseDBAsyncClient) -> str:
    return """
        ALTER TABLE "a" ADD "b_2" VARCHAR(20) NOT NULL;
        ALTER TABLE "a" RENAME COLUMN "a" TO "a_2";
        ALTER TABLE "a" ADD "c_2" INT NOT NULL;
        ALTER TABLE "a" DROP COLUMN "b";
        ALTER TABLE "a" DROP COLUMN "c";
        ALTER TABLE "b" RENAME COLUMN "b" TO "b_2";
        ALTER TABLE "b" RENAME COLUMN "a" TO "a_2";
        ALTER TABLE "b" ADD "c_2" VARCHAR(20) NOT NULL;
        ALTER TABLE "b" DROP COLUMN "c";"""


async def downgrade(db: BaseDBAsyncClient) -> str:
    return """
        ALTER TABLE "a" ADD "b" VARCHAR(20) NOT NULL;
        ALTER TABLE "a" RENAME COLUMN "a_2" TO "a";
        ALTER TABLE "a" ADD "c" INT NOT NULL;
        ALTER TABLE "a" DROP COLUMN "b_2";
        ALTER TABLE "a" DROP COLUMN "c_2";
        ALTER TABLE "b" RENAME COLUMN "b_2" TO "b";
        ALTER TABLE "b" RENAME COLUMN "a_2" TO "a";
        ALTER TABLE "b" ADD "c" VARCHAR(20) NOT NULL;
        ALTER TABLE "b" DROP COLUMN "c_2";"""

[henadzit]

Could you please add explanation to the PR description about why your changes fix the issue? Having a good description of changes really helps to review the code.