Re: [PATCH 5/5] models: Merge 'Patch' and 'Submission'
Stephen Finucane writes: > Oh, the follies of youth. Time to undo the damage of 2.0.0, specifically > commit 86172ccc16, which split Patch into two separate models using > concrete inheritance. As noted previously, this introduced a large > number of unavoidable JOINs across large tables and the performance > impacts these introduce are blocking other features we want, such as > improved tagging functionality. To combine these two models, we must do > the following: > > - Update any references to the 'Patch' model to point to the > 'Submission' model instead > - Move everything from 'Patch' to 'Submission', including both fields > and options > - Delete the 'Patch' model > - Rename the 'Submission' model to 'Patch' > > With this change, our model "hierarchy" goes from: > > Submission > Patch > PatchComment > Cover > CoverComment > > To a nice, flat: > > Patch > PatchComment > Cover > CoverComment > > I expect this migration to be intensive, particularly for MySQL users > who will see their entire tables rewritten. Unfortunately I don't see > any way to resolve this in an easier manner. > > Also note that we have to use two migrations and separate the moving of > fields from the renaming of models, in order to work around a bug in > Django 1.11. This bug has been fixed since Django 2.0 [1][2] but the fix > wasn't backported and Django 1.11 is only accepting security fixes at > this point in time. > > [1] https://code.djangoproject.com/ticket/25530 > [2] https://code.djangoproject.com/ticket/31337 > > Signed-off-by: Stephen Finucane > Cc: Daniel Axtens > Cc: Stewart Smith > --- > TODO: > - Figure out if the indexes are correct (Stewart?). I've just included > everything that's displayed on the '/list' page. You might have more luck with stew...@flamingspork.com now that Stewart has ascended to the cloud. Regards, Daniel > - If this isn't merged until 3.0, combine the migrations since we will > no longer have to support Django 1.11. > --- > patchwork/api/comment.py | 4 +- > patchwork/management/commands/dumparchive.py | 2 +- > .../0041_merge_patch_submission_a.py | 281 ++ > .../0042_merge_patch_submission_b.py | 17 ++ > patchwork/models.py | 94 +++--- > patchwork/parser.py | 16 +- > patchwork/tests/test_mboxviews.py | 26 +- > patchwork/tests/utils.py | 5 +- > patchwork/views/__init__.py | 2 +- > patchwork/views/utils.py | 4 +- > 10 files changed, 376 insertions(+), 75 deletions(-) > create mode 100644 patchwork/migrations/0041_merge_patch_submission_a.py > create mode 100644 patchwork/migrations/0042_merge_patch_submission_b.py > > diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py > index 3802dab9..43b26c61 100644 > --- a/patchwork/api/comment.py > +++ b/patchwork/api/comment.py > @@ -14,7 +14,7 @@ from patchwork.api.base import PatchworkPermission > from patchwork.api.embedded import PersonSerializer > from patchwork.models import Cover > from patchwork.models import CoverComment > -from patchwork.models import Submission > +from patchwork.models import Patch > from patchwork.models import PatchComment > > > @@ -105,7 +105,7 @@ class PatchCommentList(ListAPIView): > lookup_url_kwarg = 'pk' > > def get_queryset(self): > -if not Submission.objects.filter(pk=self.kwargs['pk']).exists(): > +if not Patch.objects.filter(pk=self.kwargs['pk']).exists(): > raise Http404 > > return PatchComment.objects.filter( > diff --git a/patchwork/management/commands/dumparchive.py > b/patchwork/management/commands/dumparchive.py > index 9ee80c8b..e9445eab 100644 > --- a/patchwork/management/commands/dumparchive.py > +++ b/patchwork/management/commands/dumparchive.py > @@ -58,7 +58,7 @@ class Command(BaseCommand): > i + 1, len(projects), project.linkname)) > > with tempfile.NamedTemporaryFile(delete=False) as mbox: > -patches = Patch.objects.filter(patch_project=project) > +patches = Patch.objects.filter(project=project) > count = patches.count() > for j, patch in enumerate(patches): > if not (j % 10): > diff --git a/patchwork/migrations/0041_merge_patch_submission_a.py > b/patchwork/migrations/0041_merge_patch_submission_a.py > new file mode 100644 > index ..80b8dc2f > --- /dev/null > +++ b/patchwork/migrations/0041_merge_patch_submission_a.py > @@ -0,0 +1,281 @@ > +# -*- coding: utf-8 -*- > +from __future__ import unicode_literals > + > +from django.conf import settings > +from django.db import connection, migrations, models > +import django.db.models.deletion > + > +import patchwork.fields > + > + > +def migrate_data(apps, schema_editor): > +if conne
[PATCH 5/5] models: Merge 'Patch' and 'Submission'
Oh, the follies of youth. Time to undo the damage of 2.0.0, specifically commit 86172ccc16, which split Patch into two separate models using concrete inheritance. As noted previously, this introduced a large number of unavoidable JOINs across large tables and the performance impacts these introduce are blocking other features we want, such as improved tagging functionality. To combine these two models, we must do the following: - Update any references to the 'Patch' model to point to the 'Submission' model instead - Move everything from 'Patch' to 'Submission', including both fields and options - Delete the 'Patch' model - Rename the 'Submission' model to 'Patch' With this change, our model "hierarchy" goes from: Submission Patch PatchComment Cover CoverComment To a nice, flat: Patch PatchComment Cover CoverComment I expect this migration to be intensive, particularly for MySQL users who will see their entire tables rewritten. Unfortunately I don't see any way to resolve this in an easier manner. Also note that we have to use two migrations and separate the moving of fields from the renaming of models, in order to work around a bug in Django 1.11. This bug has been fixed since Django 2.0 [1][2] but the fix wasn't backported and Django 1.11 is only accepting security fixes at this point in time. [1] https://code.djangoproject.com/ticket/25530 [2] https://code.djangoproject.com/ticket/31337 Signed-off-by: Stephen Finucane Cc: Daniel Axtens Cc: Stewart Smith --- TODO: - Figure out if the indexes are correct (Stewart?). I've just included everything that's displayed on the '/list' page. - If this isn't merged until 3.0, combine the migrations since we will no longer have to support Django 1.11. --- patchwork/api/comment.py | 4 +- patchwork/management/commands/dumparchive.py | 2 +- .../0041_merge_patch_submission_a.py | 281 ++ .../0042_merge_patch_submission_b.py | 17 ++ patchwork/models.py | 94 +++--- patchwork/parser.py | 16 +- patchwork/tests/test_mboxviews.py | 26 +- patchwork/tests/utils.py | 5 +- patchwork/views/__init__.py | 2 +- patchwork/views/utils.py | 4 +- 10 files changed, 376 insertions(+), 75 deletions(-) create mode 100644 patchwork/migrations/0041_merge_patch_submission_a.py create mode 100644 patchwork/migrations/0042_merge_patch_submission_b.py diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py index 3802dab9..43b26c61 100644 --- a/patchwork/api/comment.py +++ b/patchwork/api/comment.py @@ -14,7 +14,7 @@ from patchwork.api.base import PatchworkPermission from patchwork.api.embedded import PersonSerializer from patchwork.models import Cover from patchwork.models import CoverComment -from patchwork.models import Submission +from patchwork.models import Patch from patchwork.models import PatchComment @@ -105,7 +105,7 @@ class PatchCommentList(ListAPIView): lookup_url_kwarg = 'pk' def get_queryset(self): -if not Submission.objects.filter(pk=self.kwargs['pk']).exists(): +if not Patch.objects.filter(pk=self.kwargs['pk']).exists(): raise Http404 return PatchComment.objects.filter( diff --git a/patchwork/management/commands/dumparchive.py b/patchwork/management/commands/dumparchive.py index 9ee80c8b..e9445eab 100644 --- a/patchwork/management/commands/dumparchive.py +++ b/patchwork/management/commands/dumparchive.py @@ -58,7 +58,7 @@ class Command(BaseCommand): i + 1, len(projects), project.linkname)) with tempfile.NamedTemporaryFile(delete=False) as mbox: -patches = Patch.objects.filter(patch_project=project) +patches = Patch.objects.filter(project=project) count = patches.count() for j, patch in enumerate(patches): if not (j % 10): diff --git a/patchwork/migrations/0041_merge_patch_submission_a.py b/patchwork/migrations/0041_merge_patch_submission_a.py new file mode 100644 index ..80b8dc2f --- /dev/null +++ b/patchwork/migrations/0041_merge_patch_submission_a.py @@ -0,0 +1,281 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.conf import settings +from django.db import connection, migrations, models +import django.db.models.deletion + +import patchwork.fields + + +def migrate_data(apps, schema_editor): +if connection.vendor == 'postgresql': +schema_editor.execute( +""" +UPDATE patchwork_submission + SET archived = patchwork_patch.archived2, + commit_ref = patchwork_patch.commit_ref2, + delegate_id = patchwork_patch.delegate2_id, + diff = patchwork_patch.diff2, + hash = pat
