Model inheritance field name collision check error refers to related accessors as fields

[Matthew Schinckel]

I have a model structure that generates a models.E006 error under Django 1.9 onwards:

from django.db import models


class OrgUnit(models.Model):
    name = models.CharField(max_length=128)


class Organisation(OrgUnit):
    config = models.TextField(default='{}')


class Region(OrgUnit):
    organisation = models.ForeignKey(Organisation, related_name='regions')


class SubRegion(OrgUnit):
    region = models.ForeignKey(Region, related_name='sub_regions')

When running ./manage.py check, this results in:

SystemCheckError: System check identified some issues:

ERRORS:
org.Region.organisation: (models.E006) The field 'organisation' clashes with the field 'organisation' from model 'org.orgunit'.
org.SubRegion.region: (models.E006) The field 'region' clashes with the field 'region' from model 'org.orgunit'.

System check identified 2 issues (0 silenced).

However, I believe these are incorrect: the model OrgUnit does not have the fields listed.

This model structure worked fine in 1.8, but started failing in 1.9 (and still fails under 1.11).

I have a repository containing a minimal project at ​https://bitbucket.org/schinckel/inheritance-test

[Tim Graham]

Bisected to 4bc00defd0cf4de3baf90cad43da62e531987459. I think the warning is correct as there is a Region.organisation accessor that your foreign key is overriding. Using the models you provided but without with foreign keys:

u = OrgUnit.objects.create()
o = Organisation.objects.create(orgunit_ptr=u)
r = Region.objects.create(orgunit_ptr=u)
r.organisation == o

I think the warning's wording must be corrected to something like "The field 'organisation' clashes with the related accessor 'organisation' from model 'org.orgunit'." but I'm not certain about this analysis. It might be possible to continue to allow the overriding you've been doing -- I'm not sure if a patch is feasible.

[Tim Graham]

For your situation, I wonder if there's any reason not to make OrgUnit abstract?

[Matthew Schinckel]

Besides the fact that it's existing production code, and using an Abstract model would change the database structure, there actually is one reason.

A different model has a relationship to OrgUnit, and from the perspective of that model, it's important that it doesn't need to know which of the OrgUnit subclasses it relates to (i.e., as long as it relates to any OrgUnit, that's fine).

Having said that, I'm not entirely happy with that model structure, and I plan at some point to move it to a better structure (I have a similar setup that allows an arbitrary number of levels of hierarchy, for instance). However, I can't change that right now.

[Matthew Schinckel]

Ah, I think (thanks to your pointer) I understand a little better now: it's because a parent model always gets an accessor for each of it's subclasses? Is there a way to change that accessor name, I wonder?

[Matthew Schinckel]

Hmm. The documentation at ​https://docs.djangoproject.com/en/1.11/topics/db/models/#inheritance-and-reverse-relations suggests that it should already give a more informative error.

Reading more carefully, that seems to be a M2M relationship to the parent, however in this case it's an FK to a "sibling" model (even though in this specific case there is a hierarchy, but I think that's irrelevant).

I think that this error message (or something very similar) should be used for the situation I managed to find myself in.

[Paul Kenjora]

This ticket breaks abstraction, user defined fields now clash with framework defined fields. In addition the framework fields are inferred not explicit, giving the user no reasonable way to know ahead of time what the conflict may be, especially for 3rd party models that may end up clashing. Can we raise the severity of this since it forces a significant project rewrite ( rename all clashing fields ) from version 1.8 to 1.9+?

I'll continue investigating, for the curious the clash is caught at:

/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/django/db/models/base.py" [readonly] line 1453

Search for E006 in file. Looks like there is already an exception for id clash, do we add one for inherited models?

So looks like a work-around for this is defined at ( tested and works on mine ):

​https://stackoverflow.com/questions/40343238/django-inheritance-and-parent-object-related-name

[Clifford Gama]

The issue here is about how:

from django.db import models


class Parent(models.Model):
    pass


class Child(models.Model):
    child = TextField()

myapp.Child.child: (models.E006) The field 'child' clashes with the field 'child' from model 'myapp.parent'. which is intended and tested behavior in Django.

The issue being that Parent.child is not a field.

However:

>>>from myapp.models import Parent
>>>c = Parent._meta.get_field("child")
>>>assert c is not None  # Which is why it's being reported as one, I think
>>>c
<OneToOneRel: myapp.child>

After considering the options:

1. We add a note specifying that shadowing the reverse accessor in subclasses is not allowed. This option would add clarity but doesn't solve the problem directly. While useful, it doesn't address the fact that the error message is misleading by treating the reverse accessor like a field. This solution would not directly resolve the misunderstanding caused by the current error message.
2. Two messages in the check models.006, depending on whether the clash is between a field and a field or a field and an accessor. This approach would overload models.E006 to handle two separate cases, which I don't believe acceptable. (Each check ID for a single check.)
3. Create a new models.00xx error for this specific check. This option would maintain clear separation between different checks. However, it may be over-engineering for this small issue, and may lead to a bit more overhead in terms of adding and managing error codes.
4. Change the error message to: "myapp.Child.child: (models.E006) The field 'child' clashes with the field or accessor 'child' from model 'myapp.parent'" which accounts for the accessors.

I'm inclined to go with 4. Adding a note in the docs as well. LMKWYT