#29854: Altering the primary key targeted by several foreign keys incorrectly
alters the foreign key's NULL attribute
-------------------------------------+-------------------------------------
Reporter: Rick Yang | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: MySQL, Migration, | Triage Stage: Accepted
Altering primary key, |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Baptiste Mispelon):
* version: 1.11 => master
Comment:
After a bit of digging, I managed to get a consistent test failure on the
latest master (c33eb6dcd0c211f8f02b2976fe3b3463f0a54498) which helped me
diagnose a bit further.
Bear with me, it's going to be a bit verbose :)
== 1 - How to get a consistent failure
I took Tim's sample project (thanks Tim!) and managed to simplify it by
playing around with PYTHONHASHSEED [1] so I could get a consistent failure
(mysql only, as noted in the original ticket)..
Here are my models:
{{{#!python
class TestModel(models.Model):
filing_no = models.CharField(max_length=24, primary_key=True)
class OtherModel1(models.Model):
id = models.AutoField(primary_key=True)
f = models.ForeignKey(TestModel, null=True, on_delete=models.CASCADE)
class OtherModel2(models.Model):
id = models.AutoField(primary_key=True)
f = models.ForeignKey(TestModel, on_delete=models.CASCADE)
}}}
And here's the initial migration (note the last operation which is the
crux of this issue):
{{{#!python
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
initial = True
dependencies = [ ]
operations = [
migrations.CreateModel(
name='OtherModel1',
fields=[('id', models.AutoField(primary_key=True,
serialize=False))],
),
migrations.CreateModel(
name='OtherModel2',
fields=[('id', models.AutoField(primary_key=True,
serialize=False))],
),
migrations.CreateModel(
name='TestModel',
fields=[('filing_no', models.CharField(max_length=16,
primary_key=True, serialize=False))],
),
migrations.AddField(
model_name='othermodel2',
name='f',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
to='t29854.TestModel'),
),
migrations.AddField(
model_name='othermodel1',
name='f',
field=models.ForeignKey(null=True,
on_delete=django.db.models.deletion.CASCADE, to='t29854.TestModel'),
),
migrations.AlterField(
model_name='testmodel',
name='filing_no',
field=models.CharField(max_length=24, primary_key=True,
serialize=False),
),
]
}}}
With this setup, you can trigger an intermittent test failure with this
very simple test:
{{{#!python
class Tests(TestCase):
def test(self):
# Fails with IntegrityError: (1048, "Column 'f_id' cannot be
null")
OtherModel1.objects.create()
}}}
By playing with `PYTHONHASHSEED`, you can make the test failure
consistent. Start with `PYTHONHASHSEED=1 python manage.py test` and
increase by 1 until you get a failure (the magic number for me was
`PYTHONHASHSEED=2`, I'm not sure if that would work for other people as
well).
== 2 - Source of the issue
After much debugging I managed to end up at the same place as the original
reporter: `_related_non_m2m_objects` [2]:
{{{#!python
return zip(
(obj for obj in _all_related_fields(old_field.model) if
_is_relevant_relation(obj, old_field)),
(obj for obj in _all_related_fields(new_field.model) if
_is_relevant_relation(obj, new_field)),
)
}}}
With `_all_related_fields()` (defined in the same source file, just above)
being a wrapper around `model._meta.get_fields()` (with some hardcoded
parameters).
You can probably start to see where the problem is coming from:
`get_fields()` doesn't seem to guarantee a deterministic order of the
returned fields, which leads to old fields and new fields being mismatched
in some cases.
Even though this bug is present in all backends, it only seems to affect
mysql (I only tested mysql, sqlite and postgresql though).
It's mostly a lucky coincidence:
* The sqlite backend overrides `BaseDatabaseSchemaEditor._alter_field` and
doesn't make use of `related_non_m2m_objects`
* The postgresql hits the same problematic code path but has a different
way of handling the constraints (NULL vs NOT NULL) which avoids the
problem somehow.
== 3 - Fixing the problem
A quick workaround which seems to work is to change the code for
`_all_related_fields` so it's deterministic (adding a sort for example):
{{{#!python
def _all_related_fields(model):
all_fields = model._meta._get_fields(forward=False, reverse=True,
include_hidden=True)
return sorted(all_fields, key=lambda f: f.name)
}}}
I'm not sure if that's a good fix though: it feels like fixing a symptom
rather than the underlying problem.
I also wonder if there might be other tricky issues caused by the non-
deterministic nature of `meta.get_fields()` but I'm not sure how to
investigate that more systematically.
[1] https://docs.python.org/3/using/cmdline.html#envvar-PYTHONHASHSEED
[2]
https://github.com/django/django/blob/c33eb6dcd0c211f8f02b2976fe3b3463f0a54498/django/db/backends/base/schema.py#L38-L41
--
Ticket URL: <https://code.djangoproject.com/ticket/29854#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.566c4f05f330a070e9b2f0407ce5d8a6%40djangoproject.com.
