#14334 closed Bug (fixed)
Queries don't ensure that comparison objects are the correct type
Reported by: | Randy Barlow | Owned by: | ANUBHAV JOSHI |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This is best demonstrated by an example. Suppose you have the following models:
class A(models.Model): pass class B(models.Model): pass class C(models.Model): b = models.ForeignKey(B)
Then suppose you did this:
a = models.A.objects.create() b = models.B.objects.create() # This works, because the following assertion is True self.assertEquals(a.id, b.id) c = models.C.objects.create(b=b) # Let's try to query for a c where b==b. Works as expected self.assertEquals(models.C.objects.get(b=b).b, b) # Uh oh, why can I query for b=a? This first one shouldn't pass self.assertEquals(models.C.objects.get(b=a).b, b) # This one should fail, but it should fail at the .get() by # raising an exception, I think self.assertEquals(models.C.objects.get(b=a).b, a)
My proposal is that Django should complain when I pass in an instance of A for comparison with an FK to B, since a isn't the same as b. The query seems to just compare their primary keys, and not also ensure that the types are what is expected.
I've created a small django project that demonstrates this problem.
Attachments (1)
Change History (18)
by , 14 years ago
Attachment: | test_project.tar.bz2 added |
---|
comment:1 by , 14 years ago
Component: | Uncategorized → Core framework |
---|
The problem is demonstrated in the unit tests to the app. Thanks for your consideration!
comment:2 by , 14 years ago
For clarity, I propose that an exception should be raised by this code:
models.C.objects.get(b=a)
comment:3 by , 14 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Owner: | changed from | to
Status: | new → assigned |
This seems like something that should either raise an exception, or at least be noted by a warning in the documentation.
comment:4 by , 14 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Triage Stage: | Unreviewed → Accepted |
Bah, forgot "Accept ticket" doesn't set its state to Accepted, but makes me the owner :x
comment:5 by , 14 years ago
The only tricky thing with this is determining what kind of type check to do. If, for example, the a
object is actually any kind of proxy to an instance of models.B
it should not be rejected (by the normal Pythonic principles of duck typing). So a simple isinstance(the_model_class_we_expect)
will not do.
comment:6 by , 14 years ago
Component: | Core framework → Database layer (models, ORM) |
---|
comment:7 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:10 by , 11 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:11 by , 11 years ago
Has patch: | unset |
---|---|
Version: | 1.2 → master |
comment:13 by , 11 years ago
Needs documentation: | unset |
---|
comment:14 by , 11 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
ORM changes could use a final review.
comment:15 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Django project, demonstrating the problem