Inheritance of fields from a single abstract base class through multiple abstract classes causes errors.

[Stephen Burrows]

Assume the following models in app:

from django.db import models


class Orange(models.Model):
    pass


class BaseClass(models.Model):
    field = models.ForeignKey(Orange)
    class Meta:
        abstract = True


class Parent1(BaseClass):
    class Meta:
        abstract=True


class Parent2(BaseClass):
    class Meta:
        abstract=True


class Child(Parent1, Parent2):
    pass

Currently, this definition will raise the following errors during model validation:

Unhandled exception in thread started by <bound method Command.inner_run of <django.core.management.commands.runserver.Command object at 0x101470490>>
Traceback (most recent call last):
  File "..../django/core/management/commands/runserver.py", line 88, in inner_run
    self.validate(display_num_errors=True)
  File "..../django/core/management/base.py", line 253, in validate
    raise CommandError("One or more models did not validate:\n%s" % error_text)
django.core.management.base.CommandError: One or more models did not validate:
app.child: Accessor for field 'field' clashes with related field 'Orange.child_set'. Add a related_name argument to the definition for 'field'.
app.child: Accessor for field 'field' clashes with related field 'Orange.child_set'. Add a related_name argument to the definition for 'field'.

Using the %(app_label)s_%(class)s_related syntax only makes things worse:

Unhandled exception in thread started by <bound method Command.inner_run of <django.core.management.commands.runserver.Command object at 0x10146e4d0>>
Traceback (most recent call last):
  File "..../django/core/management/commands/runserver.py", line 88, in inner_run
    self.validate(display_num_errors=True)
  File "..../django/core/management/base.py", line 253, in validate
    raise CommandError("One or more models did not validate:\n%s" % error_text)
django.core.management.base.CommandError: One or more models did not validate:
app.child: Accessor for field 'field' clashes with related field 'Orange.app_child_related'. Add a related_name argument to the definition for 'field'.
app.child: Reverse query name for field 'field' clashes with related field 'Orange.app_child_related'. Add a related_name argument to the definition for 'field'.
app.child: Accessor for field 'field' clashes with related field 'Orange.app_child_related'. Add a related_name argument to the definition for 'field'.
app.child: Reverse query name for field 'field' clashes with related field 'Orange.app_child_related'. Add a related_name argument to the definition for 'field'.

Instead of causing errors, it seems like the field should only be inherited once from BaseClass. My patch handles this as follows: On each field instance, track the class it was originally declared for and the first non-abstract class that it shows up in. Then, when a field is being added to a class, check to make sure that it only gets added if it isn't a "duplicate". (The patch will incidentally also need to move the get_FIELD_display method declaration into cls._meta.add_field.)

[Stephen Burrows]

See initial discussion at ​http://groups.google.com/group/django-developers/browse_thread/thread/f980f1fa6a62e4ba.

[Stephen Burrows]

I've added a version of the ticket that should accomplish what I'm trying to do. Unfortunately, it seems to cause some issues... the test suite has a number of exceptions which don't otherwise appear. I will look into it.

======================================================================
ERROR: test_more_more_more (regressiontests.aggregation_regress.tests.AggregationTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../django/tests/regressiontests/aggregation_regress/tests.py", line 631, in test_more_more_more
    qs = HardbackBook.objects.annotate(n_authors=Count('authors')).values('name', 'n_authors')
  File ".../django/django/db/models/manager.py", line 147, in annotate
    return self.get_query_set().annotate(*args, **kwargs)
  File ".../django/django/db/models/query.py", line 644, in annotate
    is_summary=False)
  File ".../django/django/db/models/sql/query.py", line 965, in add_aggregate
    field_list, opts, self.get_initial_alias(), False)
  File ".../django/django/db/models/sql/query.py", line 1236, in setup_joins
    "Choices are: %s" % (name, ", ".join(names)))
FieldError: Cannot resolve keyword 'authors' into field. Choices are: book_ptr, contact, id, isbn, name, pages, price, pubdate, publisher, rating, store, weight

======================================================================
ERROR: test_inherited_fields (regressiontests.model_inheritance_regress.tests.ModelInheritanceTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../django/tests/regressiontests/model_inheritance_regress/tests.py", line 253, in test_inherited_fields
    m2mchildren = list(M2MChild.objects.filter(articles__isnull=False))
  File ".../django/django/db/models/manager.py", line 141, in filter
    return self.get_query_set().filter(*args, **kwargs)
  File ".../django/django/db/models/query.py", line 550, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File ".../django/django/db/models/query.py", line 568, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File ".../django/django/db/models/sql/query.py", line 1170, in add_q
    can_reuse=used_aliases, force_having=force_having)
  File ".../django/django/db/models/sql/query.py", line 1058, in add_filter
    negate=negate, process_extras=process_extras)
  File ".../django/django/db/models/sql/query.py", line 1236, in setup_joins
    "Choices are: %s" % (name, ", ".join(names)))
FieldError: Cannot resolve keyword 'articles' into field. Choices are: id, m2mbase_ptr, name

======================================================================
ERROR: test_table_exists (modeltests.proxy_model_inheritance.tests.ProxyModelInheritanceTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../django/tests/modeltests/proxy_model_inheritance/tests.py", line 25, in setUp
    call_command('syncdb', verbosity=0)
  File ".../django/django/core/management/__init__.py", line 166, in call_command
    return klass.execute(*args, **defaults)
  File ".../django/django/core/management/base.py", line 220, in execute
    output = self.handle(*args, **options)
  File ".../django/django/core/management/base.py", line 351, in handle
    return self.handle_noargs(**options)
  File ".../django/django/core/management/commands/syncdb.py", line 121, in handle_noargs
    custom_sql = custom_sql_for_model(model, self.style, connection)
  File ".../django/django/core/management/sql.py", line 150, in custom_sql_for_model
    app_dir = os.path.normpath(os.path.join(os.path.dirname(models.get_app(model._meta.app_label).__file__), 'sql'))
  File ".../django/django/db/models/loading.py", line 140, in get_app
    raise ImproperlyConfigured("App with label %s could not be found" % app_label)
ImproperlyConfigured: App with label model_inheritance could not be found

======================================================================
ERROR: test_distinct_for_inherited_m2m_in_list_filter (regressiontests.admin_changelist.tests.ChangeListTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../django/tests/regressiontests/admin_changelist/tests.py", line 197, in test_distinct_for_inherited_m2m_in_list_filter
    m.list_editable, m)
  File ".../django/django/contrib/admin/views/main.py", line 66, in __init__
    self.query_set = self.get_query_set()
  File ".../django/django/contrib/admin/views/main.py", line 193, in get_query_set
    raise IncorrectLookupParameters
IncorrectLookupParameters

======================================================================
FAIL: test_bug_8245 (regressiontests.bug8245.tests.Bug8245Test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../django/tests/regressiontests/bug8245/tests.py", line 18, in test_bug_8245
    'autodiscover should have raised a "Bad admin module" error.')
AssertionError: autodiscover should have raised a "Bad admin module" error.

======================================================================
FAIL: test_load_error (regressiontests.templates.tests.TemplateTagLoading)
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../django/tests/regressiontests/templates/tests.py", line 1578, in test_load_error
    self.assertTrue('ImportError' in e.args[0])
AssertionError: False is not True

----------------------------------------------------------------------

[Stephen Burrows]

It should also be noted that my "test" currently consists of checking for duplicate fields. Declaring the model in a models.py file prevented runtests from working, and custom validation modeled after management.validation failed to find any errors.[1] If anyone has a suggestion on how to test more directly, I'd love to hear it.

[1] ​https://github.com/melinath/django/commit/a872227b1724c098d3a9d3bf8eaa87e0c6644a52

[Stephen Burrows]

This version of the patch works a lot better. The tests actually test things, and the behavior is more consistent. However, I'm starting to wonder if this is really the way to go about this. I feel like I'm constantly struggling against the way that Options keeps track of fields. For example, I had to add code to the _fill_x_cache methods on Options to eliminate duplication there, because options.add_field isn't called for inherited fields. This makes sense, generally, since in the past, fields have not known what models they were attached to. However, since this patch introduces field-based tracking of both the model the field was first declared on and the first concrete model the field was present on, it would be possible to take that farther. Options could add every field on the model to a cache, then dish out local fields or inherited fields based on whether the options' model is the same as the first concrete model for the field.

However, a change like that would be fairly invasive and far beyond the scope of this patch, nor am I convinced that it is the best course of action in terms of efficiency.

[Preston Holmes]

#19465 demonstrates that this issue is in no way specific to related fields - but fields in general

[Baptiste Mispelon]

I could reproduce this error on 1.9 but not on 1.10. I bisected the issue and it looks like it got fixed in 85ef98dc6ec565b1add417bd76808664e7318026.

The fix includes tests that cover ​multiple inheritance from abstract models so I think this ticket can simply be marked as fixed.