Fix Django warnings about index_together with new migration

[AlanCoding]

This is triggering a Django warning

RemovedInDjango51Warning: 'index_together' is deprecated. Use 'Meta.indexes' in 'taggit.TaggedItem' instead.

This updates the index_together use to resolve this, hopefully painlessly.

[rtpg]

something to check, whether index renaming is cheap or not in the major SQL vendors (locking etc)

[rtpg]

also whether the index renaming is actually safe? we want to make sure that deployments including this don't cause downtime. I'm also partial to outright not having the rename by hardcoding the existing index name

[AlanCoding]

Right, let's look at the details of what this migration does. For this I'm using a local sqlite3 database for testing.

$ python -m django sqlmigrate taggit 0006 --settings=tests.settings
BEGIN;
--
-- Rename unnamed index for ('content_type', 'object_id') on taggeditem to taggit_tagg_content_8fc721_idx
--
DROP INDEX "taggit_taggeditem_content_type_id_object_id_196cc965_idx";
CREATE INDEX "taggit_tagg_content_8fc721_idx" ON "taggit_taggeditem" ("content_type_id", "object_id");
COMMIT;

This confirms your hunch, it does delete the old index and re-create it. This could have a performance penalty. I believe this is called out by the Django docs

On databases that don’t support an index renaming statement (SQLite and MariaDB < 10.5.2), the operation will drop and recreate the index, which can be expensive.

And this case is sqlite3. For other databases like postgres, I believe it should use the rename SQL.

Also, the checks and the docs are telling me that this would have to be Django 4.1 or higher, which is a problematic requirement for migrations.

Also, the "test" app has unmigrated changes when testing with Django 4.2. Those don't seem to have anything to do with the index changes in Django.

[rtpg]

Things to do on this one:

• set things up to make RemovedInXWarnings instead be errors for testing on djmain.
• concretise the existing index names when setting up the IndexTogether relations, making sure to not generate any actual SQL operations

[mister-rao]

Getting this error while running tests:

django.utils.deprecation.RemovedInDjango51Warning: 'index_together' is deprecated. Use 'Meta.indexes' in 'taggit.TaggedItem' instead. 

Versions:
Django = "4.2.2"
django-taggit = "4.0.0"

[laymonage]

Using migrations.SeparateDatabaseAndState might do the trick to do this with zero downtime as explained in this article: https://adamj.eu/tech/2020/07/27/how-to-modernize-your-django-index-definitions/

[igorsimb]

Why is this not merged yet?