Deprecate Model.Meta.index_together in favour of Model.Meta.indexes

[Akshesh Doshi]

Anything that index_together does can be done by indexes and the Index class.

[Akshesh Doshi]

This ticket tracks a sub-part of this deprecation process - https://code.djangoproject.com/ticket/27064

[Tim Graham]

My understanding is that we're keeping the AlterIndexTogether migration operation around indefinitely for support in historical migrations. Are there any other details from our conversations to add?

[Akshesh Doshi]

Yes, since AlterIndexTogether is currently there in the migrations of so many Django projects we need to keep maintaining it. But the idea is to deprecate the use of index_together in the models file and the generate_altered_index_together method of the MigrationAutodetector.

[Akshesh Doshi]

Changed the keywords by mistake.

[Olivier Tabone]

working on this ticket during duth sprint

[Olivier Tabone]

see ​https://github.com/django/django/pull/7509

any comment appreciated

[Tim Graham]

I left comments for improvement.

[Markus Holtermann]

While this is the one step of the deprecation process of index_together there is currently no way to migrate an existing from index_together to indexes w/o dropping and recreating indexes. Since removing and adding an index can be a quite expensive operation, this is not an option to do so for now.

The next step in the deprecation process needs to treat indexes and index_together similarly in the sense that the latter is translated into the former internally (inside the migration framework). Once this is done, the deprecation of actually using index_together can start.

[Olivier Tabone]

Hi Markus, Thank you for your reply.

Are you referring to #27064 ? or is it something different ?

[Tim Graham]

I think that's the correct ticket.

[David Wobrock]

Since #27064 is done, I'll try to tackle this issue.
Here is a new ​PR based a bit off the previous one.

I changed the assignee, since it was set about 6 years ago :)

[David Wobrock]

Here we go again! With 3 PRs:
​The main PR
​Inlining test classes
​Splitting index/unique together autodetector tests

[Mariusz Felisiak <felisiak.mariusz@…>]

In a3a1290d:

Refs #27236 -- Moved models with Meta.index_together inside of test methods.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 457cfd6f:

Refs #27236 -- Added test_autodetector.BaseAutodetectorTests.

[Mariusz Felisiak <felisiak.mariusz@…>]

In db588d4f:

Refs #27236 -- Split index_together and unique_together autodetector tests.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 57793b47:

Refs #27236 -- Refactored out DeprecationForHistoricalMigrationMixin.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 41019e48:

Refs #27236 -- Added generic mechanism to handle the deprecation of migration operations.

[Mariusz Felisiak <felisiak.mariusz@…>]

In f8f16b3c:

Refs #27236 -- Removed usage of Meta.index_together from indexes/introspection test models.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 4f284115:

Refs #27236 -- Split RenameField() tests with unique/index_together.

[David Wobrock <david.wobrock@…>]

In a6385b3:

Fixed #27236 -- Deprecated Meta.index_together in favor of Meta.indexes.

This also deprecates AlterIndexTogether migration operation.

[Collin Anderson]

I'm getting system check error: "AlterIndexTogether is deprecated. Support for it (except in historical migrations) will be removed in Django 5.1.", even though it's only used in historical migrations. Is there something I'm supposed to do? Or why give a warning if I don't need to take any action? Is there any way to clarify this? (Same with release notes deprecation section, very unclear what action I'm supposed to take.)

[Simon Charette]

I think you'll want to squash the migrations containing such operations. I'm not sure if the warnings point at doing that but if it's not the case it definitely should as this will be a common issue given AlterIndexTogether has been around forever.

[Mariusz Felisiak]

We should probably run check_deprecation_details() only for not applied migrations. What do you think?

[Mariusz Felisiak]

​PR

[Simon Charette]

We should probably run check_deprecation_details() only for not applied migrations. What do you think?

I think this will go a long way in avoiding to spam users every single time they run a management command that triggers system checks but it's not uncommon for developers to drop their local database and start from scratch.

Ultimately they'll have to squash their migrations before 5.1 if they want to avoid having their migration breaking so I wonder if we are doing them a favour by silencing the check which than can already silence using SILENCED_SYSTEM_CHECKS.

I think the system check hint should point at the proper procedure of using indexes and running makemigrations at least?

Ultimately the only way for all the warnings and check errors to go away before 5.1 where AlterIndexTogether will likely be marked as elidable = True since it will be a noop is for users to manually edit their migrations to replace their usage of AlterIndexTogether by AddIndex and friends. I'm not sure the current documentation does a good job at explaining how this must be done as it's a relatively complex operation.

Maybe there something that could be done through RenameIndex.reduce(operation: AlterIndexTogether) to ease the pain when old_fields is present here and have migration squashing handle most of the cases? Otherwise I foresee a lot of reports coming our way on the 5.1 rollout saying that RenameIndex is failing to run due to a missing index.

[Collin Anderson]

"run only on unapplied migrations"

makes sense to me.

"Ultimately they'll have to squash their migrations before 5.1 if they want to avoid having their migration breaking"

The deprecation warning/check says "Support for it (except in historical migrations) will be removed in Django 5.1", so that seems to say to me that it _won't_ be removed for historical migrations, and I as a developer don't need to worry about it for historical migrations.

I'm mostly just providing feedback here on the wording. If Django wants me to do something to my project to fix this issue, it should tell me what I should do (like squash, or is there an alternative migration operation that I should use instead?). Or if I don't need to do anything then there shouldn't be a warning.

[Mariusz Felisiak]

I think we need all three:

• more descriptive release notes for AlterIndexTogether,
• running checks only on unapplied migrations, and
• reducing AlterIndexTogether with RenameIndex.

Lots of folks don't recreate their databases so they won't have to do anything. As for the rest, we can document possible solutions, e.g.

• squash migrations,
• update the historical CreateModel() operation with options={index_together=[...]} with the current state of index_together (as long as all fields existed when the model was created),
• replace AlterIndexTogether with AddIndex/RemoveIndex.

[Mariusz Felisiak]

Alternatively, we can revert 57793b47657ace966ce8ce96d801ac0d85e5efc6, 41019e48bbf082c985e6ba3bad34d118b903bff1, and partly a6385b382e05a614a99e5a5913d8e631823159a2 and leave AlterIndexTogether fully functional forever.

[Collin Anderson]

I'm personally always in favor of keeping things fully functional forever, to reduce the amount of work that developers need to do, as long as it's easy enough to maintain ( ​https://groups.google.com/g/django-developers/c/zImXMKzWN4A/m/4SNu7MyFAQAJ ). If so, then there probably doesn't need to be a warning message at all, but certainly not for applied migrations, and no need for a deprecation timeline. Just discourage its use and move on. Again, as long as it's easy enough to maintain long-term.

If functionality is getting removed or no-op'd in the future, then I'd suggest that yes there should be a warning (not just for un-applied migrations), and in my opinion the warning message should suggest an alternative or at least link to docs about this deprecation and how to fix it (AddIndex/RemoveIndex or something?) And, ideally it would be a type of change that could be automatically applied using django-codemod or django-upgrade, but that might be asking for too much.

[Mariusz Felisiak]

I'm longer convinced that we have to deprecate and remove it. We can document that is supported only in pre-Django 4.2 migrations and will not receive any bugfixes in the future.

Simon, What do you think?

[Mariusz Felisiak]

Alternative ​PR.

[GitHub <noreply@…>]

In f810325:

Refs #27236 -- Made cosmetic edits to Meta.index_together deprecation.

This should make it more straightforward to move forward when
deprecation ends.

[Mariusz Felisiak <felisiak.mariusz@…>]

In c773d579:

Refs #27236 -- Reverted AlterIndexTogether deprecation.

This partly reverts a6385b382e05a614a99e5a5913d8e631823159a2.

[Mariusz Felisiak <felisiak.mariusz@…>]

In a1e9e9ab:

Refs #27236 -- Reverted "Refs #27236 -- Added generic mechanism to handle the deprecation of migration operations."

This reverts commit 41019e48bbf082c985e6ba3bad34d118b903bff1.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 66f30dbf:

Refs #27236 -- Reverted "Refs #27236 -- Refactored out DeprecationForHistoricalMigrationMixin."

This reverts commit 57793b47657ace966ce8ce96d801ac0d85e5efc6.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7e3c9c32:

Refs #27236 -- Doc'd that AlterIndexTogether is no longer officially supported for Django 4.2+ migration files.

[GitHub <noreply@…>]

In ca5d3c9:

Refs #27236 -- Made more cosmetic edits to Meta.index_together deprecation.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 2abf417c:

Refs #27236 -- Removed Meta.index_together per deprecation timeline.

[Sarah Boyce <42296566+sarahboyce@…>]

In 44281bc:

Refs #27236 -- Removed references to index_together from ModelState.from_model.

It's not possible for ModelMeta.index_together to exist anymore.

[Sarah Boyce <42296566+sarahboyce@…>]

In b44efdfe:

Fixed #34856 -- Fixed references to index_together in historical migrations.

While AlterUniqueTogether has been documented to be still allowed in historical
migrations for the foreseeable future it has been crashing since 2abf417c815c20
was merged because the latter removed support for Meta.index_together which the
migration framework uses to render models to perform schema changes.

CreateModel(optionsunique_together) was also affected.

Refs #27236.

Co-authored-by: Simon Charette <charette.s@…>

[Sarah Boyce <42296566+sarahboyce@…>]

In 85c35501:

[5.1.x] Refs #27236 -- Removed references to index_together from ModelState.from_model.

It's not possible for ModelMeta.index_together to exist anymore.

Backport of 44281bc2123dac1b127000c84c759f9678021826 from main.

[Sarah Boyce <42296566+sarahboyce@…>]

In 2ee6ca6d:

[5.1.x] Fixed #34856 -- Fixed references to index_together in historical migrations.

While AlterUniqueTogether has been documented to be still allowed in historical
migrations for the foreseeable future it has been crashing since 2abf417c815c20
was merged because the latter removed support for Meta.index_together which the
migration framework uses to render models to perform schema changes.

CreateModel(optionsunique_together) was also affected.

Refs #27236.

Co-authored-by: Simon Charette <charette.s@…>

Backport of b44efdfe543c9b9f12690b59777e6b275cb08103 from main.

[GitHub <noreply@…>]

In 6df19412:

Refs #27236 -- Removed references to removed index_together migration methods.