[Django] #33953: RenameModel breaking change in 4.1 with ManyToManyField
Django
------------------------------------------+-----------------------------
Reporter: Timothy Thomas | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 4.1
Severity: Normal | Keywords: RenameModel
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+-----------------------------
Steps:
1) Create model with a ManyToManyField and a db_table in the Meta.
2) Rename model and keep db_table the same as before
The generated db schema generated from generating migrations would change
the columns on the ManyToMany relationship table to match the model name.
This matched the generated sql for interacting with this model as well.
It is possible the db_table not changing is not required.
Example:
{{{
class Publication(models.Model):
title = models.CharField(max_length=30)
class Article(models.Model):
class Meta:
db_table = "article"
headline = models.CharField(max_length=100)
publications = models.ManyToManyField(Publication)
}}}
Changed to:
{{{
class OtherArticle(models.Model):
class Meta:
db_table = "article"
headline = models.CharField(max_length=100)
publications = models.ManyToManyField(Publication)
}}}
with the corresponding migration:
{{{
operations = [
migrations.RenameModel(
old_name="Article",
new_name="OtherArticle",
),
]
}}}
Previous behavior (4.0.6):
The corresponding (functional) SQL gets generated:
{{{
'SELECT COUNT(*) AS "__count" FROM "publication" INNER JOIN
"article_publications" ON ("article"."id" =
"article_publications"."publication_id") WHERE
"article_publications"."otherarticle_id" = %s'
}}}
Current behavior (4.1):
**The same SQL gets generated but fails**. The schema that is now
generated from running the same set of migrations on a fresh db would
require:
{{{
'SELECT COUNT(*) AS "__count" FROM "publication" INNER JOIN
"article_publications" ON ("article"."id" =
"article_publications"."publication_id") WHERE
"article_publications"."article_id" = %s'
}}}
as the column now matches the old model name. However, running that SQL
would fail against our existing DB as it has ended up in a state that does
not match the behavior of the existing migrations in the current version.
--
Ticket URL: <https://code.djangoproject.com/ticket/33953>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Timothy Thomas | Owner: nobody
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: RenameModel | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* type: Uncategorized => Bug
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
This was changed in afeafd6036616bac8263d762c1610f22241c0187 for #33201,
which made RenameModel a no-op where the `db_table` is specified and
unchanged.
Something looks amiss though.
With the models you provide, creating migrations for `Article` and then
the renamed `OtherArticle`, I see the following in the shell:
{{{
>>> from app.models import Publication, OtherArticle
>>> p = Publication.objects.create(title="Testing")
>>> a = OtherArticle.objects.create(headline="Will this work?")
>>> a.publications.add(p)
Traceback (most recent call last):
File
"/Users/carlton/Projects/Django/django/django/db/backends/utils.py", line
89, in _execute
return self.cursor.execute(sql, params)
File
"/Users/carlton/Projects/Django/django/django/db/backends/sqlite3/base.py",
line 369, in execute
return Database.Cursor.execute(self, query, params)
sqlite3.OperationalError: table article_publications has no column named
otherarticle_id
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<console>", line 1, in <module>
File
"/Users/carlton/Projects/Django/django/django/db/models/fields/related_descriptors.py",
line 1114, in add
self._add_items(
...[snip]...
File
"/Users/carlton/Projects/Django/django/django/db/backends/utils.py", line
89, in _execute
return self.cursor.execute(sql, params)
File
"/Users/carlton/Projects/Django/django/django/db/backends/sqlite3/base.py",
line 369, in execute
return Database.Cursor.execute(self, query, params)
django.db.utils.OperationalError: table article_publications has no column
named otherarticle_id
>>> ^D
}}}
Relevant DB schema:
{{{
sqlite> .tables
app_publication auth_user_groups
article auth_user_user_permissions
article_publications django_admin_log
auth_group django_content_type
auth_group_permissions django_migrations
auth_permission django_session
auth_user
sqlite> .schema article
CREATE TABLE IF NOT EXISTS "article" ("id" integer NOT NULL PRIMARY KEY
AUTOINCREMENT, "headline" varchar(100) NOT NULL);
sqlite> .schema article_publications
CREATE TABLE IF NOT EXISTS "article_publications" ("id" integer NOT NULL
PRIMARY KEY AUTOINCREMENT, "article_id" bigint NOT NULL REFERENCES
"article" ("id") DEFERRABLE INITIALLY DEFERRED, "publication_id" bigint
NOT NULL REFERENCES "app_publication" ("id") DEFERRABLE INITIALLY
DEFERRED);
CREATE UNIQUE INDEX
"article_publications_article_id_publication_id_8dd463d8_uniq" ON
"article_publications" ("article_id", "publication_id");
CREATE INDEX "article_publications_article_id_f32c80b0" ON
"article_publications" ("article_id");
CREATE INDEX "article_publications_publication_id_7425f904" ON
"article_publications" ("publication_id");
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:1>
Django
---------------------------------+------------------------------------
Reporter: Timothy Thomas | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: RenameModel | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by Carlton Gibson):
* cc: Iuri de Silvio (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:2>
Django
Reporter: Timothy Thomas | Owner: Iuri de
| Silvio
Type: Bug | Status: assigned
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: RenameModel | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Iuri de Silvio):
* owner: nobody => Iuri de Silvio
* status: new => assigned
Comment:
I would expect the field name using the `db_table` to generate their name
instead of the model name, but I agree it should not change the existing
behavior.
Looks like an easy fix, I'll write a test and try it today.
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Timothy Thomas | Owner: Iuri de
| Silvio
Type: Bug | Status: assigned
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: RenameModel | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Iuri de Silvio):
* has_patch: 0 => 1
Comment:
I wrote a broken test and fixed the implementation.
https://github.com/django/django/pull/15993
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Timothy Thomas | Owner: Iuri de
| Silvio
Type: Bug | Status: assigned
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution:
Keywords: RenameModel | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Iuri de Silvio):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Timothy Thomas | Owner: Iuri de
| Silvio
Type: Bug | Status: assigned
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Timothy Thomas | Owner: Iuri de
| Silvio
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution: fixed
Keywords: RenameModel | 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:"166a3b32632c141541d1c3f0eff18e1d8b389404" 166a3b32]:
{{{
#!CommitTicketReference repository=""
revision="166a3b32632c141541d1c3f0eff18e1d8b389404"
Fixed #33953 -- Reverted "Fixed #33201 -- Made RenameModel operation a
noop for models with db_table."
Regression in afeafd6036616bac8263d762c1610f22241c0187.
This reverts afeafd6036616bac8263d762c1610f22241c0187.
Thanks Timothy Thomas for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Timothy Thomas | Owner: Iuri de
| Silvio
Type: Bug | Status: closed
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution: fixed
Keywords: RenameModel | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"7d5ccbbe1a3ad99b02c25f1ce90f36334d881ab6" 7d5ccbbe]:
{{{
#!CommitTicketReference repository=""
revision="7d5ccbbe1a3ad99b02c25f1ce90f36334d881ab6"
[4.1.x] Fixed #33953 -- Reverted "Fixed #33201 -- Made RenameModel
operation a noop for models with db_table."
Regression in afeafd6036616bac8263d762c1610f22241c0187.
This reverts afeafd6036616bac8263d762c1610f22241c0187.
Thanks Timothy Thomas for the report.
Backport of 166a3b32632c141541d1c3f0eff18e1d8b389404 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Timothy Thomas | Owner: Iuri de
| Silvio
Type: Bug | Status: closed
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution: fixed
Keywords: RenameModel | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"a9e7beb959bc726eab1c192d2625d6ff6cfa70f4" a9e7beb]:
{{{
#!CommitTicketReference repository=""
revision="a9e7beb959bc726eab1c192d2625d6ff6cfa70f4"
Refs #33953 -- Fixed test_rename_model_with_db_table_rename_m2m() crash on
SQLite < 3.20.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Timothy Thomas | Owner: Iuri de
| Silvio
Type: Bug | Status: closed
Component: Migrations | Version: 4.1
Severity: Release blocker | Resolution: fixed
Keywords: RenameModel | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"524ea6b77b9bcc8cb24fd0b55e5cb8ca4186ee4f" 524ea6b7]:
{{{
#!CommitTicketReference repository=""
revision="524ea6b77b9bcc8cb24fd0b55e5cb8ca4186ee4f"
[4.1.x] Refs #33953 -- Fixed test_rename_model_with_db_table_rename_m2m()
crash on SQLite < 3.20.
Backport of a9e7beb959bc726eab1c192d2625d6ff6cfa70f4 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33953#comment:10>