Opened 6 years ago

Last modified 8 months ago

#29790 new Bug

Migration that switches a model to a UUID primary key fails with "duplicate column name: id"

Reported by: Richard Ebeling Owned by: nobody
Component: Migrations Version: 2.1
Severity: Normal Keywords: multiple primary keys migration id uuid
Cc: karyon, Richard Ebeling, Tim Martin, Ülgen Sarıkavak Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Richard Ebeling)

We migrated a model from a AutoField as id (default behaviour) to a UUIDField in the past, using a migration that looked like this with django 2.0:
https://gist.github.com/smcoll/8bb867dc631433c01fd0

There was a bug in SQLite though, which as of today only has the workaround listed in the bug tracker: #28541

Thus, we ended up writing our migration like this: https://github.com/fsr-itse/EvaP/blob/master/evap/evaluation/migrations/0062_replace_textanswer_id_with_uuid.py

When we try updating to django 2.1 now, this migration fails whenever you call migrate with the error
django.db.utils.ProgrammingError: multiple primary keys for table "evaluation_textanswer" are not allowed

This commit changed the behavior: 02365d3f38a64a5c2f3e932f23925a381d5bb151

It works if we add a RemoveField instruction for the old id before setting primary_key=True on the new UUIDField. But this will trigger the SQLite bug listed above. Changing the old AutoField to primary_key=False where the RemoveField instruction would be doesn't fix the error.

The documentation states that an object may not have more than one primary key.

Still, in the release notes for django 2.1 it should be noted that django's behavior was changed here. Also, I would appreciate if there was some kind of documentation on how you should migrate primary keys from a field to another if no two primary keys are allowed in between. There should be a way without squashing the migrations as we want to keep the history.

We are running postgres version 9.6.9 and psycopg2-binary version 2.7.5.
The migration error will not occur when using a SQLite database.

Attachments (1)

t29790-app.zip (3.2 KB ) - added by Tim Graham 6 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 by Tim Graham, 6 years ago

Description: modified (diff)
Summary: Release notes / Changelog missing information about change in duplicate pk behaviour in 2.1Release notes missing information about change in duplicate pk behavior in 2.1

It's unclear if the behavior change was intentional. Can you provide a sample project and bisect to find the commit where the behavior changed?

comment:2 by Richard Ebeling, 6 years ago

Description: modified (diff)

This commit changed the behavior: 02365d3f38a64a5c2f3e932f23925a381d5bb151

As a sample project I can link the project we are currently working on, https://github.com/fsr-itse/EvaP/tree/release. A simple ./manage.py reset_db --noinput && ./manage.py migrate will trigger the error. Is this what you wanted?

comment:3 by karyon, 6 years ago

Cc: karyon added

comment:4 by Richard Ebeling, 6 years ago

Cc: Richard Ebeling added
Description: modified (diff)

comment:5 by Tim Graham, 6 years ago

Perhaps I've made a mistake, but I tried the sample project and don't see the crash when running migrations. I tried the "master" and "release" branches and modified the DATABASES setting to use SQLite. A more minimal project to reproduce the problem would be nice.

comment:6 by Richard Ebeling, 6 years ago

Description: modified (diff)

I can not reproduce this bug with SQLite either, sorry for not testing that before. We are running postgres version 9.6.9 and use psycopg2-binary version 2.7.5.. I added this information to the initial post.

comment:7 by Tim Graham, 6 years ago

Cc: Tim Martin added
Component: DocumentationMigrations
Summary: Release notes missing information about change in duplicate pk behavior in 2.1Migration that switches a model to a UUID primary key fails with "duplicate column name: id"
Triage Stage: UnreviewedAccepted

I'm attaching a minimal application that reproduces the problem (with PostgreSQL). I'm not sure that this problem should be included in the release notes rather than fixed (if possible). Solving #28541 may be more worthwhile.

by Tim Graham, 6 years ago

Attachment: t29790-app.zip added

comment:8 by Christian Kreuzberger, 6 years ago

Hi!

We are also affected by this bug, see this Github issue: https://github.com/anx-ckreuzberger/django-rest-passwordreset/issues/8

I basically had a model that had this as primary key:

class ResetPasswordToken(models.Model):
    key = models.CharField(
        _("Key"),
        max_length=64,
        primary_key=True
    )

And I wanted to use a different primary key now:

class ResetPasswordToken(models.Model):
    id = models.AutoField(
        primary_key=True
    )

    # Key field, though it is not the primary key of the model
    key = models.CharField(
        _("Key"),
        max_length=64,
        db_index=True,
        unique=True
    )

which lead to the following migration (including some "self-written" runpython stuff to fill the primary keys):

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


def populate_auto_incrementing_pk_field(apps, schema_editor):
    ResetPasswordToken = apps.get_model('django_rest_passwordreset', 'ResetPasswordToken')

    # Generate values for the new id column
    for i, o in enumerate(ResetPasswordToken.objects.all()):
        o.id = i + 1
        o.save()


class Migration(migrations.Migration):

    dependencies = [
        ('django_rest_passwordreset', '0001_initial',),
    ]

    operations = [
        migrations.AddField(
            model_name='resetpasswordtoken',
            name='id',
            field=models.IntegerField(null=True),
            preserve_default=True,
        ),
        migrations.RunPython(
            populate_auto_incrementing_pk_field,
            migrations.RunPython.noop
        ),
        # add primary key information to id field
        migrations.AlterField(
            model_name='resetpasswordtoken',
            name='id',
            field=models.AutoField(primary_key=True, serialize=False)
        ),
        # remove primary key information from 'key' field
        migrations.AlterField(
            model_name='resetpasswordtoken',
            name='key',
            field=models.CharField(db_index=True, max_length=64, unique=True, verbose_name='Key'),
        ),
    ]

Unfortunately, this migration only works with Django 1.11 and 2.0, but not with the 2.1.1 (and also 2.1.2), where it spits out the following error:

  File "manage.py", line 31, in <module>
    execute_from_command_line(sys.argv)
  File "\venv\lib\site-packages\django\core\management\__init__.py", line 381, in execute_from_command_line

    utility.execute()
  File "\venv\lib\site-packages\django\core\management\__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "\venv\lib\site-packages\django\core\management\base.py", line 316, in run_from_argv
    self.execute(*args, **cmd_options)
  File "\venv\lib\site-packages\django\core\management\base.py", line 353, in execute
    output = self.handle(*args, **options)
  File "\venv\lib\site-packages\django\core\management\base.py", line 83, in wrapped
    res = handle_func(*args, **kwargs)
  File "\venv\lib\site-packages\django\core\management\commands\migrate.py", line 203, in handle
    fake_initial=fake_initial,
  File "\venv\lib\site-packages\django\db\migrations\executor.py", line 117, in migrate
    state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "\venv\lib\site-packages\django\db\migrations\executor.py", line 147, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "\venv\lib\site-packages\django\db\migrations\executor.py", line 244, in apply_migration
    state = migration.apply(state, schema_editor)
  File "\venv\lib\site-packages\django\db\migrations\migration.py", line 124, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "\venv\lib\site-packages\django\db\migrations\operations\fields.py", line 216, in database_forwards
    schema_editor.alter_field(from_model, from_field, to_field)
  File "\venv\lib\site-packages\django\db\backends\base\schema.py", line 523, in alter_field
    old_db_params, new_db_params, strict)
  File "\venv\lib\site-packages\django\db\backends\postgresql\schema.py", line 122, in _alter_field
    new_db_params, strict,
  File "\venv\lib\site-packages\django\db\backends\base\schema.py", line 719, in _alter_field
    "columns": self.quote_name(new_field.column),
  File "\venv\lib\site-packages\django\db\backends\base\schema.py", line 133, in execute
    cursor.execute(sql, params)
  File "\venv\lib\site-packages\django\db\backends\utils.py", line 100, in execute
    return super().execute(sql, params)
  File "\venv\lib\site-packages\django\db\backends\utils.py", line 68, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "\venv\lib\site-packages\django\db\backends\utils.py", line 77, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "\venv\lib\site-packages\django\db\backends\utils.py", line 85, in _execute
    return self.cursor.execute(sql, params)
  File "\venv\lib\site-packages\django\db\utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "\venv\lib\site-packages\django\db\backends\utils.py", line 85, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: Multiple primary keys for table «django_rest_passwordreset_resetpasswordtoken» are not allowed.

I thought I would be able to fix this by changing the order of creating/deleting the primary key within that migration. Now it works with Django 2.1, but not with 1.11 (and also not with 2.0), where it spits out this error:

Operations to perform:
  Apply all migrations: admin, auth, contenttypes, django_rest_passwordreset, sessions
Running migrations:
  Applying django_rest_passwordreset.0002_pk_migration...Traceback (most recent call last):
  File "/venv/lib/python3.6/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
psycopg2.ProgrammingError: multiple primary keys for table "django_rest_passwordreset_resetpasswordtoken" are not allowed

So essentially I can not have my django package compatible with Django 2.1, as I want to maintain compatibility with the 1.11 LTS.

I also took a quick look at the example of Tim Graham, this seems to be exactly the same problem.

edit: I forgot to mention: This issue does not occur when using sqlite, as all my travis-ci tests are running fine. It seems to be easy to reproduce with postgres 9.6 and psycopg2-binary 2.7.5.

Last edited 6 years ago by Christian Kreuzberger (previous) (diff)

comment:9 by Simon Charette, 6 years ago

If you want to maintain Django 1.11 support you could add a RunPython operation that does something along the following before the swapped AlterField

def delete_primary_key(apps, schema_editor):
    model = apps.get_model('django_rest_passwordreset', 'ResetPasswordToken')
    opts = model._meta
    connection = schema_editor.connection
    # Retrieve primary key constraint.
    with connection.cursor() as cursor:
        constraints = connection.introspection.get_constraints(cursor, opts.db_table)
    for constraint_name, constraint in constraints.items():
        if constraint.get('primary_key'):
            break
    else:
        raise Exception('Missing primary key constraint')
    # Delete primary key constraint
    quote_name = connection.ops.quote_name
    schema_editor.execute(schema_editor.sql_delete_pk.format(
        table_name=quote_name(table_name),
        name=quote_name(constraint_name),
    ))
   

comment:10 by Ülgen Sarıkavak, 8 months ago

Cc: Ülgen Sarıkavak added
Note: See TracTickets for help on using tickets.
Back to Top