Unique index lost when renaming a ManyToManyField

[sparrowt]

Problem

When doing migrations.RenameField on a field which is a models.ManyToManyField, the filtered UNIQUE INDEX (which enforces the uniqueness constraint on A_ID,B_ID pairs on the through table) is dropped before the rename, but not re-instated afterwards ⚠

Example

So I see something like this:

DROP INDEX myapp_mymodel_fieldname_x_id_y_id_uniq ON myapp_mymodel_fieldname
EXEC sp_rename [myapp_mymodel_fieldname], [myapp_mymodel_newfieldname]

and aftewards the dropped index is missing.

The lost index is the one created earlier (when the m2m field was added) to enforce the unique_together constraint on the 2 ID fields (on the through model which create_many_to_many_intermediary_model generates) something like this:

CREATE UNIQUE INDEX [myapp_mymodel_fieldname_x_id_y_id_uniq ] ON [myapp_mymodel_fieldname] ([x_id], [y_id]) WHERE [x_id] IS NOT NULL AND [y_id] IS NOT NULL

Cause

This seems to be the code at fault: https://github.com/microsoft/mssql-django/blame/1.1.1/mssql/schema.py#L231-L232 which came in ESSolutions/django-mssql-backend#24 where unique_together was first changed to be implemented by a filtered-unique-index rather than a constraint (in order to support nullability).

Test

I've written a regression test which fails due to the above bug: #88

[absci]

Thanks for the added unit test.

[sparrowt]

Before trying to investigate how to recreate the same set of indexes which _alter_many_to_many is dropping, I thought it was worth checking if the drop is actually required - if I just remove the drop then the test on #88 passes (!)

...so I'm not sure why 2e60754#diff-2ff8800556894aa014e93ed51e2d0ad60305eb905fcdd5cf399f76eb918ec483R177 added the drop but it seems MSSQL allows you to rename the table just fine, with a unique index remaining in place on that table.

Of course the name of the index isn't changed so it still contains a reference to the old name of the field, but that is far less of a problem (and a side effect which I've observed in other scenarios occasionally) compared to dropping the index entirely and not recreating it.

[sparrowt]

I have pushed the proposed fix to the same PR: #88, let me know what you think

[absci]

Close as the fix has been merged.

[sparrowt]

Thanks @absci - is there any chance of a new release soon please?

I'm hoping to officially start using this project as soon as it is published, and don't really want to start building my own wheels.

[absci]

Thanks @absci - is there any chance of a new release soon please?

I'm hoping to officially start using this project as soon as it is published, and don't really want to start building my own wheels.

Hi, the management plans to release a new version in July 2022. Hopefully it's not too late for you.

[sparrowt]

Unfortunately this is blocking us as follows:

• we cannot upgrade to this fork microsoft/mssql-django (from ESSolutions/django-mssql-backend) until a new version is released with the fixes which are already in dev
• we cannot upgrade to Django 3 until we have done the above upgrade (because to get Django 3 support, we would need to upgrade and get at a minimum all of these changes, but unfortunately pr 24 on that repo introduced a bunch of bugs, which haven't all been resolved until now in latest microsoft/mssql-django:dev)
• Django 2.2 LTS goes out of support in April 2022 i.e. next month (https://www.djangoproject.com/download/#supported-versions)

So it would be very much appreciated to have a release within the next month - otherwise we will be stuck on out-of-support Django, or have to consider trying to build our own "1.1.3" wheel off dev until an official one comes out.

Given the fixes are already merged, hopefully only a version bump is required? I'm happy to open that PR. Many thanks.

[vwarchu]

@sparrowt - Thanks for the context. Our team is now aiming to release by end of April, sooner if possible. We'll keep you posted.

[sparrowt]

Many thanks @vwarchu!

[vwarchu]

@sparrowt - We've now released version 1.1.3 which includes the aforementioned merges. Cheers

[sparrowt]

That's brilliant, thank you!

[sparrowt]

P.S. is it expected that 1.1.3...dev (and master...dev) still shows all those commits as missing from master/1.1.3 (and 1.1.2...1.1.3 only has 1 commit)?

Perhaps because #124 was squash-merged?

It would seem preferable if git blame on master showed the original commit messages, rather than "prepare for 1.1.3 release" like it now does e.g. here https://github.com/microsoft/mssql-django/blame/master/mssql/schema.py#L233

EDIT: filed #129