[Django] #33710: RenameIndex() crashes when unnamed index is moving backward and forward.
Django
--------------------------------------------+------------------------
Reporter: Mariusz Felisiak | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------
`RenameIndex()` should restore the old auto-generated name when an unnamed
index for `unique_together` is moving backward. Now re-applying
`RenameIndex()` crashes. For example:
{{{#!diff
diff --git a/tests/migrations/test_operations.py
b/tests/migrations/test_operations.py
index cfd28b1b39..c0a55023bb 100644
--- a/tests/migrations/test_operations.py
+++ b/tests/migrations/test_operations.py
@@ -2988,6 +2988,10 @@ class OperationTests(OperationTestBase):
with connection.schema_editor() as editor,
self.assertNumQueries(0):
operation.database_backwards(app_label, editor, new_state,
project_state)
self.assertIndexNameExists(table_name, "new_pony_test_idx")
+ # Re-apply renaming.
+ with connection.schema_editor() as editor:
+ operation.database_forwards(app_label, editor, project_state,
new_state)
+ self.assertIndexNameExists(table_name, "new_pony_test_idx")
# Deconstruction.
definition = operation.deconstruct()
self.assertEqual(definition[0], "RenameIndex")
}}}
crashes on PostgreSQL:
{{{
django.db.utils.ProgrammingError: relation "new_pony_test_idx" already
exists
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33710>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Mariusz Felisiak | Owner: David
| Wobrock
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Release blocker | 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 David Wobrock):
* owner: nobody => David Wobrock
* status: new => assigned
Comment:
I understand the issue that arises when one reverses a `RenameIndex`, but
it was made "on purpose" somehow.
In https://code.djangoproject.com/ticket/27064,
> For backwards operations with unnamed old indexes, RenameIndex is a
noop.
From my understanding, when an unnamed index becomes "named", the idea was
that it remained "named" even when reversing the operation.
I guess the implementation is not entirely correct, since it doesn't allow
idempotency of the operation when applying/un-applying it.
I'll try to find a fix
--
Ticket URL: <https://code.djangoproject.com/ticket/33710#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: David
| Wobrock
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Release blocker | 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 Mariusz Felisiak):
Replying to [comment:1 David Wobrock]:
> I understand the issue that arises when one reverses a `RenameIndex`,
but it was made "on purpose" somehow.
> In https://code.djangoproject.com/ticket/27064,
> > For backwards operations with unnamed old indexes, RenameIndex is a
noop.
>
> From my understanding, when an unnamed index becomes "named", the idea
was that it remained "named" even when reversing the operation.
Yes, sorry, I should predict that this is going to cause naming issues.
> I guess the implementation is not entirely correct, since it doesn't
allow idempotency of the operation when applying/un-applying it.
>
> I'll try to find a fix
We should be able to find the old name with
`SchemaEditor._create_index_name()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/33710#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: David
| Wobrock
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Release blocker | 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 David Wobrock):
I think making the database operation a noop when the old index has the
same name as the new index name can be enough.
Here is a first draft https://github.com/django/django/pull/15695
--
Ticket URL: <https://code.djangoproject.com/ticket/33710#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: David
| Wobrock
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* stage: Unreviewed => Ready for checkin
Comment:
Replying to [comment:3 David Wobrock]:
> I think making the database operation a noop when the old index has the
same name as the new index name can be enough.
> Here is a first draft https://github.com/django/django/pull/15695
Agreed.
--
Ticket URL: <https://code.djangoproject.com/ticket/33710#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Mariusz Felisiak | Owner: David
| Wobrock
Component: Migrations | Version: dev
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"11310e9abbdd784aea4ba50451144fc9c239f71f" 11310e9a]:
{{{
#!CommitTicketReference repository=""
revision="11310e9abbdd784aea4ba50451144fc9c239f71f"
Fixed #33710 -- Made RenameIndex operation a noop when the old and new
name match.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33710#comment:5>