[Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

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

[Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
-----------------------------------------+------------------------
               Reporter:  Jeremy Bowman  |          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              |
-----------------------------------------+------------------------
 In #28305, a bug was fixed that prevented a MySQL field with a unique
 constraint from being altered if there was a foreign key constraint on it.
 However, the fix itself seems to have a more subtle bug in it: instead of
 dropping and recreating just foreign keys affecting that particular field,
 it does so for all foreign key constraints on any field in that table.
 This still yields a valid result, but for large tables with many incoming
 FK constraints this can dramatically increase the duration of the
 migration.

 In particular, migration 0008 from django.contrib.auth (which changes the
 length of the username, a field with a unique constraint) now causes all
 foreign keys to the user ID to be dropped and later recreated throughout
 the database.  I'm working with a MySQL database where this increases the
 duration of the migration from 10-20 minutes to nearly 48 hours, during
 most of which writes are blocked on at least one of the tables for which
 the indexes need to be recreated. Thankfully, we caught it during load
 testing with a production-sized database before attempting a production
 deployment.  This particular manifestation of the problem only hits
 installations that were populated with data using a pre-1.10 version of
 Django but are attempting to go straight to 1.11 or above (which I suspect
 is actually fairly common given that 1.8 and 1.11 are LTS versions, and
 1.10 is no longer being supported).

 The problem seems to be in the usage of `_related_non_m2m_objects` to
 enumerate the foreign key constraints; this function returns all relations
 involving the table of the field provided, not just that specific field.
 I discovered the bug in 1.11, but it still seems to be present in master.

--
Ticket URL: <https://code.djangoproject.com/ticket/29193>
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/051.343cd7cb081033db1fd04813760dd5bf%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  nobody
         Type:  Bug              |                   Status:  new
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               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 Graham):

 * severity:  Normal => Release blocker
 * stage:  Unreviewed => Accepted


Comment:

 The report seems credible, although I haven't confirmed it. The patch for
 #28305 was difficult for me. Do you have any time to work on this issue?
 We'll probably make an exception to the 1.11 support timeline and backport
 a fix to 1.11 since it's a serious performance regression.

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.48837d0900c1a44a2b8831a7d17d4d0c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  nobody
         Type:  Bug              |                   Status:  new
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               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 Jeremy Bowman):

 Sure, I'll go ahead and attempt a patch sometime in the next few days.
 I'm helping deploy the upgrade to Django 1.11 on our main service today,
 so my availability will partially depend on how smoothly that goes.

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.27e4e50ff366e97c8c8606b713a83d14%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  nobody
         Type:  Bug              |                   Status:  new
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               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 Joshua Massover):

 https://github.com/massover/bug_29193

 The above is a small project to reproduce.

 I could not reproduce it with the built in auth User model. I used a
 custom User model. While using the built in auth User model, it looked
 `_related_non_m2m_objects` did not return my related models because of
 `related_objects`.

 It looks like the code that changed the behavior is this
 https://github.com/django/django/commit/c3e0adcad8d8ba94b33cabd137056166ed36dae0
 #diff-b69190ab88f6c5737b2562d94d7bf36bR539

 {{{
         # Drop incoming FK constraints if the field is a primary key or
 unique,
         # which might be a to_field target, and things are going to
 change.
         drop_foreign_keys = (
             (
                 (old_field.primary_key and new_field.primary_key)
                 (old_field.unique and new_field.unique)
             ) and old_type != new_type
         )
 }}}

 Before it would only drop and recreate constraints if the field was a
 foreign key. Now it drops and recreates all constraints if the field is
 unique.

 Naively removing the unique check that was added in
 https://code.djangoproject.com/ticket/28305 will fix this problem. All the
 tests pass. Looking back at the ticket I don't really understand the
 intention, so this seems like an awful idea.

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.709b78a3366899aee18d6ec66b72acc5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  nobody
         Type:  Bug              |                   Status:  new
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               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 Jeremy Bowman):

 I have a tentative fix for this now, but still working on a regression
 test case.  I'm also having enough trouble getting mysqlclient working on
 my Mac that I'm probably going to switch over to a Linux VM for testing
 this (most of my regular work involving MySQL is in a Docker container
 using the non-Python-3-compatible MySQL-python).  The change itself is
 simple enough:

 {{{
 --- a/django/db/backends/base/schema.py
 +++ b/django/db/backends/base/schema.py
 @@ -18,8 +18,10 @@ def _related_non_m2m_objects(old_field, new_field):
      # Filter out m2m objects from reverse relations.
      # Return (old_relation, new_relation) tuples.
      return zip(
 -        (obj for obj in old_field.model._meta.related_objects if not
 obj.field.many_to_many),
 -        (obj for obj in new_field.model._meta.related_objects if not
 obj.field.many_to_many)
 +        (obj for obj in old_field.model._meta.related_objects
 +         if not obj.field.many_to_many and old_field.name in
 obj.field.to_fields),
 +        (obj for obj in new_field.model._meta.related_objects
 +         if not obj.field.many_to_many and new_field.name in
 obj.field.to_fields)
      )
 }}}

 This restricts the returned relations to just the ones involving the given
 field.  If anybody spots a problem with this approach while I'm getting
 the tests written, let me know.

 A quick note regarding #28305 - the problem there was that MySQL would
 fail to modify a field with a unique index if there were still any foreign
 key constraints on it from another model.  That was a legitimate issue,
 and the fix provided did solve that; it was just accidentally overzealous
 in temporarily dropping all foreign key constraints to ''any field in the
 table'' instead of just the single field being modified.  This was
 actually a problem even with the previous code that only did the FK
 dropping when changing the primary key; that just didn't cause a problem
 very often because the vast majority of FK constraints are in fact on the
 primary key, so there was rarely any collateral damage in dropping any FK
 constraints that happened to exist on other fields in the table.

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.fb1595553dc2c0e00509190cd2730146%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  nobody
         Type:  Bug              |                   Status:  new
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               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 Graham):

 You'll notice some test failures in the `migrations` app (using PostgreSQL
 or MySQL) with that patch.

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.a088c69ec73079c228cb161cedc128fd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+-----------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  Jeremy Bowman
         Type:  Bug              |                   Status:  assigned
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               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 Jeremy Bowman):

 * owner:  nobody => Jeremy Bowman
 * status:  new => assigned
 * has_patch:  0 => 1


Comment:

 I've [https://github.com/django/django/pull/9866 submitted a PR] for this;
 the problem with my first attempt above was that foreign keys which don't
 specify a target field end up with `to_fields` set to `[None]` instead of
 `['id']`.  The new version correctly handles this case, splits out the
 relevant logic into a separate function, and adds a regression test which
 counts the number of SQL statements executed by a unique field alteration
 that previously dropped and recreated an FK on the primary key of its
 table.

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.82cea8f9d82a2757dc0a73723a40cf62%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+-----------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  Jeremy Bowman
         Type:  Bug              |                   Status:  assigned
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               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 Carlton Gibson):

 * needs_better_patch:  0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.3d6fa4224cd6e3c0284f095b4cf53184%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+-----------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  Jeremy Bowman
         Type:  Bug              |                   Status:  closed
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               Resolution:  fixed
     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 <timograham@…>):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"ee17bb8a67a9e7e688da6e6f4b3be1b3a69c09b0" ee17bb8a]:
 {{{
 #!CommitTicketReference repository=""
 revision="ee17bb8a67a9e7e688da6e6f4b3be1b3a69c09b0"
 Fixed #29193 -- Prevented unnecessary foreign key drops when altering a
 unique field.

 Stopped dropping and recreating foreign key constraints on other fields
 in the same table as the one which is actually being altered in an
 AlterField operation.

 Regression in c3e0adcad8d8ba94b33cabd137056166ed36dae0.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.a8bf38668ce4e7b06eb19019d4a527a1%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+-----------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  Jeremy Bowman
         Type:  Bug              |                   Status:  closed
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               Resolution:  fixed
     Keywords:                   |             Triage Stage:  Accepted
    Has patch:  1                |      Needs documentation:  0
  Needs tests:  0                |  Patch needs improvement:  1
Easy pickings:  0                |                    UI/UX:  0
---------------------------------+-----------------------------------------

Comment (by Tim Graham <timograham@…>):

 In [changeset:"d5018abf1c4e3c68f1a825d05eb9035325707e16" d5018abf]:
 {{{
 #!CommitTicketReference repository=""
 revision="d5018abf1c4e3c68f1a825d05eb9035325707e16"
 [2.0.x] Fixed #29193 -- Prevented unnecessary foreign key drops when
 altering a unique field.

 Stopped dropping and recreating foreign key constraints on other fields
 in the same table as the one which is actually being altered in an
 AlterField operation.

 Regression in c3e0adcad8d8ba94b33cabd137056166ed36dae0.

 Backport of ee17bb8a67a9e7e688da6e6f4b3be1b3a69c09b0 from master
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.f1537ce74ae49285471acbcb97d70f48%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29193: Altering a field with a unique constraint drops and rebuilds FKs to other fields in the table

Django
In reply to this post by Django
#29193: Altering a field with a unique constraint drops and rebuilds FKs to other
fields in the table
---------------------------------+-----------------------------------------
     Reporter:  Jeremy Bowman    |                    Owner:  Jeremy Bowman
         Type:  Bug              |                   Status:  closed
    Component:  Migrations       |                  Version:  1.11
     Severity:  Release blocker  |               Resolution:  fixed
     Keywords:                   |             Triage Stage:  Accepted
    Has patch:  1                |      Needs documentation:  0
  Needs tests:  0                |  Patch needs improvement:  1
Easy pickings:  0                |                    UI/UX:  0
---------------------------------+-----------------------------------------

Comment (by Tim Graham <timograham@…>):

 In [changeset:"8f76939f54924b01b41d4242e71ca98eb35964f2" 8f76939f]:
 {{{
 #!CommitTicketReference repository=""
 revision="8f76939f54924b01b41d4242e71ca98eb35964f2"
 [1.11.x] Fixed #29193 -- Prevented unnecessary foreign key drops when
 altering a unique field.

 Stopped dropping and recreating foreign key constraints on other fields
 in the same table as the one which is actually being altered in an
 AlterField operation.

 Regression in c3e0adcad8d8ba94b33cabd137056166ed36dae0.

 Backport of ee17bb8a67a9e7e688da6e6f4b3be1b3a69c09b0 from master
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29193#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/066.d1fb5745e655f43abffecaa144e89f60%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.