Redundant migration code

[Silvio]

I've been digging around the 3.1 migration code just out of curiosity, and I stumbled upon this in autodetector.py#772:

            for name in sorted(related_fields):

                self.add_operation(

                    app_label,

                    operations.RemoveField(

                        model_name=model_name,

                        name=name,

                    )

                )

It appears that all related fields for a model are removed before removing a model. This is great, *except*, optimizer.py will always strip these migrations out.

That is: the RemoveField operations for related fields are always adjacent to the DeleteModel operation, so I cannot think of any case where optimizer.py will *not* strip these out.

Am I missing something, or is this code that no longer contributes value? Perhaps the optimizer has gotten better since?

Best,

Silvio

[Adam Johnson]

Did you try deleting this code and seeing if any test failed?

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/46e53585-64f3-4e88-87a7-2cab7722aefan%40googlegroups.com.

--

Adam

[Silvio J. Gutierrez]

3 tests fail when I comment out the code, however, I suspect it's because of this:

test_autodetector.py#586

It's directly calling _detect_changes before piping it to the optimizer. So rightly so, it expects more operations than post optimization.

I could be totally wrong though.

Basically, with code (and what the unit test asserts):

['RemoveField', 'RemoveField', 'DeleteModel', 'DeleteModel']

Without code:

['DeleteModel', 'DeleteModel']

With code but after optimizer:

['DeleteModel', 'DeleteModel']

But the unit test is looking at pre-optimizer output. I think.

You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/fwwSmD3zBpQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM3Y9WD-8VR2hC93tu7BtgWrEr0j4nfOx9HSLbi0mm%3D50A%40mail.gmail.com.

[Andrew Godwin]

I suspect the reason for this might be to undo circular references of ForeignKeys between apps - in this situation, you have to first have a migration that removes one FK, then in the other app remove the model, then in the first app remove the model.

I can't quite remember though - but that would be my suspicion as to why this would be structured this way in my original code. As you say, the optimiser has improved in the years since, but I don't think you can optimise away the circular reference problem?

Andrew

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CANfEt_abmcbUp_QdUJSebRm9od5dKjtz7DkFgOXVBL5O8yisfg%40mail.gmail.com.

[Silvio J. Gutierrez]

That's what I originally thought. But if I look at the code, it's entirely self contained to the existing model. There's no variable in scope that lets it reach outside the app/model in question.

And thanks for writing the migration feature! I remember when it first came out. Easily one of my favorite Django batteries.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/ff59c1a1-f6b7-49d2-b588-26edf2b8293f%40www.fastmail.com.