Opened 9 days ago

Last modified 17 hours ago

#35991 assigned Bug

Migrations crash on SQLite when renaming part of CompositePrimaryKey.

Reported by: Mariusz Felisiak Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: dev
Severity: Release blocker Keywords:
Cc: Csirmaz Bendegúz Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I've created a sample project that tries to rename a column included in CompositePrimaryKey. Unfortunately, it crashes:

$ python manage.py migrate test_one 0002
...
  File "/django/db/backends/base/schema.py", line 509, in create_model
    sql, params = self.table_sql(model)
                  ^^^^^^^^^^^^^^^^^^^^^
  File "/django/db/backends/base/schema.py", line 290, in table_sql
    constraint_sqls.append(self._pk_constraint_sql(pk.columns))
                                                   ^^^^^^^^^^
  File "/django/utils/functional.py", line 47, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
                                         ^^^^^^^^^^^^^^^^^^^
  File "/django/db/models/fields/composite.py", line 81, in columns
    return tuple(field.column for field in self.fields)
                                           ^^^^^^^^^^^
  File "/django/utils/functional.py", line 47, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
                                         ^^^^^^^^^^^^^^^^^^^
  File "/django/db/models/fields/composite.py", line 77, in fields
    return tuple(meta.get_field(field_name) for field_name in self.field_names)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/django/db/models/fields/composite.py", line 77, in <genexpr>
    return tuple(meta.get_field(field_name) for field_name in self.field_names)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/django/db/models/options.py", line 685, in get_field
    raise FieldDoesNotExist(
django.core.exceptions.FieldDoesNotExist: NewRelease has no field named 'name'

I've attached a sample project. I can prepare a patch in the coming days.

Attachments (1)

ticket_373_rename_column.zip (7.9 KB ) - added by Mariusz Felisiak 9 days ago.

Download all attachments as: .zip

Change History (6)

by Mariusz Felisiak, 9 days ago

comment:1 by Mariusz Felisiak, 9 days ago

I've found other migrations that crashes in exactly the same way on SQLite.

Version 0, edited 9 days ago by Mariusz Felisiak (next)

comment:2 by Simon Charette, 9 days ago

Triage Stage: UnreviewedAccepted

Thanks for the report!

The issue likely lies in ProjectState.rename_field and in MigrationAutoDetector.create_renamed_fields which shouldn't generate the AlterField against the composite primary key in the first place.

comment:3 by Mariusz Felisiak, 9 days ago

Owner: set to Mariusz Felisiak
Status: newassigned

comment:4 by Sarah Boyce, 18 hours ago

Cc: Csirmaz Bendegúz added

in reply to:  2 comment:5 by Mariusz Felisiak, 17 hours ago

Replying to Simon Charette:

Thanks for the report!

The issue likely lies in ProjectState.rename_field and in MigrationAutoDetector.create_renamed_fields which shouldn't generate the AlterField against the composite primary key in the first place.

In my case the second migration has three operations:

        migrations.RenameField(
            model_name='release',
            old_name='name',
            new_name='name2',
        ),
        migrations.AddField(
            model_name='release',
            name='rename',
            field=models.CharField(default='x', max_length=20),
            preserve_default=False,
        ),
        migrations.AlterField(
            model_name='release',
            name='pk',
            field=models.CompositePrimaryKey('version', 'rename', blank=True, editable=False, primary_key=True, serialize=False),
        ),

AddField crashes like in the ticket description, not AlterField, at least on SQLite where it tries to remake a table. I think we should update field references for CompositePrimaryKey before remaking a table. The following draft patch fixes this crash for me:

  • django/db/migrations/state.py

    diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py
    index 6cf675d0e4..9a4549f1f1 100644
    a b from django.core.exceptions import FieldDoesNotExist  
    1111from django.db import models
    1212from django.db.migrations.utils import field_is_referenced, get_references
    1313from django.db.models import NOT_PROVIDED
     14from django.db.models.fields.composite import CompositePrimaryKey
    1415from django.db.models.fields.related import RECURSIVE_RELATIONSHIP_CONSTANT
    1516from django.db.models.options import DEFAULT_NAMES, normalize_together
    1617from django.db.models.utils import make_model_tuple
    class ProjectState:  
    355356                    field = to_model[model_key].pop(old_name_lower)
    356357                    field.name = new_name_lower
    357358                    to_model[model_key][new_name_lower] = field
     359        # Fix field references in a composite primary key.
     360        for field_name, field in model_state.fields.items():
     361            if isinstance(field, CompositePrimaryKey):
     362                if old_name in field.field_names:
     363                    field.field_names = tuple(
     364                        new_name if sub_field_name == old_name else sub_field_name
     365                        for sub_field_name in field.field_names
     366                    )
     367
    358368        self.reload_model(*model_key, delay=delay)
    359369
    360370    def _find_reload_model(self, app_label, model_name, delay=False):

What do you think?

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