[Django] #34856: Running tests with historical migrations that contain index together fails with TypeError.

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-----------------------------------------+------------------------
Reporter: Sage Abdullah | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
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 |
-----------------------------------------+------------------------
When running tests against Django's `main` branch on a project that has a
historical migration that contains `index_together` (either via the
`AlterIndexTogether` operation or as part of a `CreateModel` operation),
running that migration fails with `TypeError: 'class Meta' got invalid
attribute(s): index_together`. This is the case even if the model has
already been updated to use `indexes` instead of `index_together` and new
migrations have been created to update it.

To reproduce:
- Install Django < 5.1
- Start a new project and a new app
- Create a model with `index_together` and run `makemigrations`
- or run `makemigrations` before adding `index_together` and run it
again so `AlterIndexTogether` operation is created, same result
- Write a test that uses the database (a simple test with `TestCase` and
not `SimpleTestCase` should suffice)
- Optional: Update the model to use `indexes` instead of `index_together`
and run `makemigrations`
- Install latest Django `main`
- Run `python manage.py test -v3` and it fails when applying the migration
that contains `index_together`

Reproducible example: https://github.com/laymonage/django-test-repro

This is likely because `"index_together"` has been removed from
`django.db.models.options.DEFAULT_NAMES` in
2abf417c815c20f41c0868d6f66520b32347106e. Meanwhile, the historical model
constructed by the migrations framework will try to build the model with
the `Meta.index_together` option.

I'm not sure if this is expected. If it is, then that means developers
have to update their old migrations to not use `index_together` at all,
which doesn't seem to be ideal.

If it is not, the solution might be to reintroduce `"index_together"` in
`DEFAULT_NAMES` even though it will be no-op. Not sure if that's all there
is to it, but I'm happy to make a PR for a start.

--
Ticket URL: <https://code.djangoproject.com/ticket/34856>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.

-------------------------------+--------------------------------------

Reporter: Sage Abdullah | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev

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 Sage Abdullah):

To add, the [https://docs.djangoproject.com/en/dev/releases/4.2/#index-
together-option-is-deprecated-in-favor-of-indexes 4.2 release notes] do
say the following though:

> Next, consider squashing migrations to remove `index_together` from
historical migrations.
> The `AlterIndexTogether` migration operation is now officially supported
only for pre-Django 4.2 migration files. For backward compatibility
reasons, it’s still part of the public API, and there’s no plan to
deprecate or remove it, but it should not be used for new migrations. Use
`AddIndex` and `RemoveIndex` operations instead.

But with the problem in this ticket, it seems squashing the migrations
will be mandatory?

--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:1>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------+--------------------------------------
Reporter: Sage Abdullah | Owner: nobody

Type: Bug | Status: closed
Component: Migrations | Version: dev
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 David Sanders):

* status: new => closed
* resolution: => invalid

Comment:

I was about to reply linking to the 4.2 release notes then saw you
answered your own question:

> To add, the ​4.2 release notes do say the following though:
> ...

> But with the problem in this ticket, it seems squashing the migrations
will be mandatory?

Yes :)

--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:2>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------+--------------------------------------
Reporter: Sage Abdullah | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: dev
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
-------------------------------+--------------------------------------

Comment (by David Sanders):

PS: If you disagree with the removal at the end of deprecation feel free
to start a thread on the Django forum:
https://www.djangoproject.com/community/

--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:3>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------+--------------------------------------
Reporter: Sage Abdullah | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: dev
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
-------------------------------+--------------------------------------

Comment (by Sage Abdullah):

Replying to [comment:3 David Sanders]:

> PS: If you disagree with the removal at the end of deprecation feel free
to start a thread on the Django forum:
https://www.djangoproject.com/community/

Thanks! I don't mind the removal itself, though I wonder if the release
notes should be updated to better reflect the fact that squashing the
migrations will be mandatory? The current wording seems to suggest it's
optional. That said, this ticket is probably enough as additional
documentation about this issue if anyone else encounters it in the future.

--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:4>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------+--------------------------------------
Reporter: Sage Abdullah | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: dev
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
-------------------------------+--------------------------------------

Comment (by Tim Graham):

And the release notes say, "The `AlterIndexTogether` migration operation

is now officially supported only for pre-Django 4.2 migration files. For
backward compatibility reasons, it’s still part of the public API, and
there’s no plan to deprecate or remove it, but it should not be used for
new migrations. "

I'm not immediately seeing what purpose it serves if it crashes (see
#35679). I think removing an index, e.g. `AlterIndexTogether("Pony",
None)`, still works, but I'm not sure this provides sufficient value to
keep it around.
--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:5>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------+--------------------------------------
Reporter: Sage Abdullah | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: dev
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
-------------------------------+--------------------------------------

Comment (by Andrés Reverón Molina):

Hi, I'm in the process of upgrading a project from Django 4.2.x to 5.1.x
and I'm facing this issue.

Every instance of {{{ Meta.index_together }}} has already been updated to
use {{{ Meta.indexes }}}, but there are still some {{{ AlterIndexTogether
}}} in historical migrations, which happily crash with a
{{{ TypeError: 'class Meta' got invalid attribute(s): index_together }}}
error.

Unfortunately squashing migrations does not help, as {{{
AlterIndexTogether }}} is still there after the migrations have been
squashed.

Is there a recommended workaround to this? Maybe what is suggested here
https://code.djangoproject.com/ticket/35679#comment:1 ?
--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:6>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.

---------------------------------+------------------------------------

Reporter: Sage Abdullah | Owner: nobody

Type: Bug | Status: new
Component: Migrations | Version: dev
Severity: Release blocker | 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):

* resolution: invalid =>
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
* status: closed => new

Comment:

Reopening per [https://forum.djangoproject.com/t/django-5-1
-alterindextogether-raising-typeerror-class-meta-got-invalid-attribute-s
-index-together/ Django forum thread].
--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:7>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.

-------------------------------------+-------------------------------------
Reporter: Sage Abdullah | Owner: Simon
| Charette
Type: Bug | Status: assigned

Component: Migrations | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* has_patch: 0 => 1
* owner: nobody => Simon Charette
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:8>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------------+-------------------------------------

Reporter: Sage Abdullah | Owner: Andrés
| Reverón Molina

Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* owner: Simon Charette => Andrés Reverón Molina

--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:9>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------------+-------------------------------------
Reporter: Sage Abdullah | Owner: Andrés
| Reverón Molina
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:10>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------------+-------------------------------------
Reporter: Sage Abdullah | Owner: Andrés
| Reverón Molina

Type: Bug | Status: closed
Component: Migrations | Version: dev
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"b44efdfe543c9b9f12690b59777e6b275cb08103" b44efdfe]:
{{{#!CommitTicketReference repository=""
revision="b44efdfe543c9b9f12690b59777e6b275cb08103"
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(options["unique_together"]) was also affected.

Refs #27236.

Co-authored-by: Simon Charette <chare...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:11>

[Django]

#34856: Running tests with historical migrations that contain index together fails
with TypeError.
-------------------------------------+-------------------------------------
Reporter: Sage Abdullah | Owner: Andrés
| Reverón Molina
Type: Bug | Status: closed
Component: Migrations | Version: dev
Severity: Release blocker | Resolution: fixed
Keywords: | 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 Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"2ee6ca6d35f667c54cba7efacccee3b9f178a6e7" 2ee6ca6d]:
{{{#!CommitTicketReference repository=""
revision="2ee6ca6d35f667c54cba7efacccee3b9f178a6e7"
[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(options["unique_together"]) was also affected.

Refs #27236.

Co-authored-by: Simon Charette <chare...@gmail.com>

Backport of b44efdfe543c9b9f12690b59777e6b275cb08103 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34856#comment:12>