Opened 11 years ago

Closed 11 years ago

#22182 closed Bug (duplicate)

Reverse accessor should not be added for models with `ForeignKey` fields belonging to apps not installed.

Reported by: Tai Lee Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Normal Keywords: OperationalError no such table
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It seems that the act of importing a model that has a ForeignKey, even when the imported model is located in an app that is not installed, will add a reverse accessor to the target model.

Django might be using the accessor directly (or another internal mechanism) in its deletion collector, which therefore references a table that does not exist.

The end result is that simply importing a model for an optional contrib app that is not installed can break the deletion of other model objects (OperationalError: no such table: ... exception), and perhaps other operations.

It can be an easy mistake to make, to import a model from an optional app that might not end up being installed in a particular project. It can also be difficult to track down when you get errors about a table not existing for an app that you have not installed and are not using, when there is only deep Django ORM code referenced in the traceback.

For example, say you have a lib/module that provides several middleware classes, and one of them works with django.contrib.comments. You might assume that it's safe to import django.contrib.comments at the top of your middleware module, because it will not be an ImportError even if the comments app is not installed, and your middleware methods won't run unless explicitly enabled.

But you will get OperationalError: no such table: django_comments if you enable any other middleware from that module and you attempt to delete a User object. Here's an interactive shell session demonstrating the problem as simply as possible:

>>> from django.contrib.auth.models import *
>>> from django.contrib import comments
>>> u = User.objects.latest('pk')
>>> u.delete()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/models/base.py", line 694, in delete
    collector.collect([self])
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/models/deletion.py", line 196, in collect
    elif sub_objs:
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/models/query.py", line 100, in __nonzero__
    self._fetch_all()
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/models/query.py", line 854, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/models/query.py", line 220, in iterator
    for row in compiler.results_iter():
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/models/sql/compiler.py", line 712, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/models/sql/compiler.py", line 788, in execute_sql
    cursor.execute(sql, params)
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/backends/util.py", line 69, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/backends/util.py", line 53, in execute
    return self.cursor.execute(sql, params)
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/utils.py", line 99, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/backends/util.py", line 53, in execute
    return self.cursor.execute(sql, params)
  File "/Users/tailee/.virtualenvs/openturbia/src/django/django/db/backends/sqlite3/base.py", line 450, in execute
    return Database.Cursor.execute(self, query, params)
OperationalError: no such table: django_comments

A possible fix might be to check if the app that owns a model being imported is installed before adding reverse accessors. If it is not installed, either:

a) don't add the reverse accessor; or
b) raise an exception loudly when the error is easy to diagnose, instead of waiting until runtime where is difficult to trace.

Another consideration might be for the case where a contrib app is installed, but is later removed from INSTALLED_APPS. So the database table does exist, in which case it may still be necessary for Django to account for the table in it's "fast delete" SQL.

So perhaps it would be better to check if the database table exists, rather than if the app is installed? That could potentially cause a chicken and egg problem for the syncdb management command, though?

Change History (1)

comment:1 by Aymeric Augustin, 11 years ago

Resolution: duplicate
Status: newclosed

There's already a ticket for option (b): #21680.

Note: See TracTickets for help on using tickets.
Back to Top