Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#34138 closed Bug (fixed)

Adding ManyToManyField on SQLite rebuilds table.

Reported by: David Wobrock Owned by: Mariusz Felisiak
Component: Migrations Version: 4.1
Severity: Release blocker Keywords: sqlite3
Cc: Mariusz Felisiak Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hey there,

While updating the django-migration-linter for Django 4.1 (yeah, a bit late to the party, but I was busy eh :P), I bumped into what seems to be a regression.

On SQLite, when adding a ManyToManyField to a table, it seems to rebuild the table - whereas it did not in Django 4.0.
From my understanding, rebuilding the table is not necessary in this case. Or perhaps I'm missing something.

Steps to reproduce:

1/ Before models:

class A(models.Model):
    pass

class B(models.Model):
    pass

(with it's boring migration)

2/ After models:

class A(models.Model):
    pass

class B(models.Model):
    many_to_many = models.ManyToManyField(A)

Which, expectedly, generates the migration:

from django.db import migrations, models


class Migration(migrations.Migration):

    dependencies = [("app_add_manytomany_field", "0001_initial")]

    operations = [
        migrations.AddField(
            model_name="b",
            name="many_to_many",
            field=models.ManyToManyField(to="app_add_manytomany_field.A"),
        )
    ]

All good up until here.

Now the "regression", in Django 4.0, a sqlmigrate generates:

BEGIN;
--
-- Add field many_to_many to b
--
CREATE TABLE "app_add_manytomany_field_b_many_to_many" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "b_id" integer NOT NULL REFERENCES "app_add_manytomany_field_b" ("id") DEFERRABLE INITIALLY DEFERRED, "a_id" integer NOT NULL REFERENCES "app_add_manytomany_field_a" ("id") DEFERRABLE INITIALLY DEFERRED);
CREATE UNIQUE INDEX "app_add_manytomany_field_b_many_to_many_b_id_a_id_3e15251d_uniq" ON "app_add_manytomany_field_b_many_to_many" ("b_id", "a_id");
CREATE INDEX "app_add_manytomany_field_b_many_to_many_b_id_953b185b" ON "app_add_manytomany_field_b_many_to_many" ("b_id");
CREATE INDEX "app_add_manytomany_field_b_many_to_many_a_id_4b44832a" ON "app_add_manytomany_field_b_many_to_many" ("a_id");
COMMIT;

whereas in Django 4.1:

BEGIN;
--
-- Add field many_to_many to b
--
CREATE TABLE "new__app_add_manytomany_field_b" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "x" integer NOT NULL);
CREATE TABLE "app_add_manytomany_field_b_many_to_many" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "b_id" integer NOT NULL REFERENCES "app_add_manytomany_field_b" ("id") DEFERRABLE INITIALLY DEFERRED, "a_id" integer NOT NULL REFERENCES "app_add_manytomany_field_a" ("id") DEFERRABLE INITIALLY DEFERRED);
INSERT INTO "new__app_add_manytomany_field_b" ("id", "x") SELECT "id", "x" FROM "app_add_manytomany_field_b";
DROP TABLE "app_add_manytomany_field_b";
ALTER TABLE "new__app_add_manytomany_field_b" RENAME TO "app_add_manytomany_field_b";
CREATE UNIQUE INDEX "app_add_manytomany_field_b_many_to_many_b_id_a_id_3e15251d_uniq" ON "app_add_manytomany_field_b_many_to_many" ("b_id", "a_id");
CREATE INDEX "app_add_manytomany_field_b_many_to_many_b_id_953b185b" ON "app_add_manytomany_field_b_many_to_many" ("b_id");
CREATE INDEX "app_add_manytomany_field_b_many_to_many_a_id_4b44832a" ON "app_add_manytomany_field_b_many_to_many" ("a_id");
COMMIT;

I could bisect it down to this commit 2f73e5406d54cb8945e187eff302a3a3373350be (from #32502 and this PR).

In the diff we see that the # Special-case implicit M2M tables comment and its code were removed.
That's potentially a lead for a fix here I guess :)

(On a side note, this commit introduced another regression #33408. But that's not related to the issue at hand)

Thank you!

Change History (4)

comment:1 by Simon Charette, 2 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:2 by Mariusz Felisiak, 2 years ago

Has patch: set
Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

comment:3 by GitHub <noreply@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 7b0e9ea5:

Fixed #34138 -- Avoided table rebuild when adding inline m2m fields on SQLite.

Regression in 2f73e5406d54cb8945e187eff302a3a3373350be.

Thanks David Wobrock for the report.

comment:4 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 84a2b2e:

[4.1.x] Fixed #34138 -- Avoided table rebuild when adding inline m2m fields on SQLite.

Regression in 2f73e5406d54cb8945e187eff302a3a3373350be.

Thanks David Wobrock for the report.
Backport of 7b0e9ea53ca99de2f485ec582f3a79be34b531d4 from main

Note: See TracTickets for help on using tickets.
Back to Top