[PATCH] migrations: Optimize 0007 patch comment for psql

[Finucane, Stephen]

On 17 Mar 16:04, Andy Doan wrote:
> By handling comment copying/deletion via SQL we can make migration take
> <3 minutes rather than >30minutes for big instances. The SQL is vendor
> specific, so a similar query would need to be added to support MySQL.
> 
> Signed-off-by: Andy Doan <andy.doan at linaro.org>

Some nits below, but nothing important so happy to merge. Any chance
you want to do the same for MySQL? :)

Reviewed-by: Stephen Finucane <stephen.finucane at intel.com>

Stephen

> ---
>  .../0007_move_comment_content_to_patch_content.py  | 89 +++++++++++++---------
>  1 file changed, 51 insertions(+), 38 deletions(-)
> 
> diff --git a/patchwork/migrations/0007_move_comment_content_to_patch_content.py b/patchwork/migrations/0007_move_comment_content_to_patch_content.py
> index 63d57ba..aea65fe 100644
> --- a/patchwork/migrations/0007_move_comment_content_to_patch_content.py
> +++ b/patchwork/migrations/0007_move_comment_content_to_patch_content.py
> @@ -1,47 +1,60 @@
>  # -*- coding: utf-8 -*-
>  from __future__ import unicode_literals
>  
> -from django.db import migrations
> -
> -
> -def copy_comment_field(apps, schema_editor):
> -    Comment = apps.get_model('patchwork', 'Comment')
> -    Patch = apps.get_model('patchwork', 'Patch')
> -
> -    for patch in Patch.objects.all():
> -        try:
> -            # when available, this can only return one entry due to the
> -            # unique_together constraint
> -            comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
> -        except Comment.DoesNotExist:
> -            # though there's no requirement to actually have a comment
> -            continue
> -
> -        patch.content = comment.content
> -        patch.save()
> +from django.db import connection, migrations
> +
> +if connection.vendor == 'postgresql'

I'd rather this check be done in each function, rather than having multiple
functions. It just seems...cleaner.

> +    def copy_comment_field(apps, schema_editor):
> +        schema_editor.execute('''
> +            UPDATE patchwork_patch
> +              SET content = patchwork_comment.content
> +            FROM patchwork_comment
> +              WHERE patchwork_patch.id=patchwork_comment.patch_id
> +                    AND patchwork_patch.msgid=patchwork_comment.msgid
> +        ''')
> +
> +    def remove_duplicate_comments(apps, schema_editor):
> +        schema_editor.execute('''
> +            DELETE FROM patchwork_comment
> +              USING patchwork_patch
> +              WHERE patchwork_patch.id=patchwork_comment.patch_id
> +                    AND patchwork_patch.msgid=patchwork_comment.msgid
> +        ''')
> +else:
> +    def copy_comment_field(apps, schema_editor):
> +        Comment = apps.get_model('patchwork', 'Comment')
> +        Patch = apps.get_model('patchwork', 'Patch')
> +
> +        for patch in Patch.objects.all():
> +            try:
> +                # when available, this can only return one entry due to the
> +                # unique_together constraint
> +                comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
> +            except Comment.DoesNotExist:
> +                # though there's no requirement to actually have a comment
> +                continue
> +
> +            patch.content = comment.content
> +            patch.save()
> +
> +    def remove_duplicate_comments(apps, schema_editor):
> +        Comment = apps.get_model('patchwork', 'Comment')
> +        Patch = apps.get_model('patchwork', 'Patch')
> +
> +        for patch in Patch.objects.all():
> +            try:
> +                # when available, this can only return one entry due to the
> +                # unique_together constraint
> +                comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
> +                comment.delete()
> +            except Comment.DoesNotExist:
> +                # though there's no requirement to actually have a comment
> +                continue
>  
>  
>  def uncopy_comment_field(apps, schema_editor):
> -    Patch = apps.get_model('patchwork', 'Patch')
> -
> -    for patch in Patch.objects.all():
> -        patch.content = None
> -        patch.save()
> -
> -
> -def remove_duplicate_comments(apps, schema_editor):
> -    Comment = apps.get_model('patchwork', 'Comment')
> -    Patch = apps.get_model('patchwork', 'Patch')
> -
> -    for patch in Patch.objects.all():
> -        try:
> -            # when available, this can only return one entry due to the
> -            # unique_together constraint
> -            comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
> -            comment.delete()
> -        except Comment.DoesNotExist:
> -            # though there's no requirement to actually have a comment
> -            continue
> +    # This is no-op because the column is being deleted
> +    pass
>  
>  
>  def recreate_comments(apps, schema_editor):

Is it possible to migrate this also? Is it worth it?

Stephen