Opened 7 years ago
Last modified 20 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 )
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)
Change History (11)
comment:1 by , 7 years ago
| Description: | modified (diff) |
|---|---|
| Summary: | Release notes / Changelog missing information about change in duplicate pk behaviour in 2.1 → Release notes missing information about change in duplicate pk behavior in 2.1 |
comment:2 by , 7 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 , 7 years ago
| Cc: | added |
|---|
comment:4 by , 7 years ago
| Cc: | added |
|---|---|
| Description: | modified (diff) |
comment:5 by , 7 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 , 7 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 , 7 years ago
| Cc: | added |
|---|---|
| Component: | Documentation → Migrations |
| Summary: | Release notes missing information about change in duplicate pk behavior in 2.1 → Migration that switches a model to a UUID primary key fails with "duplicate column name: id" |
| Triage Stage: | Unreviewed → Accepted |
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 , 7 years ago
| Attachment: | t29790-app.zip added |
|---|
comment:8 by , 7 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.
comment:9 by , 7 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 , 20 months ago
| Cc: | added |
|---|
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?