Opened 2 years ago

Last modified 6 months ago

#28110 new Bug

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

Reported by: Matthew Schinckel Owned by: nobody
Component: Core (System checks) Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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

Change History (6)

comment:1 Changed 2 years ago by Tim Graham

Component: Database layer (models, ORM)Core (System checks)
Summary: Model inheritance mistakenly associates child fields with parent (when running checks)Model inheritance field name collision check error refers to related accessors as fields
Triage Stage: UnreviewedAccepted

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.

comment:2 Changed 2 years ago by Tim Graham

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

comment:3 Changed 2 years ago by 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.

Last edited 2 years ago by Matthew Schinckel (previous) (diff)

comment:4 Changed 2 years ago by 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?

comment:5 Changed 2 years ago by 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.

comment:6 Changed 6 months ago by 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?

Version 0, edited 6 months ago by Paul Kenjora (next)
Note: See TracTickets for help on using tickets.
Back to Top