[Django] #28339: Setting db_index=False on a ForeignKey doesn't actually remove the index
Django
--------------------------------------------+------------------------
Reporter: Craig de Stigter | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------
1. Make a `ForeignKey`. Don't specify a value for `db_index` (django sets
it to `True` by default)
2. Make a migration for the foreignkey. This creates an index:
{{{
#!sql
CREATE INDEX "a_c_b_id_5a026661" ON "a_c" ("b_id");
}}}
3. Explicitly set `db_index=False` and create another migration
4. Check `sqlmigrate` for the new migration. Index is never actually
removed:
{{{
#!sql
BEGIN;
--
-- Alter field b on c
--
COMMIT;
}}}
One note: This is with postgres settings. I'm not sure if it's db-specific
--
Ticket URL: <https://code.djangoproject.com/ticket/28339>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Craig de Stigter | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.11
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 Craig de Stigter):
A tiny test project: https://github.com/craigds/django-28339-testproject
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:1>
Django
Reporter: Craig de Stigter | Owner: nobody
Component: Migrations | Version: 1.11
Severity: Normal | 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 Craig de Stigter):
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:2>
Django
Reporter: Craig de Stigter | Owner: nobody
Component: Migrations | Version: 1.11
Severity: Normal | 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 Craig de Stigter):
Turns out, it's not just ForeignKey. Indices are not dropped on other
fields too:
{{{
#!sql
$ ./tp3/manage.py sqlmigrate a 0003
BEGIN;
--
-- Add field charfield to b
--
ALTER TABLE "a_b" ADD COLUMN "charfield" varchar(1) DEFAULT '' NOT NULL;
ALTER TABLE "a_b" ALTER COLUMN "charfield" DROP DEFAULT;
--
-- Add field intfield to b
--
ALTER TABLE "a_b" ADD COLUMN "intfield" integer DEFAULT 0 NOT NULL;
ALTER TABLE "a_b" ALTER COLUMN "intfield" DROP DEFAULT;
CREATE INDEX "a_b_charfield_a5049317" ON "a_b" ("charfield");
CREATE INDEX "a_b_charfield_a5049317_like" ON "a_b" ("charfield"
varchar_pattern_ops);
CREATE INDEX "a_b_intfield_b730ef11" ON "a_b" ("intfield");
COMMIT;
}}}
{{{
#!sql
$ ./tp3/manage.py sqlmigrate a 0004
BEGIN;
--
-- Alter field charfield on b
--
--
-- Alter field intfield on b
--
COMMIT;
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:3>
Django
----------------------------------+--------------------------------------
Reporter: Craig de Stigter | Owner: nobody
Component: Migrations | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Craig de Stigter:
Old description:
> 1. Make a `ForeignKey`. Don't specify a value for `db_index` (django sets
> it to `True` by default)
> 2. Make a migration for the foreignkey. This creates an index:
>
> {{{
> #!sql
> CREATE INDEX "a_c_b_id_5a026661" ON "a_c" ("b_id");
> }}}
>
> 3. Explicitly set `db_index=False` and create another migration
> 4. Check `sqlmigrate` for the new migration. Index is never actually
> removed:
>
> {{{
> #!sql
> BEGIN;
> --
> -- Alter field b on c
> --
> COMMIT;
> }}}
>
> One note: This is with postgres settings. I'm not sure if it's db-
> specific
New description:
1. Make a field with `db_index=True` (for `ForeignKey`, django sets this
by default)
2. Make a migration. This creates an index:
{{{
#!sql
CREATE INDEX "a_c_b_id_5a026661" ON "a_c" ("b_id");
}}}
3. Explicitly set `db_index=False` and create another migration
4. Check `sqlmigrate` for the new migration. Index is never actually
removed:
{{{
#!sql
BEGIN;
--
-- Alter field b on c
--
COMMIT;
}}}
One note: This is with postgres settings. I'm not sure if it's db-
specific.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:4>
Django
----------------------------------+--------------------------------------
Reporter: Craig de Stigter | Owner: nobody
Component: Migrations | Version: 1.11
Severity: Normal | Resolution: invalid
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 Craig de Stigter):
* status: new => closed
* resolution: => invalid
Comment:
My apologies, this is a case of me not understanding how exactly
`sqlmigrate` works.
It appears that maybe Django doesn't know what the index is named ahead of
time.
If you `sqlmigrate` a migration that drops an index _without_ actually
creating the index first in the database, the resulting SQL skips the
index drop as described above.
However if you actually create the index first, then the `sqlmigrate`
connects to the database and inspects the schema to determine which
indexes have been created. Then the resulting SQL correctly includes the
index drop.
This is rather counterintuitive (to me), but perhaps it's the only
possible way to do it?
Unfortunately this means I cannot convert autogenerated django migrations
to SQL up-front.
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:5>
Django
----------------------------------+--------------------------------------
Reporter: Craig de Stigter | Owner: nobody
Component: Migrations | Version: 1.11
Severity: Normal | 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 Craig de Stigter):
* status: closed => new
* resolution: invalid =>
Comment:
On second thoughts, this is still a bug, right?
sqlmigrate above shows that the operation creating the indices _knows_ the
name of the indices:
{{{
#!sql
CREATE INDEX "a_b_charfield_a5049317" ON "a_b" ("charfield");
}}}
so clearly this can be predicted up-front by the migration - it shouldn't
need to consult the database to list indices prior to dropping them.
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:6>
Django
index names
--------------------------------------+------------------------------------
Reporter: Craig de Stigter | Owner: nobody
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* component: Migrations => Documentation
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
I think the constraint and index names generated by Django aren't
necessarily the same across different versions -- that's probably why
introspection is used. [https://docs.djangoproject.com/en/dev/ref/django-
admin/#django-admin-sqlmigrate As the documentation for sqlmigrate] says,
"This requires an active database connection, which it will use to
resolve constraint names; this means you must generate the SQL against a
copy of the database you wish to later apply it on."
See also bugs like #23577 which cause names to get out of sync with that
Django initially generated (and #27064).
I'm not sure if anything can be done besides improving the documentation
of these limitations.
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:7>
Django
index names
--------------------------------------+------------------------------------
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aarav Sharma):
Submitted PR: https://github.com/django/django/pull/20872
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:8>
Django
index names
--------------------------------------+------------------------------------
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by Aarav Sharma):
* cc: Aarav Sharma (added)
* has_patch: 0 => 1
Comment:
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:9>
Django
index names
--------------------------------------+------------------------------------
Component: Documentation | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by David Smith):
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/28339#comment:10>