Opened 15 years ago
Closed 12 years ago
#11263 closed Bug (fixed)
Bug in lookup with generic relation in model inheritance
Reported by: | veena | Owned by: | nobody |
---|---|---|---|
Component: | contrib.contenttypes | Version: | dev |
Severity: | Normal | Keywords: | generic relations, model inheritance |
Cc: | ramusus@…, nator | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
class Order(models.Model): content_type = models.ForeignKey(ContentType) object_id = models.PositiveIntegerField() content_object = generic.GenericForeignKey() status = models.CharField(max_length=10, default='ordered') class Visitor(Person): order = generic.GenericRelation(Order) Visitor.objects.filter(order__status='ordered').count()
It creates this broken query:
SELECT COUNT(*) FROM "expo_visitor" INNER JOIN "expo_order" ON ("expo_visitor"."person_ptr_id" = "expo_order"."object_id") INNER JOIN "expo_order" T3 ON ("expo_order"."person_ptr_id" = T3."id") WHERE (T3."status" = %s AND T3."content_type_id" = %s )'
There is non existent column "expo_order"."person_ptr_id"
Attachments (2)
Change History (28)
comment:1 by , 15 years ago
milestone: | → 1.2 |
---|
comment:2 by , 15 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Needs documentation: | set |
Needs tests: | set |
comment:3 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 15 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 15 years ago
verified that WITHOUT inheritance this works OK:
class Order(models.Model): content_type = models.ForeignKey(ContentType) object_id = models.PositiveIntegerField() content_object = generic.GenericForeignKey() status = models.CharField(max_length=10, default='ordered') class Visitor(models.Model): # DOES NOT have parent model order = generic.GenericRelation(Order)
SELECT COUNT(*) FROM "testapp_visitor" INNER JOIN "testapp_order" ON ("testapp_visitor"."id" = "testapp_order"."object_id") WHERE ("testapp_order"."status" = E\'ordered\' AND "testapp_order"."content_type_id" = 9 )
comment:6 by , 15 years ago
WITH inheritance it still breaks in trunk:
rom django.db import models from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes import generic from django.contrib.auth.models import User class Order(models.Model): content_type = models.ForeignKey(ContentType) object_id = models.PositiveIntegerField() content_object = generic.GenericForeignKey() status = models.CharField(max_length=10, default='ordered') class Visitor(User): # HAS a parent model order = generic.GenericRelation(Order) Visitor.objects.filter(order__status='ordered').count() # generates: ProgrammingError: column testapp_order.user_ptr_id does not exist
SELECT COUNT(*) FROM "testapp_visitor" INNER JOIN "testapp_order" ON ("testapp_visitor"."user_ptr_id" = "testapp_order"."object_id") INNER JOIN "testapp_order" T3 ON ("testapp_order"."user_ptr_id" = T3."id") WHERE (T3."status" = E\'ordered\' AND T3."content_type_id" = 9 )
by , 15 years ago
Attachment: | generic_relations_inheritance_order_test.diff added |
---|
comment:7 by , 15 years ago
added a patch with a test for this bug. (it's there but it doesn't show up in the Trac diff viewer - any idea why?)
comment:8 by , 15 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
I may be able to fix this but bumping back to nobody in case someone else gets to it before I do. I'll re-accept the ticket if/when I have time to work on it.
Any pointers to what code generates this SQL would be appreciated :)
comment:10 by , 15 years ago
Uhh...this is a pretty big fucking bug in the ORM... Any chance of you gusy looking at this??
comment:11 by , 15 years ago
@jawn: Offers of assistance are welcome. Abuse and foul language is not.
comment:12 by , 15 years ago
Cc: | added |
---|
follow-up: 14 comment:13 by , 15 years ago
Has patch: | set |
---|
comment:15 by , 14 years ago
Caveat: I'm new to Django and Python.
That said, I did some poking around in django/db/models/sql/query.py and found a tweak that "fixes" this. In the setup_joins function there's a block of code that looks like this (it's within the test block for direct, and then within the test for m2m):
if int_alias == table2 and from_col2 == to_col2: joins.append(int_alias) alias = int_alias else: alias = self.join( (int_alias, table2, from_col2, to_col2), dupe_multis, exclusions, nullable=True, reuse=can_reuse) joins.extend([int_alias, alias])
The problem with the bad SQL being generated is a redundant and wrong inner_join. If I change that first test to:
if int_alias == table2 and from_col2 == from_col1:
it works. (basically changing the test to check for this redundant situation)
I don't really recommend this as a patch since I don't fully understand the ramifications of the change - what else am I breaking by doing this?
comment:16 by , 14 years ago
I also did some testing with various ways of doing the inheritance. Say we have a non-abstract superclass and a subclass, and instead of adding the m2m relationship to the subclass we add it to the superclass -- does it still break that way?
class Order(models.Model): content_type = models.ForeignKey(ContentType) object_id = models.PositiveIntegerField() content_object = generic.GenericForeignKey() status = models.CharField(max_length=10, default='ordered') class Media(models.Model): order = generic.GenericRelation(Order) class Photo(Media): title = models.CharField() Photo.objects.filter(order__status='ordered').count() --> doesn't break, but always returns 0.
So the answer is yes, it's still broken, but differently. The basic problem is that when the Order object is created Django inserts it into the DB using the content_type of Photo, but when the select is run Django realizes Order actually lives in Media and tries to use that content_type. So the inserts all "work", but the count() never finds them. It's a mismatch between the content_type_id when it runs the insert and select.
After digging through django/config/contenttypes/generic.py a bit I feel like the correct answer is to change the inserts to use the content_type of the class that actually contains the m2m relationship. (Media, in this case) Some quick hacking proves this work, but as with the above note I know I'm not familiar enough with the logic here to safely make this change.
Sorry to not be more help, but I think between this note and the last it should give someone a good shot at fixing this if they understand the codebase better than me.
comment:17 by , 14 years ago
Cc: | added |
---|
comment:18 by , 14 years ago
Can confirm that at least the content_type_id between the two queries is different.
Photo.objects.filter(order__status='ordered').query
SELECT "generic_relations_regress_media"."id", "generic_relations_regress_photo"."media_ptr_id" FROM "generic_relations_regress_photo" INNER JOIN "generic_relations_regress_media" ON ("generic_relations_regress_photo"."media_ptr_id" = "generic_relations_regress_media"."id") INNER JOIN "generic_relations_regress_order" ON ("generic_relations_regress_media"."id" = "generic_relations_regress_order"."object_id") WHERE ("generic_relations_regress_order"."status" = ordered AND "generic_relations_regress_order"."content_type_id" = 26 )
photo.order.filter(status='ordered').query
SELECT "generic_relations_regress_order"."id", "generic_relations_regress_order"."content_type_id", "generic_relations_regress_order"."object_id", "generic_relations_regress_order"."status" FROM "generic_relations_regress_order" WHERE ("generic_relations_regress_order"."object_id" = 1 AND "generic_relations_regress_order"."content_type_id" = 27 AND "generic_relations_regress_order"."status" = ordered )
patch attached.
comment:19 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:20 by , 14 years ago
Component: | Database layer (models, ORM) → contrib.contenttypes |
---|
follow-up: 14 comment:13 by , 12 years ago
There are two separate issues:
- The original reporter's issue with invalid SQL
- The issue raised by nator with filtering concrete model subclasses based on a generic relation in the parent.
Patch is here: https://github.com/jaklaassen/django/compare/ticket_11263
The original issue appears to be fixed in trunk and the patch includes a passing test for this.
The second issue happens because the incorrect content_type_id is used in the query.
This is still happening in trunk, and is fixed by the patch.
comment:14 by , 12 years ago
Replying to jaklaassen:
- The issue raised by nator with filtering concrete model subclasses based on a generic relation in the parent.
Patch is here: https://github.com/jaklaassen/django/compare/ticket_11263
The second issue happens because the incorrect content_type_id is used in the query.
This is still happening in trunk, and is fixed by the patch.
I've opened #19722 to track that issue.
comment:15 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
verifying this error