Bug in lookup with generic relation in model inheritance

[veena]

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"

[Tobias McNulty]

verifying this error

[Tobias McNulty]

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 )

[Tobias McNulty]

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 )

[Tobias McNulty]

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?)

[Tobias McNulty]

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 :)

[cheslavv]

When it will be fixed?

[jawn]

Uhh...this is a pretty big fucking bug in the ORM... Any chance of you gusy looking at this??

[Russell Keith-Magee]

@jawn: Offers of assistance are welcome. Abuse and foul language is not.

[Russell Keith-Magee]

Not critical for 1.2

[nator]

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?

[nator]

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.

[amarkor]

updated test

[amarkor]

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.

[Jacob]

Milestone 1.3 deleted

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[jaklaassen]

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.

[Ramiro Morales]

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.

[Ramiro Morales <cramm0@…>]

In c4b6659269260e0a9d75859bbe430b0971b1c15d:

Added test to demonstrate issue 11263 isn't there anymore.

Thanks veena for the report and jaklaassen for the patch. Fixes #11263.