[Django] #28542: migration that introduces uuid field is not reversible with unique=True

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
#28542: migration that introduces uuid field is not reversible with unique=True
--------------------------------------+------------------------
               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.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/049.f666506b38bd8f9e1086d1309797fc20%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     Reporter:  karyon      |                    Owner:  nobody
         Type:  Bug         |                   Status:  new
    Component:  Migrations  |                  Version:  1.11
     Severity:  Normal      |               Resolution:
     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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.c879041729383083e16c6e866cd4559d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     Reporter:  karyon      |                    Owner:  nobody
         Type:  Bug         |                   Status:  new
    Component:  Migrations  |                  Version:  1.11
     Severity:  Normal      |               Resolution:
     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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.5644c8117e45e7c1f4f9d861680dbb4a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     Reporter:  karyon      |                    Owner:  nobody
         Type:  Bug         |                   Status:  new
    Component:  Migrations  |                  Version:  1.11
     Severity:  Normal      |               Resolution:
     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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.4ab1ed1e6379ae3fc7d8b0fceff418e3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.d77d937c1f70428285512d4fc9fb6855%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     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
----------------------------+--------------------------------------

Comment (by karyon):

 sorry for the confusion!

--
Ticket URL: <https://code.djangoproject.com/ticket/28542#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.da98e7be064c16ff12117fc83c5d11b6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     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
----------------------------+--------------------------------------

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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.497b277694195e3a0830e8bce6a187f5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     Reporter:  karyon      |                    Owner:  Tim Martin
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  1.11
     Severity:  Normal      |               Resolution:
     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 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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.4c0883a0f62b9bdf3cea6aef91eec8f5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     Reporter:  karyon      |                    Owner:  Tim Martin
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  1.11
     Severity:  Normal      |               Resolution:
     Keywords:              |             Triage Stage:  Accepted
    Has patch:  1           |      Needs documentation:  0
  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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.cec04666199597d1c659a86e8c7c069b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     Reporter:  karyon      |                    Owner:  Tim Martin
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  1.11
     Severity:  Normal      |               Resolution:
     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 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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.db7e6a7dabc105367a31e2db9bf30db3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     Reporter:  karyon      |                    Owner:  Tim Martin
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  1.11
     Severity:  Normal      |               Resolution:
     Keywords:              |             Triage Stage:  Accepted
    Has patch:  1           |      Needs documentation:  0
  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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.e90b54d9025c8532c6540ff27007fb62%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28542: migration that introduces uuid field is not reversible with unique=True

Django
In reply to this post by Django
#28542: migration that introduces uuid field is not reversible with unique=True
----------------------------+--------------------------------------
     Reporter:  karyon      |                    Owner:  Tim Martin
         Type:  Bug         |                   Status:  closed
    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>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.dba7b2a3ebaf9b65f818691a75de3652%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.