[PATCH 2/4] migrations: 0043_merge_patch_submission: do less work
Stephen Finucane
stephen at that.guru
Fri Aug 13 02:51:46 AEST 2021
On Sat, 2021-07-17 at 03:19 +1000, Daniel Axtens wrote:
> - we do a lot of legwork to prevent there being a column with the same
> name in a base class and subclass. This is purely for django's benefit
> as databases don't see the class relationship and handle column names
> being the same across tables just fine. Unfortunately renaming columns
> in MySQL/MariaDB is _extremely_ expensive. So just don't do it: lie to
> django about the names of the duplicate columns.
>
> - This means we don't have to worry about deleting the index and the
> unique_together constraints. Let deleting the table do all of that at
> once.
>
> - I don't think we gain anything from deleting foreign keys out of the
> patch table before deleting it. I initially thought this might relate
> to an ON DELETE CASCADE, but the ON DELETE rules in the patches table
> relate to what happens to the row in the patches tables if you delete
> an element from the associated table, not the other way around.
>
> Signed-off-by: Daniel Axtens <dja at axtens.net>
This seems very sensible and I see no reason not to apply it. I've held off
though because your reply to Stewart down thread suggests you might want to
rework this? If not, feel free to apply this (add a 'Related: #420' trailer to
the commit message) and backport ('git cherry-pick -x') back to stable/3.0, or
let me know and I'll do it.
Reviewed-by: Stephen Finucane <stephen at that.guru>
Thanks!
Stephen
> ---
> .../migrations/0043_merge_patch_submission.py | 140 +++++++++---------
> 1 file changed, 68 insertions(+), 72 deletions(-)
>
> diff --git a/patchwork/migrations/0043_merge_patch_submission.py b/patchwork/migrations/0043_merge_patch_submission.py
> index d351892ef7f2..465e527812ba 100644
> --- a/patchwork/migrations/0043_merge_patch_submission.py
> +++ b/patchwork/migrations/0043_merge_patch_submission.py
> @@ -10,16 +10,16 @@ def migrate_data(apps, schema_editor):
> 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 = patchwork_patch.hash2,
> - number = patchwork_patch.number2,
> - pull_url = patchwork_patch.pull_url2,
> - related_id = patchwork_patch.related2_id,
> - series_id = patchwork_patch.series2_id,
> - state_id = patchwork_patch.state2_id
> + SET archived = patchwork_patch.archived,
> + commit_ref = patchwork_patch.commit_ref,
> + delegate_id = patchwork_patch.delegate_id,
> + diff = patchwork_patch.diff,
> + hash = patchwork_patch.hash,
> + number = patchwork_patch.number,
> + pull_url = patchwork_patch.pull_url,
> + related_id = patchwork_patch.related_id,
> + series_id = patchwork_patch.series_id,
> + state_id = patchwork_patch.state_id
> FROM patchwork_patch
> WHERE patchwork_submission.id = patchwork_patch.submission_ptr_id
> """
> @@ -28,16 +28,16 @@ def migrate_data(apps, schema_editor):
> schema_editor.execute(
> """
> UPDATE patchwork_submission, patchwork_patch
> - SET patchwork_submission.archived = patchwork_patch.archived2,
> - patchwork_submission.commit_ref = patchwork_patch.commit_ref2,
> - patchwork_submission.delegate_id = patchwork_patch.delegate2_id,
> - patchwork_submission.diff = patchwork_patch.diff2,
> - patchwork_submission.hash = patchwork_patch.hash2,
> - patchwork_submission.number = patchwork_patch.number2,
> - patchwork_submission.pull_url = patchwork_patch.pull_url2,
> - patchwork_submission.related_id = patchwork_patch.related2_id,
> - patchwork_submission.series_id = patchwork_patch.series2_id,
> - patchwork_submission.state_id = patchwork_patch.state2_id
> + SET patchwork_submission.archived = patchwork_patch.archived,
> + patchwork_submission.commit_ref = patchwork_patch.commit_ref,
> + patchwork_submission.delegate_id = patchwork_patch.delegate_id,
> + patchwork_submission.diff = patchwork_patch.diff,
> + patchwork_submission.hash = patchwork_patch.hash,
> + patchwork_submission.number = patchwork_patch.number,
> + patchwork_submission.pull_url = patchwork_patch.pull_url,
> + patchwork_submission.related_id = patchwork_patch.related_id,
> + patchwork_submission.series_id = patchwork_patch.series_id,
> + patchwork_submission.state_id = patchwork_patch.state_id
> WHERE patchwork_submission.id = patchwork_patch.submission_ptr_id
> """ # noqa
> )
> @@ -50,16 +50,16 @@ def migrate_data(apps, schema_editor):
> pull_url, related_id, series_id, state_id
> ) = (
> SELECT
> - patchwork_patch.archived2,
> - patchwork_patch.commit_ref2,
> - patchwork_patch.delegate2_id,
> - patchwork_patch.diff2,
> - patchwork_patch.hash2,
> - patchwork_patch.number2,
> - patchwork_patch.pull_url2,
> - patchwork_patch.related2_id,
> - patchwork_patch.series2_id,
> - patchwork_patch.state2_id
> + patchwork_patch.archived,
> + patchwork_patch.commit_ref,
> + patchwork_patch.delegate_id,
> + patchwork_patch.diff,
> + patchwork_patch.hash,
> + patchwork_patch.number,
> + patchwork_patch.pull_url,
> + patchwork_patch.related_id,
> + patchwork_patch.series_id,
> + patchwork_patch.state_id
> FROM patchwork_patch
> WHERE patchwork_patch.submission_ptr_id = patchwork_submission.id
> )
> @@ -152,39 +152,44 @@ class Migration(migrations.Migration):
> # rename all the fields on 'Patch' so we don't have duplicates when we
> # add them to 'Submission'
>
> - migrations.RemoveIndex(
> - model_name='patch', name='patch_list_covering_idx',
> - ),
> - migrations.AlterUniqueTogether(name='patch', unique_together=set([]),),
> - migrations.RenameField(
> - model_name='patch', old_name='archived', new_name='archived2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='commit_ref', new_name='commit_ref2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='delegate', new_name='delegate2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='diff', new_name='diff2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='hash', new_name='hash2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='number', new_name='number2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='pull_url', new_name='pull_url2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='related', new_name='related2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='series', new_name='series2',
> - ),
> - migrations.RenameField(
> - model_name='patch', old_name='state', new_name='state2',
> + # Again we only to rename these fields in the django view of the world (state)
> + # to avoid errors about the same fields being in the base class (Submission)
> + # and a subclass (Patch). The db doesn't care, and we migrate the data in raw
> + # SQL, so basically just lie to django here.
> + migrations.SeparateDatabaseAndState(
> + database_operations=[],
> + state_operations=[
> + migrations.RenameField(
> + model_name='patch', old_name='archived', new_name='archived2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='commit_ref', new_name='commit_ref2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='delegate', new_name='delegate2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='diff', new_name='diff2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='hash', new_name='hash2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='number', new_name='number2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='pull_url', new_name='pull_url2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='related', new_name='related2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='series', new_name='series2',
> + ),
> + migrations.RenameField(
> + model_name='patch', old_name='state', new_name='state2',
> + ),
> + ],
> ),
>
> # add the fields found on 'Patch' to 'Submission'
> @@ -306,15 +311,6 @@ class Migration(migrations.Migration):
> ),
> ),
>
> - # remove the foreign key fields from the 'Patch' model
> -
> - migrations.RemoveField(model_name='patch', name='delegate2',),
> - migrations.RemoveField(model_name='patch', name='patch_project',),
> - migrations.RemoveField(model_name='patch', name='related2',),
> - migrations.RemoveField(model_name='patch', name='series2',),
> - migrations.RemoveField(model_name='patch', name='state2',),
> - migrations.RemoveField(model_name='patch', name='submission_ptr',),
> -
> # drop the 'Patch' model and rename 'Submission' to 'Patch'
>
> migrations.DeleteModel(name='Patch',),
More information about the Patchwork
mailing list