Unmanaged models with ForeignKeys do not get those fields serialized into their migration state when CreateModel happens.

[Keryn Knight]

Given models like the following, where B is unmanaged, and C is a normal concrete Model:

from django.db import models

class A(models.Model):
    class Meta:
        db_table = "test_a"


class B(models.Model):
    a = models.ForeignKey(A, related_name="+", on_delete=models.CASCADE)

    class Meta:
        managed = False
        db_table = "test_b"


class C(models.Model):
    a = models.ForeignKey(A, related_name="+", on_delete=models.CASCADE)

    class Meta:
    db_table = "test_c"

when python manage.py makemigrations <appname> is called, B ends up looking like:

migrations.CreateModel(
    name='B',
    fields=[
        ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
    ],
    options={
        'db_table': 'test_b',
        'managed': False,
    },
)

whilst C correctly gets:

migrations.CreateModel(
    name='C',
    fields=[
        ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
        ('a', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to='whee.A')),
    ],
    options={
        'db_table': 'test_c',
    },
)

Because B doesn't have the a attribute, any subsequent data migrations which would like to query on it, like so:

def forwards(apps, schema_editor):
    B = apps.get_model('whee', 'B')
    B.objects.filter(a=1)

will crash.

With the assistance of Markus on IRC:

MarkusH: do you want to try what's gonna happen when you just drop the if-condition in https://github.com/django/django/blob/75527c0f8360ae3555fcf67ce19b4cb910b69b9d/django/db/migrations/autodetector.py#L570-L572 ?

commenting out the lines

if not model_opts.managed:
    continue

allows the autodetector to generate the following:

migrations.CreateModel(
    name='B',
    fields=[
        ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
    ],
    options={
        'db_table': 'test_b',
        'managed': False,
    },
),
migrations.AddField(
    model_name='b',
    name='a',
    field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to='whee.A'),
)

and subsequently the data migration can access B.a

If A itself is managed=False the migration generated is instead:

migrations.CreateModel(
    name='B',
    fields=[
        ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
        ('a', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to='whee.A')),
    ],
    options={
        'db_table': 'test_b',
        'managed': False,
    },
)

Running the test suite via ./runtests.py as of commit 75527c0f8360ae3555fcf67ce19b4cb910b69b9d doesn't seem to cause any failures, with those lines commented out.

Markus also pointed out commit ​https://github.com/django/django/commit/215aa4f53b6bbd07d5c1eecfa94e7fcd00da813e which references #23415

I noticed this "issue" while bringing a project up from 1.9, so it's not a recent problem.

[Simon Charette]

Seems like a legitimate bug.

Do you think you could submit a PR with the removed lines and a test in ​test_autodetector.py for it?

[Keryn Knight]

WIP PR: ​https://github.com/django/django/pull/9741

[YBull]

Using version 2.0.5, I just encountered this bug and when trying to work around it, discovered that it's a little more expansive than the example model with A, B, C above shows. In my case, I had used the 'inspectdb' command to generate models for 12 legacy tables (all with managed = False). The inspectdb command did a reasonably good job and included all the ForeignKey fields it should have. But even after I commented out the lines in autodetector.py to work around this bug, I discovered that the 'makemigrations' command still has another related problem with Fkeys on unmanaged models. Specifically, whenever a new unmanaged model is referenced by another model's Fkeys, you must run 'makemigrations' for the referenced table BEFORE adding the model(s) containing the reference. So in my case, even though the models.py was valid and complete at the beginning, I had to temporarily remove all the new dependent models to carefully generate a sequence of six separate migrations to get all my 12 legacy tables in with the correct FKey declarations in the migration scripts.

Being new to Django and the code base, I wanted to report this here, but I'm not sure whether the fix for this aspect is actually a separate related bug.

[Sven R. Kunze]

Bug is also present on 1.11.

[Daniel Naab]

I hit this bug also, and worked around by:

1) Making the models managed = True
2) Running manage.py makemigrations for those models
3) Manually switched all 'managed': True models in the migration to 'managed': False
4) Switch models back to managed = False

[Sergey Yurchenko]

Unmanaged models should not work in migrations by their nature.
You can make direct import of this model in migrations and use it.
I think it is expected behaviour not a bug.

[Simon Charette]

Sergey,

Unmanaged models should not work in migrations by their nature.

Is there a reason for that? How would the migration framework know how to generate references from managed models to unmanaged ones then (e.g. a foreign key)? The migration framework has supported managed model references for a while and even ​documents how to move an unmanaged model to a managed one.

You can make direct import of this model in migrations and use it.

Direct model imports in migrations will break over time if the model definition happen to change and won't work when mixed up with state/migration models. The ​documentation warns about this pattern.

I still believe this is a legitimate bug.

[Ponytech]

Replying to Daniel Naab:

I hit this bug also, and worked around by:

1) Making the models managed = True
2) Running manage.py makemigrations for those models
3) Manually switched all 'managed': True models in the migration to 'managed': False
4) Switch models back to managed = False

Same bug here and this workaround saved my day.
Thanks a lot!

[João Paulo Limberger - JOTAPE - Shoo]

Thanks! This workaround save my day Too!!!

Replying to Daniel Naab:

I hit this bug also, and worked around by:

1) Making the models managed = True
2) Running manage.py makemigrations for those models
3) Manually switched all 'managed': True models in the migration to 'managed': False
4) Switch models back to managed = False

Same bug here and this workaround saved my day.
Thanks a lot!

[Michal Mládek]

Currently, it seems there is a lot of questions about indended behavior of unmanaged models and related migrations. I will do the research tommorow and report the results here. Maybe if I will be lucky, I will add some recommendations.

[Michal Mládek]

There is an django project dj_bug_29177 contains code to trigger the bug reported here.

[Michal Mládek]

I have created a Django project named dj_bug_29177. It contains an app called foo with models and migrations written in such a way that the reported bug is triggered. Additionally, I’ve added code that generalizes the issue - this bug occurs for any column added to model B. The error disappears if I uncomment lines 19 and 20 in migration foo 0001. I confirm that the issue is not related to the SQL generated by the python manage.py migrate command, but rather to the python manage.py makemigrations command. The bug affects any column used in an unmanaged model, except for id. The solution is to fix the Django project code so that all fields defined on unmanaged models are included in automatically generated migrations. I will try to check today whether the ticket already contains a test; if not, I’ll attempt to write one. As it is described ​here it must be regression test.

[Michal Mládek]

Replying to Michal Mládek:
Finally, there is problem only with FK. I made a mistake while I was testing unmanaged models migrations.
​Here is current state of PR, still only with regression tests, one of them is more likely a test of solution.

I have created a Django project named dj_bug_29177. It contains an app called foo with models and migrations written in such a way that the reported bug is triggered. Additionally, I’ve added code that generalizes the issue - this bug occurs for any column added to model B. The error disappears if I uncomment lines 19 and 20 in migration foo 0001. I confirm that the issue is not related to the SQL generated by the python manage.py migrate command, but rather to the python manage.py makemigrations command. The bug affects any column used in an unmanaged model, except for id. The solution is to fix the Django project code so that all fields defined on unmanaged models are included in automatically generated migrations. I will try to check today whether the ticket already contains a test; if not, I’ll attempt to write one. As it is described ​here it must be regression test.

[Michal Mládek]

I decided to add unit tests to cover the new functionality of Autodetector which has no unit tests still.

[Jacob Walls]

​PR

There is a test failure. When you've got it patched up, please unset "Patch needs improvement", thanks!

[Michal Mládek]

I am sorry, I had misunderstand some things in CI/CD pipeline ... But now it should work fine.
Replying to Jacob Walls:

​PR

There is a test failure. When you've got it patched up, please unset "Patch needs improvement", thanks!