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?
There's already a ticket for option (b): #21680.