Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix postgres fk rename #378

Merged
merged 3 commits into from
Dec 9, 2024
Merged

Fix postgres fk rename #378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7027aba
Update postgres drop fk template
waketzheng Dec 6, 2024
19f6e39
fix test error
waketzheng Dec 6, 2024
c10c305
docs: update changlog
waketzheng Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### [0.8.1](Unreleased)

#### Fixed
- PostgreSQL: Cannot drop constraint after deleting or rename FK on a model. (#378)
- Sort m2m fields before comparing them with diff. (#271)

### [0.8.0](../../releases/tag/v0.8.0) - 2024-12-04
Expand Down
2 changes: 1 addition & 1 deletion aerich/ddl/postgres/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class PostgresDDL(BaseDDL):
'ALTER TABLE "{table_name}" ALTER COLUMN "{column}" TYPE {datatype}{using}'
)
_SET_COMMENT_TEMPLATE = 'COMMENT ON COLUMN "{table_name}"."{column}" IS {comment}'
_DROP_FK_TEMPLATE = 'ALTER TABLE "{table_name}" DROP CONSTRAINT "{fk_name}"'
_DROP_FK_TEMPLATE = 'ALTER TABLE "{table_name}" DROP CONSTRAINT IF EXISTS "{fk_name}"'

def alter_column_null(self, model: "Type[Model]", field_describe: dict) -> str:
db_table = model._meta.db_table
Expand Down
2 changes: 1 addition & 1 deletion tests/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def default_name():
class Category(Model):
slug = fields.CharField(max_length=100)
name = fields.CharField(max_length=200, null=True, default=default_name)
user: fields.ForeignKeyRelation[User] = fields.ForeignKeyField(
owner: fields.ForeignKeyRelation[User] = fields.ForeignKeyField(
"models.User", description="User"
)
title = fields.CharField(max_length=20, unique=False)
Expand Down
28 changes: 14 additions & 14 deletions tests/test_ddl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ def test_create_table():
`name` VARCHAR(200),
`title` VARCHAR(20) NOT NULL,
`created_at` DATETIME(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6),
`user_id` INT NOT NULL COMMENT 'User',
CONSTRAINT `fk_category_user_e2e3874c` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON DELETE CASCADE
`owner_id` INT NOT NULL COMMENT 'User',
CONSTRAINT `fk_category_user_110d4c63` FOREIGN KEY (`owner_id`) REFERENCES `user` (`id`) ON DELETE CASCADE
) CHARACTER SET utf8mb4"""
)

Expand All @@ -30,7 +30,7 @@ def test_create_table():
"name" VARCHAR(200),
"title" VARCHAR(20) NOT NULL,
"created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
"user_id" INT NOT NULL REFERENCES "user" ("id") ON DELETE CASCADE /* User */
"owner_id" INT NOT NULL REFERENCES "user" ("id") ON DELETE CASCADE /* User */
)"""
)

Expand All @@ -43,9 +43,9 @@ def test_create_table():
"name" VARCHAR(200),
"title" VARCHAR(20) NOT NULL,
"created_at" TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
"user_id" INT NOT NULL REFERENCES "user" ("id") ON DELETE CASCADE
"owner_id" INT NOT NULL REFERENCES "user" ("id") ON DELETE CASCADE
);
COMMENT ON COLUMN "category"."user_id" IS 'User'"""
COMMENT ON COLUMN "category"."owner_id" IS 'User'"""
)


Expand Down Expand Up @@ -137,8 +137,8 @@ def test_set_comment():
ret = Migrate.ddl.set_comment(Category, Category._meta.fields_map.get("name").describe(False))
assert ret == 'COMMENT ON COLUMN "category"."name" IS NULL'

ret = Migrate.ddl.set_comment(Category, Category._meta.fields_map.get("user").describe(False))
assert ret == 'COMMENT ON COLUMN "category"."user_id" IS \'User\''
ret = Migrate.ddl.set_comment(Category, Category._meta.fields_map.get("owner").describe(False))
assert ret == 'COMMENT ON COLUMN "category"."owner_id" IS \'User\''


def test_drop_column():
Expand Down Expand Up @@ -181,27 +181,27 @@ def test_drop_index():

def test_add_fk():
ret = Migrate.ddl.add_fk(
Category, Category._meta.fields_map.get("user").describe(False), User.describe(False)
Category, Category._meta.fields_map.get("owner").describe(False), User.describe(False)
)
if isinstance(Migrate.ddl, MysqlDDL):
assert (
ret
== "ALTER TABLE `category` ADD CONSTRAINT `fk_category_user_e2e3874c` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON DELETE CASCADE"
== "ALTER TABLE `category` ADD CONSTRAINT `fk_category_user_110d4c63` FOREIGN KEY (`owner_id`) REFERENCES `user` (`id`) ON DELETE CASCADE"
)
else:
assert (
ret
== 'ALTER TABLE "category" ADD CONSTRAINT "fk_category_user_e2e3874c" FOREIGN KEY ("user_id") REFERENCES "user" ("id") ON DELETE CASCADE'
== 'ALTER TABLE "category" ADD CONSTRAINT "fk_category_user_110d4c63" FOREIGN KEY ("owner_id") REFERENCES "user" ("id") ON DELETE CASCADE'
)


def test_drop_fk():
ret = Migrate.ddl.drop_fk(
Category, Category._meta.fields_map.get("user").describe(False), User.describe(False)
Category, Category._meta.fields_map.get("owner").describe(False), User.describe(False)
)
if isinstance(Migrate.ddl, MysqlDDL):
assert ret == "ALTER TABLE `category` DROP FOREIGN KEY `fk_category_user_e2e3874c`"
assert ret == "ALTER TABLE `category` DROP FOREIGN KEY `fk_category_user_110d4c63`"
elif isinstance(Migrate.ddl, PostgresDDL):
assert ret == 'ALTER TABLE "category" DROP CONSTRAINT "fk_category_user_e2e3874c"'
assert ret == 'ALTER TABLE "category" DROP CONSTRAINT IF EXISTS "fk_category_user_110d4c63"'
else:
assert ret == 'ALTER TABLE "category" DROP FOREIGN KEY "fk_category_user_e2e3874c"'
assert ret == 'ALTER TABLE "category" DROP FOREIGN KEY "fk_category_user_110d4c63"'
55 changes: 22 additions & 33 deletions tests/test_migrate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from pathlib import Path
from typing import List, cast

import pytest
import tortoise
Expand All @@ -12,6 +11,9 @@
from aerich.migrate import MIGRATE_TEMPLATE, Migrate
from aerich.utils import get_models_describe

# tortoise-orm>=0.21 changes IntField constraints
# from {"ge": 1, "le": 2147483647} to {"ge": -2147483648, "le": 2147483647}
MIN_INT = 1 if tortoise.__version__ < "0.21" else -2147483648
old_models_describe = {
"models.Category": {
"name": "models.Category",
Expand All @@ -34,7 +36,7 @@
"default": None,
"description": None,
"docstring": None,
"constraints": {"ge": 1, "le": 2147483647},
"constraints": {"ge": MIN_INT, "le": 2147483647},
"db_field_types": {"": "INT"},
},
"data_fields": [
Expand Down Expand Up @@ -101,7 +103,7 @@
"default": None,
"description": "User",
"docstring": None,
"constraints": {"ge": 1, "le": 2147483647},
"constraints": {"ge": MIN_INT, "le": 2147483647},
"db_field_types": {"": "INT"},
},
{
Expand Down Expand Up @@ -184,7 +186,7 @@
"default": None,
"description": None,
"docstring": None,
"constraints": {"ge": 1, "le": 2147483647},
"constraints": {"ge": MIN_INT, "le": 2147483647},
"db_field_types": {"": "INT"},
},
"data_fields": [
Expand Down Expand Up @@ -291,7 +293,7 @@
"default": None,
"description": None,
"docstring": None,
"constraints": {"ge": 1, "le": 2147483647},
"constraints": {"ge": MIN_INT, "le": 2147483647},
"db_field_types": {"": "INT"},
},
"data_fields": [
Expand Down Expand Up @@ -337,7 +339,7 @@
"default": None,
"description": None,
"docstring": None,
"constraints": {"ge": 1, "le": 2147483647},
"constraints": {"ge": MIN_INT, "le": 2147483647},
"db_field_types": {"": "INT"},
},
],
Expand Down Expand Up @@ -384,7 +386,7 @@
"default": None,
"description": None,
"docstring": None,
"constraints": {"ge": 1, "le": 2147483647},
"constraints": {"ge": MIN_INT, "le": 2147483647},
"db_field_types": {"": "INT"},
},
"data_fields": [
Expand Down Expand Up @@ -578,7 +580,7 @@
"default": None,
"description": None,
"docstring": None,
"constraints": {"ge": 1, "le": 2147483647},
"constraints": {"ge": MIN_INT, "le": 2147483647},
"db_field_types": {"": "INT"},
},
"data_fields": [
Expand Down Expand Up @@ -763,7 +765,7 @@
"default": None,
"description": None,
"docstring": None,
"constraints": {"ge": 1, "le": 2147483647},
"constraints": {"ge": MIN_INT, "le": 2147483647},
"db_field_types": {"": "INT"},
},
"data_fields": [
Expand Down Expand Up @@ -822,16 +824,6 @@
}


def should_add_user_id_column_type_alter_sql() -> bool:
if tortoise.__version__ < "0.21":
return False
# tortoise-orm>=0.21 changes IntField constraints
# from {"ge": 1, "le": 2147483647} to {"ge": -2147483648, "le": 2147483647}
data_fields = cast(List[dict], old_models_describe["models.Category"]["data_fields"])
user_id_constraints = data_fields[-1]["constraints"]
return tortoise.fields.data.IntField.constraints != user_id_constraints


def test_migrate(mocker: MockerFixture):
"""
models.py diff with old_models.py
Expand All @@ -850,8 +842,9 @@ def test_migrate(mocker: MockerFixture):
- drop unique field: Config.name
- alter default: Config.status
- rename column: Product.image -> Product.pic
- rename fk column: Category.user -> Category.owner
"""
mocker.patch("asyncclick.prompt", side_effect=(True,))
mocker.patch("asyncclick.prompt", side_effect=(True, True))

models_describe = get_models_describe("models")
Migrate.app = "models"
Expand All @@ -871,14 +864,15 @@ def test_migrate(mocker: MockerFixture):
"ALTER TABLE `category` MODIFY COLUMN `name` VARCHAR(200)",
"ALTER TABLE `category` MODIFY COLUMN `slug` VARCHAR(100) NOT NULL",
"ALTER TABLE `category` DROP INDEX `title`",
"ALTER TABLE `category` RENAME COLUMN `user_id` TO `owner_id`",
"ALTER TABLE `category` ADD CONSTRAINT `fk_category_user_110d4c63` FOREIGN KEY (`owner_id`) REFERENCES `user` (`id`) ON DELETE CASCADE",
"ALTER TABLE `config` DROP COLUMN `name`",
"ALTER TABLE `config` DROP INDEX `name`",
"ALTER TABLE `config` ADD `user_id` INT NOT NULL COMMENT 'User'",
"ALTER TABLE `config` ADD CONSTRAINT `fk_config_user_17daa970` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON DELETE CASCADE",
"ALTER TABLE `config` ALTER COLUMN `status` DROP DEFAULT",
"ALTER TABLE `config` MODIFY COLUMN `value` JSON NOT NULL",
"ALTER TABLE `email` ADD `address` VARCHAR(200) NOT NULL",
"ALTER TABLE `email` DROP COLUMN `user_id`",
"ALTER TABLE `configs` RENAME TO `config`",
"ALTER TABLE `product` DROP COLUMN `uuid`",
"ALTER TABLE `product` DROP INDEX `uuid`",
Expand Down Expand Up @@ -909,9 +903,10 @@ def test_migrate(mocker: MockerFixture):
"ALTER TABLE `category` MODIFY COLUMN `name` VARCHAR(200) NOT NULL",
"ALTER TABLE `category` MODIFY COLUMN `slug` VARCHAR(200) NOT NULL",
"ALTER TABLE `category` ADD UNIQUE INDEX `title` (`title`)",
"ALTER TABLE `category` RENAME COLUMN `owner_id` TO `user_id`",
"ALTER TABLE `category` DROP FOREIGN KEY `fk_category_user_110d4c63`",
"ALTER TABLE `config` ADD `name` VARCHAR(100) NOT NULL UNIQUE",
"ALTER TABLE `config` ADD UNIQUE INDEX `name` (`name`)",
"ALTER TABLE `config` DROP COLUMN `user_id`",
"ALTER TABLE `config` DROP FOREIGN KEY `fk_config_user_17daa970`",
"ALTER TABLE `config` ALTER COLUMN `status` SET DEFAULT 1",
"ALTER TABLE `email` ADD `user_id` INT NOT NULL",
Expand Down Expand Up @@ -943,10 +938,6 @@ def test_migrate(mocker: MockerFixture):
"ALTER TABLE `product` MODIFY COLUMN `body` LONGTEXT NOT NULL",
"ALTER TABLE `email` MODIFY COLUMN `is_primary` BOOL NOT NULL DEFAULT 0",
}
if should_add_user_id_column_type_alter_sql():
sql = "ALTER TABLE `category` MODIFY COLUMN `user_id` INT NOT NULL COMMENT 'User'"
expected_upgrade_operators.add(sql)
expected_downgrade_operators.add(sql)
assert not set(Migrate.upgrade_operators).symmetric_difference(expected_upgrade_operators)

assert not set(Migrate.downgrade_operators).symmetric_difference(
Expand All @@ -959,6 +950,8 @@ def test_migrate(mocker: MockerFixture):
'ALTER TABLE "category" ALTER COLUMN "name" DROP NOT NULL',
'ALTER TABLE "category" ALTER COLUMN "slug" TYPE VARCHAR(100) USING "slug"::VARCHAR(100)',
'ALTER TABLE "category" ALTER COLUMN "created_at" TYPE TIMESTAMPTZ USING "created_at"::TIMESTAMPTZ',
'ALTER TABLE "category" RENAME COLUMN "user_id" TO "owner_id"',
'ALTER TABLE "category" ADD CONSTRAINT "fk_category_user_110d4c63" FOREIGN KEY ("owner_id") REFERENCES "user" ("id") ON DELETE CASCADE',
'ALTER TABLE "config" DROP COLUMN "name"',
'DROP INDEX "uid_config_name_2c83c8"',
'ALTER TABLE "config" ADD "user_id" INT NOT NULL',
Expand All @@ -967,7 +960,6 @@ def test_migrate(mocker: MockerFixture):
'ALTER TABLE "config" ALTER COLUMN "value" TYPE JSONB USING "value"::JSONB',
'ALTER TABLE "configs" RENAME TO "config"',
'ALTER TABLE "email" ADD "address" VARCHAR(200) NOT NULL',
'ALTER TABLE "email" DROP COLUMN "user_id"',
'ALTER TABLE "email" RENAME COLUMN "id" TO "email_id"',
'ALTER TABLE "email" ALTER COLUMN "is_primary" TYPE BOOL USING "is_primary"::BOOL',
'DROP INDEX "uid_product_uuid_d33c18"',
Expand Down Expand Up @@ -997,11 +989,12 @@ def test_migrate(mocker: MockerFixture):
'ALTER TABLE "category" ALTER COLUMN "name" SET NOT NULL',
'ALTER TABLE "category" ALTER COLUMN "slug" TYPE VARCHAR(200) USING "slug"::VARCHAR(200)',
'ALTER TABLE "category" ALTER COLUMN "created_at" TYPE TIMESTAMPTZ USING "created_at"::TIMESTAMPTZ',
'ALTER TABLE "category" RENAME COLUMN "owner_id" TO "user_id"',
'ALTER TABLE "category" DROP CONSTRAINT IF EXISTS "fk_category_user_110d4c63"',
'ALTER TABLE "config" ADD "name" VARCHAR(100) NOT NULL UNIQUE',
'CREATE UNIQUE INDEX "uid_config_name_2c83c8" ON "config" ("name")',
'ALTER TABLE "config" ALTER COLUMN "status" SET DEFAULT 1',
'ALTER TABLE "config" DROP COLUMN "user_id"',
'ALTER TABLE "config" DROP CONSTRAINT "fk_config_user_17daa970"',
'ALTER TABLE "config" DROP CONSTRAINT IF EXISTS "fk_config_user_17daa970"',
'ALTER TABLE "config" RENAME TO "configs"',
'ALTER TABLE "config" ALTER COLUMN "value" TYPE JSONB USING "value"::JSONB',
'ALTER TABLE "email" ADD "user_id" INT NOT NULL',
Expand Down Expand Up @@ -1030,10 +1023,6 @@ def test_migrate(mocker: MockerFixture):
'DROP TABLE IF EXISTS "email_user"',
'DROP TABLE IF EXISTS "newmodel"',
}
if should_add_user_id_column_type_alter_sql():
sql = 'ALTER TABLE "category" ALTER COLUMN "user_id" TYPE INT USING "user_id"::INT'
expected_upgrade_operators.add(sql)
expected_downgrade_operators.add(sql)
assert not set(Migrate.upgrade_operators).symmetric_difference(expected_upgrade_operators)
assert not set(Migrate.downgrade_operators).symmetric_difference(
expected_downgrade_operators
Expand Down
Loading