[Django] #28542: migration that introduces uuid field is not reversible with unique=True
Django
--------------------------------------+------------------------
Reporter: karyon | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28542>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: karyon | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by karyon):
same with current master.
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:1>
Django
Reporter: karyon | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* cc: Tim Martin (added)
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:2>
Django
Reporter: karyon | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:3>
Django
Reporter: karyon | Owner: Tim Martin
Type: Bug | Status: assigned
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* owner: nobody => Tim Martin
* status: new => assigned
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:4>
Django
Reporter: karyon | Owner: Tim Martin
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by karyon):
sorry for the confusion!
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:5>
Django
Reporter: karyon | Owner: Tim Martin
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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
[https://github.com/timmartin/django/tree/ticket_28542 here]
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:6>
Django
Reporter: karyon | Owner: Tim Martin
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/9299 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:7>
Django
Reporter: karyon | Owner: Tim Martin
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:8>
Django
Reporter: karyon | Owner: Tim Martin
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* needs_better_patch: 1 => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:9>
Django
Reporter: karyon | Owner: Tim Martin
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by karyon):
ping - this seems to be essentially done?
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:10>
Django
Reporter: karyon | Owner: Tim Martin
Component: Migrations | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"02365d3f38a64a5c2f3e932f23925a381d5bb151" 02365d3]:
{{{
#!CommitTicketReference repository=""
revision="02365d3f38a64a5c2f3e932f23925a381d5bb151"
Fixed #28542 -- Fixed deletion of primary key constraint if the new field
is unique.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:11>