Opened 20 months ago

Closed 20 months ago

Last modified 20 months ago

#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)

0001_initial.py (733 bytes ) - added by Benoît Vinot 20 months ago.
0002_rename_id_son_base_ptr.py (342 bytes ) - added by Benoît Vinot 20 months ago.
0003_alter_son_base_ptr.py (650 bytes ) - added by Benoît Vinot 20 months ago.

Download all attachments as: .zip

Change History (16)

comment:1 by Mariusz Felisiak, 20 months ago

Can you provide a sample project to reproduce? This transition is quite tricky.

comment:2 by Mariusz Felisiak, 20 months ago

Cc: Florian Apolloner added
Severity: NormalRelease blocker
Summary: Error in migration with version 4.1Altering AutoFields with renaming a column crashes on PostgreSQL.
Triage Stage: UnreviewedAccepted

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):  
    175175                self.sql_drop_indentity
    176176                % {
    177177                    "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)),
    179179                }
    180180            )
    181181            column = strip_quotes(new_field.column)

Benoît, would you like to prepare a patch? (tests can be tricky.)

by Benoît Vinot, 20 months ago

Attachment: 0001_initial.py added

by Benoît Vinot, 20 months ago

by Benoît Vinot, 20 months ago

Attachment: 0003_alter_son_base_ptr.py added

comment:3 by Benoît Vinot, 20 months 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...

Version 0, edited 20 months ago by Benoît Vinot (next)

comment:4 by Mariusz Felisiak, 20 months ago

Can you confirm the diff works for you?

comment:5 by Benoît Vinot, 20 months ago

I confirm it works

comment:6 by Mariusz Felisiak, 20 months 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):  
    15881588        # OneToOneField.
    15891589        self.assertEqual(counts, {"fks": expected_fks, "uniques": 1, "indexes": 0})
    15901590
     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
    15911613    def test_alter_field_fk_keeps_index(self):
    15921614        with connection.schema_editor() as editor:
    15931615            editor.create_model(Author)
Last edited 20 months ago by Mariusz Felisiak (previous) (diff)

in reply to:  3 comment:7 by Mariusz Felisiak, 20 months 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 Mariusz Felisiak, 20 months ago

Owner: changed from nobody to Benoît Vinot
Status: newassigned

comment:9 by Benoît Vinot, 20 months ago

I have created a PR here with your test and your solution. https://github.com/django/django/pull/15964

comment:10 by Benoît Vinot, 20 months ago

Has patch: set

comment:11 by Mariusz Felisiak, 20 months ago

Triage Stage: AcceptedReady for checkin

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 20 months ago

Resolution: fixed
Status: assignedclosed

In e3cb8bc:

Fixed #33932 -- Fixed altering AutoFields to OneToOneField on PostgreSQL.

Regression in 2eea361eff58dd98c409c5227064b901f41bd0d6.

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 20 months ago

In 85942cf6:

[4.1.x] Fixed #33932 -- Fixed altering AutoFields to OneToOneField on PostgreSQL.

Regression in 2eea361eff58dd98c409c5227064b901f41bd0d6.

Backport of e3cb8bcb7d2a2d392e726ee1f7e32a8d9038e14c from main

Note: See TracTickets for help on using tickets.
Back to Top