#33932 closed Bug (fixed)
Altering AutoFields with renaming a column crashes on PostgreSQL.
Reported by: | Benoît Vinot | Owned by: | Benoît Vinot |
---|---|---|---|
Component: | Migrations | Version: | 4.1 |
Severity: | Release blocker | Keywords: | |
Cc: | Florian Apolloner | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hi, here is my problem. I have an independent table with a id
primary key. I would like this table to inherit from a new parent table. I have generated migrations which work with Django version 4.0.7 but fail with Django 4.1.
I have renamed the field id
into base_ptr
(Base
is the new parent table) and then, in another migration file, I try to make this column to be a 1 to 1 column. To do that, I have in my migration an operation like this
migrations.AlterField( model_name="son", name="base_ptr", field=models.OneToOneField( auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to="app.Base", ), ),
which leads to this SQL code
BEGIN; -- -- Alter field base_ptr on son -- ALTER TABLE "app_son" RENAME COLUMN "base_ptr" TO "base_ptr_id"; ALTER TABLE "app_son" ALTER COLUMN "base_ptr" DROP IDENTITY IF EXISTS; ALTER TABLE "app_son" ALTER COLUMN "base_ptr_id" TYPE integer; DROP SEQUENCE IF EXISTS "app_son_base_ptr_id_seq" CASCADE; ALTER TABLE "app_son" ADD CONSTRAINT "app_son_base_ptr_id_d3673d48_fk_app_base" FOREIGN KEY ("base_ptr_id") REFERENCES "app_base" ("id") DEFERRABLE INITIALLY DEFERRED; --
This SQL code fails because the second line (DROP IDENTITY EXISTS
) references the base_ptr
column which has been renamed into base_ptr_id
by the first line...
Attachments (3)
Change History (16)
comment:1 by , 2 years ago
comment:2 by , 2 years ago
Cc: | added |
---|---|
Severity: | Normal → Release blocker |
Summary: | Error in migration with version 4.1 → Altering AutoFields with renaming a column crashes on PostgreSQL. |
Triage Stage: | Unreviewed → Accepted |
Regression in 2eea361eff58dd98c409c5227064b901f41bd0d6. We should use the new column name when dropping an identity:
-
django/db/backends/postgresql/schema.py
diff --git a/django/db/backends/postgresql/schema.py b/django/db/backends/postgresql/schema.py index 633146a80b..cfc30516e6 100644
a b class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): 175 175 self.sql_drop_indentity 176 176 % { 177 177 "table": self.quote_name(table), 178 "column": self.quote_name(strip_quotes( old_field.column)),178 "column": self.quote_name(strip_quotes(new_field.column)), 179 179 } 180 180 ) 181 181 column = strip_quotes(new_field.column)
Benoît, would you like to prepare a patch? (tests can be tricky.)
by , 2 years ago
Attachment: | 0001_initial.py added |
---|
by , 2 years ago
Attachment: | 0002_rename_id_son_base_ptr.py added |
---|
by , 2 years ago
Attachment: | 0003_alter_son_base_ptr.py added |
---|
follow-up: 7 comment:3 by , 2 years ago
I have attached the three migrations files of the small example but you were too fast for me...
It would be my first contribution... I can start a PR on GitHub
comment:6 by , 2 years ago
Great!, I created a regression test inspired by your migrations:
-
tests/schema/tests.py
diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 9e2385f5d8..c5beb18304 100644
a b class SchemaTests(TransactionTestCase): 1588 1588 # OneToOneField. 1589 1589 self.assertEqual(counts, {"fks": expected_fks, "uniques": 1, "indexes": 0}) 1590 1590 1591 def test_autofield_to_o2o(self): 1592 with connection.schema_editor() as editor: 1593 editor.create_model(Author) 1594 editor.create_model(Note) 1595 1596 # Rename the field. 1597 old_field = Author._meta.get_field("id") 1598 new_field = AutoField(primary_key=True) 1599 new_field.set_attributes_from_name("note_ptr") 1600 with connection.schema_editor() as editor: 1601 editor.alter_field(Author, old_field, new_field, strict=True) 1602 # Alter AutoField to OneToOneField. 1603 new_field_o2o = OneToOneField(Note, CASCADE) 1604 new_field_o2o.set_attributes_from_name("note_ptr") 1605 with connection.schema_editor() as editor: 1606 editor.alter_field(Author, new_field, new_field_o2o, strict=True) 1607 columns = self.column_classes(Author) 1608 field_type, _ = columns["note_ptr_id"] 1609 self.assertEqual( 1610 field_type, connection.features.introspected_field_types["IntegerField"] 1611 ) 1612 1591 1613 def test_alter_field_fk_keeps_index(self): 1592 1614 with connection.schema_editor() as editor: 1593 1615 editor.create_model(Author)
comment:7 by , 2 years ago
Replying to Benoît Vinot Roseau Technologies:
It would be my first contribution... I can start a PR on GitHub
Thanks!
comment:8 by , 2 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:9 by , 2 years ago
I have created a PR here with your test and your solution. https://github.com/django/django/pull/15964
comment:10 by , 2 years ago
Has patch: | set |
---|
comment:11 by , 2 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Can you provide a sample project to reproduce? This transition is quite tricky.