Opened 18 years ago
Closed 17 years ago
#4193 closed (duplicate)
Error in creation of foreign key depends on spelling of field class
Reported by: | Owned by: | Philippe Raoult | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Keywords: | foreignKey, spelling, SQL error | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Summary
When a field class named "Vehicle" is attempted to be used as a foreign key, the SQL generated by django is incorrect.
Changing the name of the class to "Vahicle" or some other misspelling that does not begin with "Veh" results in the correct behavior.
Example
The following model definition does not work as expected. (N.B. I've trimmed some code to reduce the model definition to its essential bug-revealing form.)
from django.db import models import datetime class Vehicle(models.Model): type = models.CharField(maxlength=15) # e.g. truck, car manufacturer = models.CharField(maxlength=15) # e.g. Volkswagen, Isuzu theModel = models.CharField(maxlength=15) # e.g. Passat, Trooper year = models.IntegerField() def __str__(self): return repr(self.year) + ' ' + self.manufacturer + ' ' + self.theModel class Route(models.Model): locFrom = models.CharField('Starting Location', maxlength=15) locTo = models.CharField('Destination', maxlength=15) def __str__(self): return self.locFrom + ' to ' + self.locTo class Trip(models.Model): vehicle = models.ForeignKey(Vehicle) route = models.ForeignKey(Route) def __str__(self): return repr(self.start) + ' ' + repr(self.route) + ' ' + repr(self.vehicle)
The models.py file is in the commuteTime app in the commute project
yoshiAshi:django$ ls -al commute total 56 drwxr-xr-x 11 gerry gerry 374 Apr 30 23:58 . drwxr-xr-x 8 gerry gerry 272 May 1 08:33 .. -rw-r--r-- 1 gerry gerry 0 Apr 30 07:38 __init__.py -rw-r--r-- 1 gerry gerry 157 Apr 30 08:07 __init__.pyc drwxr-xr-x 9 gerry gerry 306 May 1 08:33 commuteTime drwxr-xr-x 3 gerry gerry 102 Apr 30 23:32 db -rw-r--r-- 1 gerry gerry 546 Apr 30 07:38 manage.py -rw-r--r-- 1 gerry gerry 2828 Apr 30 23:06 settings.py -rw-r--r-- 1 gerry gerry 1824 Apr 30 23:06 settings.pyc -rw-r--r-- 1 gerry gerry 227 Apr 30 07:38 urls.py yoshiAshi:django$ ls -al commute/commuteTime total 48 drwxr-xr-x 9 gerry gerry 306 May 1 08:33 . drwxr-xr-x 11 gerry gerry 374 Apr 30 23:58 .. -rw-r--r-- 1 gerry gerry 0 Apr 30 08:10 __init__.py -rw-r--r-- 1 gerry gerry 169 Apr 30 23:01 __init__.pyc drwxr-xr-x 3 gerry gerry 102 Apr 30 08:13 db -rw-r--r-- 1 gerry gerry 876 May 1 08:13 models.py -rw-r--r-- 1 gerry gerry 2312 May 1 08:13 models.pyc -rw-r--r-- 1 gerry gerry 26 Apr 30 08:10 views.py
Testing the SQL generation give this
yoshiAshi:commute$ python manage.py sql commuteTime BEGIN; CREATE TABLE "commuteTime_route" ( "id" integer NOT NULL PRIMARY KEY, "locFrom" varchar(15) NOT NULL, "locTo" varchar(15) NOT NULL ); CREATE TABLE "commuteTime_trip" ( "id" integer NOT NULL PRIMARY KEY, "vehicle_id" integer NOT NULL, "route_id" integer NOT NULL REFERENCES "commuteTime_route" ("id") ); CREATE TABLE "commuteTime_vehicle" ( "id" integer NOT NULL PRIMARY KEY, "type" varchar(15) NOT NULL, "manufacturer" varchar(15) NOT NULL, "theModel" varchar(15) NOT NULL, "year" integer NOT NULL ); COMMIT;
Note that the commuteTime_trip table does not have commuteTime_vehicle as a proper foreign key. We would expect
"vehicle_id" integer NOT NULL REFERENCES "commuteTime_vehicle" ("id"),
but instead get
"vehicle_id" integer NOT NULL,
Work-Around
Misspelling "Vehicle" results in the correct behavior. For example if the Vehicle class and foreign key reference are replaced by
class Vahicle(models.Model): type = models.CharField(maxlength=15) # e.g. truck, car manufacturer = models.CharField(maxlength=15) # e.g. Volkswagen, Isuzu theModel = models.CharField(maxlength=15) # e.g. Passat, Trooper year = models.IntegerField() ... class Trip(models.Model): vehicle = models.ForeignKey(Vahicle) route = models.ForeignKey(Route) ...
the correct SQL is generated
yoshiAshi:commute$ python manage.py sql commuteTime BEGIN; CREATE TABLE "commuteTime_route" ( "id" integer NOT NULL PRIMARY KEY, "locFrom" varchar(15) NOT NULL, "locTo" varchar(15) NOT NULL ); CREATE TABLE "commuteTime_vahicle" ( "id" integer NOT NULL PRIMARY KEY, "type" varchar(15) NOT NULL, "manufacturer" varchar(15) NOT NULL, "theModel" varchar(15) NOT NULL, "year" integer NOT NULL ); CREATE TABLE "commuteTime_trip" ( "id" integer NOT NULL PRIMARY KEY, "vehicle_id" integer NOT NULL REFERENCES "commuteTime_vahicle" ("id"), "route_id" integer NOT NULL REFERENCES "commuteTime_route" ("id") ); COMMIT;
I've experimented with different spellings: "Ve" gives the correct foreign key
definition, but "Veh" does not.
Weird.
Hardware and Software Configuration
Mac OS 10.4.9
python 2.3.5
django svn trunk revision 5127
Database: sqlite (for development server)
I started the discovery of this bug on another Mac running python 2.5, so I don't think it depends on the python version. I'll check later today, or maybe tonight, depending on when I get the time.
Attachments (1)
Change History (22)
comment:1 by , 18 years ago
Version: | 0.96 → SVN |
---|
follow-up: 3 comment:2 by , 18 years ago
comment:3 by , 18 years ago
Replying to SmileyChris:
Thanks for the report, Gerry - weird sounding bug...
I know it's asking a lot, but do you think you could make a model test for this?
I started looking into this yesterday, but got distracted by my real(TM) job. :)
I'll give it a try, but my effort will be sporadic.
I dug through the django documentation and found this page:
By "model test" I assume building a doctest string in the models.py file.
I've thought about it, but I haven't gotten any traction on how to test
that the correct SQL was created. Any suggestions on that front are welcome.
Gerry
comment:4 by , 17 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 17 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
comment:6 by , 17 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
SQLite does not support foreign key constraints, though they are parsed. The models are stored as dictionary values per-application, so the ordering of when statements are created is not guaranteed. The SQLite backend has supports_constraints set to False, and thus sql_for_pending_references() does not add the ALTER TABLE statements to add the constraints.
The only problem I see is since the constraint isn't added to the query, the dump of the database does not contain the constraint and executing the queries on another DB like PostgreSQL will not have the proper constraints.
The solution would be to add ordering. Since Vehicle was defined before Trip, Vehicle should already be a known model by the time the constraint is needed in Trip.
comment:7 by , 17 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:9 by , 17 years ago
#2720 mentions the fact that foreign keys might be created after the models. Obviously it doesn't always work.
by , 17 years ago
Attachment: | 4193_model_registry_fix.diff added |
---|
possible patch that maintains model ordering in the app cache since SQL creation depends on it.
comment:11 by , 17 years ago
Triage Stage: | Design decision needed → Accepted |
---|
follow-up: 14 comment:13 by , 17 years ago
comment:14 by , 17 years ago
comment:15 by , 17 years ago
Marked #6374 as a duplicate. It contains a patch as well. I haven't looked into this for a while so not sure what is the correct fix in this matter.
comment:16 by , 17 years ago
comment:17 by , 17 years ago
With reference to comment 14, do you mean the constraint statement is missing for some backend other than SQLite? If so, which backend?
There won't be any constraint for SQLite, since it doesn't support it and the goal isn't to generate SQL for one backend that is completely portable to another one (there are too many subtle differences between backends, so the thought in comment 6, whilst well intentioned, isn't really practical).
comment:18 by , 17 years ago
comment:19 by , 17 years ago
The patch in #6374 looks like it's closer to the root cause of fixing this problem. I haven't completely convinced myself it's the right thing yet, but that's a matter of having time to work through what's going on in there. Just noting it here in case another maintainer comes along wanting to work on this.
comment:20 by , 17 years ago
comment:21 by , 17 years ago
Resolution: | → duplicate |
---|---|
Status: | assigned → closed |
Thanks for the report, Gerry - weird sounding bug...
I know it's asking a lot, but do you think you could make a model test for this?