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 )
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 , 6 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 , 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 , 6 years ago
Cc: | added |
---|
comment:4 by , 6 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
comment:5 by , 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 , 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 , 6 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 , 6 years ago
Attachment: | t29790-app.zip added |
---|
comment:8 by , 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.
comment:9 by , 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 , 8 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?