Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#22960 closed Bug (fixed)

Initial migration is in alphabetical order causing 'multiple primary keys' error

Reported by: jamesbeith Owned by: andrewgodwin
Component: Migrations Version: 1.7-rc-1
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When the initial migration for an app is created the create models operations are ordered in alphabetical order. If a class has its primary key set to an alphabetically preceding class then an add field operation is included and when the migrate is run an error occurs.

Create an app with the following models.

class Company(models.Model):
    pass

class Brand(models.Model):
    company = models.OneToOneField(Company, primary_key=True)

Run makemigrations which creates an initial migration for the app with the following operations. The CreateModel operations are ordered in alphabetical order and then a subsequent AddField operation is used to add the primary key.

operations = [
    migrations.CreateModel(
        name='Brand',
        fields=[
        ],
        options={
        },
        bases=(models.Model,),
    ),
    migrations.CreateModel(
        name='Company',
        fields=[
            ('id', models.AutoField(serialize=False, auto_created=True, primary_key=True, verbose_name='ID')),
        ],
        options={
        },
        bases=(models.Model,),
    ),
    migrations.AddField(
        model_name='brand',
        name='company',
        field=models.OneToOneField(serialize=False, primary_key=True, to='companies.Company'),
        preserve_default=True,
    ),
]

When running migrate the following error occurs

  Applying companies.0001_initial...Traceback (most recent call last):
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
psycopg2.ProgrammingError: multiple primary keys for table "companies_brand" are not allowed


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/James/.virtualenvs/dts-migrations/bin/django-admin.py", line 5, in <module>
    management.execute_from_command_line()
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/core/management/base.py", line 337, in execute
    output = self.handle(*args, **options)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/core/management/commands/migrate.py", line 160, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/migrations/executor.py", line 62, in migrate
    self.apply_migration(migration, fake=fake)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/migrations/executor.py", line 96, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/migrations/migration.py", line 107, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/migrations/operations/fields.py", line 37, in database_forwards
    field,
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/backends/schema.py", line 409, in add_field
    self.execute(sql, params)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/backends/schema.py", line 98, in execute
    cursor.execute(sql, params)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/backends/utils.py", line 81, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/utils/six.py", line 549, in reraise
    raise value.with_traceback(tb)
  File "/Users/James/.virtualenvs/dts-migrations/lib/python3.4/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: multiple primary keys for table "companies_brand" are not allowed

If however the class with the specified primary key were named lower down the alphabet then the issue does not occur. For example creating the following models.

class Company(models.Model):
    pass

class Supplier(models.Model):
    company = models.OneToOneField(Company, primary_key=True)

Then running makemigrations produces two CreateModel operations with the primary key included and not a subsequent AddField operation to do so.

operations = [
    migrations.CreateModel(
        name='Company',
        fields=[
            ('id', models.AutoField(primary_key=True, serialize=False, verbose_name='ID', auto_created=True)),
        ],
        options={
        },
        bases=(models.Model,),
    ),
    migrations.CreateModel(
        name='Supplier',
        fields=[
            ('company', models.OneToOneField(primary_key=True, to='companies.Company', serialize=False)),
        ],
        options={
        },
        bases=(models.Model,),
    ),
]

Change History (4)

comment:1 Changed 13 months ago by charettes

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

I managed to reproduce locally. This is what happens internally when migrate is called:

  1. Since no primary key field is explicitly specified for the Brand model it inherits one automatically from ModelBase.__new__;
  2. The Brand table is then created with a serial id primary key;
  3. It crashes when trying to add a another primary key.

Ideally the autodetector should be able to reorder models automatically in this case. I guess this should be done around here by providing a sorted key that wraps swappable_key_first and take into account related primary keys.

I guess it wouldn't hurt to also add a check in generate_created_models to make sure we don't move a related primary key field from the CreateModel to a AddField operation.

comment:2 Changed 13 months ago by andrewgodwin

  • Owner changed from nobody to andrewgodwin
  • Status changed from new to assigned

So the problem here is that all related fields are deferred to be added after the model is created, and the autodetector is built around this assumption as part of the core constraints to ensure that circular dependencies aren't possible. However, in the case of primary keys you can't have

Merely ordering the models by relations would not be enough; the only solution to this would be to implement a full topographical ordering on the relationship tree, which is actually impossible in Django as circular references are possible. The solution we have implemented instead is an internal dependency system on the individual operations, which ensures that things are pulled around to have correct ordering.

The approach I'm going to take here is to force-bake the primary key field into the CreateModel always, which should then interact with the dependency resolver to make things happen.

comment:3 Changed 13 months ago by Andrew Godwin <andrew@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 80a12f21e30149529d831029f3cebc5728092aad:

Fixed #22960: Bad handling of relations as PKs in autodetector

comment:4 Changed 13 months ago by Andrew Godwin <andrew@…>

In 1f889800d6c3c2989c0f8c12c631967e8e3ce428:

[1.7.x] Fixed #22960: Bad handling of relations as PKs in autodetector

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