migration that introduces uuid field is not reversible with unique=True

[karyon]

sorry for the confusing title, couldn't think of a better one...

this migration here (from ​https://github.com/fsr-itse/EvaP/pull/1002/files)

     # -*- coding: utf-8 -*-
# Generated by Django 1.11.3 on 2017-07-03 18:31
from __future__ import unicode_literals

from django.db import migrations, models
import uuid

def fill_textanswer_uuid(apps, schema_editor):
    db_alias = schema_editor.connection.alias
    TextAnswer = apps.get_model('evaluation', 'TextAnswer')
    for obj in TextAnswer.objects.using(db_alias).all():
        obj.uuid = uuid.uuid4()
        obj.save()

class Migration(migrations.Migration):
    """ this migration changes a model from a auto-generated id field to a uuid-primary key. """

    dependencies = [
        ('evaluation', '0059_add_yes_no_questions'),
    ]

    # Based on
    # https://gist.github.com/smcoll/8bb867dc631433c01fd0

    operations = [
        migrations.AddField(
            model_name='textanswer',
            name='uuid',
            field=models.UUIDField(null=True),
        ),
        migrations.RunPython(fill_textanswer_uuid, migrations.RunPython.noop),
        migrations.AlterField(
            model_name='textanswer',
            name='uuid',
            field=models.UUIDField(primary_key=False, default=uuid.uuid4, serialize=False, editable=False), # add 'unique=True' here
        ),
        migrations.RemoveField('TextAnswer', 'id'),
        migrations.RenameField(
            model_name='textanswer',
            old_name='uuid',
            new_name='id'
        ),
        migrations.AlterField(
            model_name='textanswer',
            name='id',
            field=models.UUIDField(primary_key=True, default=uuid.uuid4, serialize=False, editable=False),
        ),
    ]

runs fine forwards and backwards.

adding unique=True in the line indicated above makes the backward direction fail, although it should intuitively have little to no effect. especially the error multiple primary keys for table "evaluation_textanswer" are not allowed doesn't make much sense to me. traceback:

Traceback (most recent call last):
  File "/home/vagrant/.local/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 "evaluation_textanswer" are not allowed
 
 
The above exception was the direct cause of the following exception:
 
Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/core/management/__init__.py", line 363, in execute_from_command_line
    utility.execute()
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/core/management/__init__.py", line 355, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/core/management/base.py", line 283, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/core/management/base.py", line 330, in execute
    output = self.handle(*args, **options)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/core/management/commands/migrate.py", line 204, in handle
    fake_initial=fake_initial,
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/migrations/executor.py", line 119, in migrate
    state = self._migrate_all_backwards(plan, full_plan, fake=fake)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/migrations/executor.py", line 194, in _migrate_all_backwards
    self.unapply_migration(states[migration], migration, fake=fake)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/migrations/executor.py", line 264, in unapply_migration
    state = migration.unapply(state, schema_editor)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/migrations/migration.py", line 178, in unapply
    operation.database_backwards(self.app_label, schema_editor, from_state, to_state)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/migrations/operations/fields.py", line 160, in database_backwards
    schema_editor.add_field(from_model, to_model._meta.get_field(self.name))
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/backends/base/schema.py", line 429, in add_field
    self.execute(sql, params)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/backends/base/schema.py", line 120, in execute
    cursor.execute(sql, params)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/backends/utils.py", line 80, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/vagrant/.local/lib/python3.4/site-packages/django/utils/six.py", line 685, in reraise
    raise value.with_traceback(tb)
  File "/home/vagrant/.local/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 "evaluation_textanswer" are not allowed

console output including sql statements when running the migration backwards without the unique=True: ​https://pastebin.com/gs2SaBjt
with unique=True: ​https://pastebin.com/QUebX0aa

check them out, the sql is vastly different. i haven't investigated further what the problem is.

[karyon]

same with current master.

[Tim Martin]

I can't reproduce this using django branch stable/1.11.x or master. With sqlite, the migration fails to apply forwards, with the error:

django.db.utils.OperationalError: duplicate column name: id

AFAICT, this is hitting after it has deleted the id field. I think this is happening because in sqlite you can't actually remove the id field from a table?

With Postgres 9.6.5 and Django stable/1.11.x I can apply the migration forwards and backwards without a problem.

Can you provide more details about the environment where this is failing? Perhaps the output of pip freeze so I can get the same versions of psycopg2 etc.

[karyon]

the sqlite failure is described here: https://code.djangoproject.com/ticket/28541

have you added unique=True to the migration? without that the error does not appear.

i'll experiment with psycop versions and will provide a pip freeze tomorrow.

[Tim Martin]

You're right, I assumed that the code you gave was the reproduction case and I didn't see that the unique=True is commented out. I can confirm that I can reproduce the problem now, with the master branch and with stable branches back to 1.8. I'll investigate further.

[karyon]

sorry for the confusion!

[Tim Martin]

I've got a theory about this now. When a primary key constraint is removed and the target field state is unique, then the primary key constraint is deleted along with all other unique constraints on the field. This happens in BaseDatabaseSchemaEditor._alterField, in the second conditional block. But I think this masks the fact that the primary key isn't explicitly removed. If the removal isn't triggered by the removal of uniqueness, then it doesn't happen.

I've created a branch with a UT that reproduces this case ​here

[Tim Martin]

​PR

[Tim Martin]

Thanks for the review. I've addressed your comments on the pull request. For the failure on Oracle, it seemed that the problem was just that the primary key needs to be deleted before the unique key is created.

[Tim Graham <timograham@…>]

In 02365d3:

Fixed #28542 -- Fixed deletion of primary key constraint if the new field is unique.