When deleting a model with foreignkey, redundant RemoveField created.

[Aur Saraf]

$ pip freeze
Django==1.7.1
wsgiref==0.1.2

I add a model that inherits from another model, makemigrations, delete that model, makemigrations, and the SQLite backend generates this illegal SQL when trying to delete the last field "model_ptr" from the table:

INSERT INTO "bug_b__new" () SELECT  FROM "bug_b"

Easy repro:

$ mkvirtualenv bug
$ git clone git@github.com:SonOfLilit/django-sqlite-migrations-bug.git
$ cd django-sqlite-migrations-bug
$ python manage.py test

Here is what happens: ​https://dpaste.de/LYqP

---

RELATED ISSUE:

Consider following:

class ModelA(models.Model):
    field1 = models.BooleanField(default=False)

class ModelB(models.Model):
    another_field1 = models.TextField(blank=True)
    related_field = models.ForeignKey(ModelA)

After creating inital migration, I deleted model, so migration will look like:

class Migration(migrations.Migration):

    dependencies = [
        ('blankapp', '0001_initial'),
    ]

    operations = [
        migrations.RemoveField(
            model_name='modelb',
            name='related_field',
        ),
        migrations.DeleteModel(
            name='ModelB',
        ),
    ]

And, when you will try to migrate backwards, LookupError: App 'blankapp' doesn't have a 'modelb' model. will be raised, because migration will try to add related_field to model which does not exists yet.

[Aur Saraf]

My workaround is to comment out the problematic RemoveField in the migration. Does it have any adverse effects? I'm not aware of any...

    operations = [
        # bug in SQLite backend
        # https://code.djangoproject.com/ticket/24061
        # migrations.RemoveField(
            # model_name='student',
            # name='user_ptr',
        # ),
        migrations.RemoveField(
            model_name='assignment',
            name='student',
        ),
        migrations.RemoveField(
            model_name='exercise',
            name='students',
        ),
        migrations.DeleteModel(
            name='Student',
        ),
    ]

[Andriy Sokolovskiy]

Checked on sqlite and django 1.7.2, issue exists - redundant RemoveField has been created.
I'll check on other backends, I think issue exists not only for sqlite.

(Leaving unaccepted to be reviewed by core devs, maybe this ticket related to another one).
I'll try to fix it will be accepted.

[Andriy Sokolovskiy]

Checked on postgres and current django master, issue exists.
I'm checking this as accepted.
After fix this should be backported to 1.7 branch.

[Andriy Sokolovskiy]

Opened question:

truecoldmind> Can anyone explain the exact reason why while deleting model in migration need to create foreign key deletion operations?
<truecoldmind> I'm talking about this https://github.com/django/django/blob/572ad9a92e08797f12e547477efcce7893159cfb/django/db/migrations/autodetector.py#L619
<truecoldmind> if I understand right, when deleting model, where was a foreign key, deleting a whole model will delete foreign key index too. But field operation removal will be created anyway.

After this I found another related bug.
Consider following:

class ModelA(models.Model):
    field1 = models.BooleanField(default=False)

class ModelB(models.Model):
    another_field1 = models.TextField(blank=True)
    related_field = models.ForeignKey(ModelA)

After creating inital migration, I deleted model, so migration will look like:

class Migration(migrations.Migration):

    dependencies = [
        ('blankapp', '0001_initial'),
    ]

    operations = [
        migrations.RemoveField(
            model_name='modelb',
            name='related_field',
        ),
        migrations.DeleteModel(
            name='ModelB',
        ),
    ]

And, when you will try to migrate backwards, LookupError: App 'blankapp' doesn't have a 'modelb' model. will be raised, because migration will try to add related_field to model which does not exists yet.

Should this issue be resolved in this ticket or in another?

[Markus Holtermann]

Regarding your first question: the autodetector needs to remove all foreign key fields in the same app that point towards to model you are removing to keep a working state.

Regarding your second question: I don't see a correlation between those two issues. Please open a new ticket for that problem.

[Andriy Sokolovskiy]

<truecoldmind> MarkusH, and issue that I posted in last comment is related if RemoveField operation on foreignkey (when deleting a whole model) is redundant 
<MarkusH> truecoldmind: a state that is working.
<MarkusH> if you keep a fk field around w/o the reverenced model being available, it is as if you define a fk w/o the model being available.
<MarkusH> truecoldmind: Try a models.py with https://dpaste.de/WAud
<MarkusH> Django will complain and fail to start
<MarkusH> if the FK fields are not removed, you end up with an internal state that is exactly as shown there
<shaib> MarkusH: At a glance, it looks like those operations remove the FKs from the models to be removed, not those pointing at them.
<truecoldmind> MarkusH, lets look on example https://dpaste.de/ThsG. When deleting ModelB, RemoveField operation will be created. For what? We remove only ModelB, this foreignkey is the field of ModelB, and will be deleted if ModelB will be dropped
<shaib> truecoldmind: As I said above, I think this is to make sure that the reverse relationships are removed from the models that remain.
<MarkusH> ok, I'll have a more detailed look at the code
<truecoldmind> shaib, in that case foreignkey is only part of ModelB and does not affects other models
<shaib> no, ModelA has a modelb attribute. Look up reverse relationships.
<shaib> Sorry, modelb_set.
<truecoldmind> shaib, it have it because foreignkey is present in model. How it is related to schema migration ?
<MarkusH> This is indeed weired.
<shaib> migrations also manage the models in memory (so that data migrations can use them)
<MarkusH> I don't see a reason for the RemoveField operations (yet)
<MarkusH> truecoldmind: the reverse relation for a FK field (e.g. FK from Book to Author: book_set) is created by the FK field
<MarkusH> shaib: please correct me if I'm wrong ;)
<MarkusH> So, if I delete the Book model (and thus don't have a FK to Author anymore), there can't be a reverse relation
<shaib> It would be better IMO if removing a model would clean up all reverse relations created by FKs in that model, but I'm not sure this is the case.
<apollo13> that ^
<truecoldmind> MarkusH, yes. and with Book as OneToOne field will be as author.book. But I don't understand how it is related to table drop operation
<MarkusH> shaib: that's what I'd expected as well
<MarkusH> I guess I'm missing something
<apollo13> if you look at the comment of that function, might be that the optimizer is missing something…
<MarkusH> apollo13: I don't see the reason for adding the RemoveField operations in the first place
<apollo13> maybe simplicity for now
<truecoldmind> apollo13, what simplicity? If you look at #24061, this thing is producing at least 2 bugs (first is it the title of ticket, second is in comments)
<ticketbot> https://code.djangoproject.com/ticket/24061
<apollo13> so?
<truecoldmind> apollo13, so what is the exact reason for RemoveField in this concrete example?
<apollo13> truecoldmind: as I said: maybe simplicity
<shaib> I'll elaborate: Perhaps the people who wrote migrations realized they need to clean up reverse relationships and thought it was simpler to do in the autodetector than in the model-deletion operation.

---

Some thoughts how it can be resolved:

<truecoldmind> maybe another operation is needed here, where only  `state_forwards` (without database operations) will be available?
<truecoldmind> for example, we can create `StateOperation(Operation)`, then prohibit database operations here, so we will can do cleanup in that way. Or it is must be hidden from user that will be reading the migration source?