Django

Code

Ticket #5937 (closed: fixed)

Opened 1 year ago

Last modified 3 months ago

Generic Relations backward lookups generate invalid SQL

Reported by: max Assigned to: mtredinnick
Milestone: 1.0 Component: Contrib apps
Version: SVN Keywords: generic relation, qs-rf
Cc: jag@flowtheory.net, drackett@mac.com, aaron@earthman.ca, matt@eitheror.org, wonlay@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

Given the following model is would seem that the Generic Relations' GenericRelatedObjectManager? generates invalid SQL, as the doctest in the following snippet proves:

from django.db import models
from django.contrib.contenttypes.generic import GenericForeignKey, GenericRelation
from django.contrib.contenttypes.models import ContentType

#==============================================================================
class Alert(models.Model):
    """
    Alerts for all kinds of widgets
    
    # create a blue widget and add an alert
    >>> blue = BlueWidget.objects.create(name='Blue 1')
    >>> alert = Alert(name='alert 1')
    >>> alert.object = blue
    >>> alert.save()
    
    # create a red widget and add an alert
    >>> red = RedWidget.objects.create(name='Red 1')
    >>> alert = Alert(name='alert 2')
    >>> alert.object = red
    >>> alert.save()
    
    # create a red widget with no alert
    >>> red = RedWidget.objects.create(name='Red 2')
    
    # create a blue widget and add an alert
    >>> blue = BlueWidget.objects.create(name='Blue 2')
    >>> alert = Alert(name='alert 3')
    >>> alert.object = blue
    >>> alert.save()
    
    # get a list of red widgets with alerts - there should be only 1
    >>> reds = RedWidget.objects.filter(alerts__name__contains='alert').distinct()
    >>> reds
    [<RedWidget: RedWidget Red 1>]
    
    # this currently yields: [<RedWidget: RedWidget Red 1>, <RedWidget: RedWidget Red 2>]
    
    """
    created = models.DateTimeField(auto_now_add=True)
    name = models.CharField(maxlength=128)
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    object = GenericForeignKey('content_type', 'object_id')
    
    #--------------------------------------------------------------------------
    def __str__(self):
        return '%s alert for %s widget' % (self.name, self.object)
    
#==============================================================================
class RedWidget(models.Model):
    """
    The red kind of widget
    """
    created = models.DateTimeField(auto_now_add=True)
    name = models.CharField(maxlength=128)
    alerts = GenericRelation(Alert)
    
    #--------------------------------------------------------------------------
    def __str__(self):
        return 'RedWidget %s' % (self.name)
    
#==============================================================================
class BlueWidget(models.Model):
    """
    The blue kind of widget
    """
    created = models.DateTimeField(auto_now_add=True)
    name = models.CharField(maxlength=128)
    alerts = GenericRelation(Alert)
    
    #--------------------------------------------------------------------------
    def __str__(self):
        return 'BlueWidget %s' % (self.name)    
    

Following the doctest above, the generated SQL after the last statement in the doctest:

reds = RedWidget.objects.filter(alerts__name__contains='alert').distinct()

is this:

SELECT DISTINCT `testapp_redwidget`.`id`,`testapp_redwidget`.`created`,`testapp_redwidget`.`name` 
FROM `testapp_redwidget` 
LEFT OUTER JOIN `testapp_alert` AS `m2m_testapp_redwidget__alerts` ON `testapp_redwidget`.`id` = `m2m_testapp_redwidget__alerts`.`object_id` 
INNER JOIN `testapp_alert` AS `testapp_redwidget__alerts` ON `m2m_testapp_redwidget__alerts`.`id` = `testapp_redwidget__alerts`.`id` 
WHERE (`testapp_redwidget__alerts`.`name` LIKE '%alert%')

That is - although it is linking to the right table (redwidget) it is not filtering on content type in the alert table. As a result it picks up the id of the second blue widget (2), which matches the id of the second, alert-less red widget (2) and links it in the query to produce the buggy result set.

One would expect this SQL to rather look like this:

SELECT DISTINCT `testapp_redwidget`.`id`,`testapp_redwidget`.`created`,`testapp_redwidget`.`name` 
FROM `testapp_redwidget` 
INNER JOIN `testapp_alert` AS `testapp_redwidget__alerts` ON `testapp_redwidget`.`id` = `testapp_redwidget__alerts`.`object_id` 
INNER JOIN `django_content_type` AS `testapp_redwidget__django_content_type` ON `testapp_redwidget__alerts`.`content_type_id` = `testapp_redwidget__django_content_type`.`id`
WHERE (`testapp_redwidget__alerts`.`name` LIKE '%alert%')
AND (`testapp_redwidget__django_content_type`.`app_label` = 'testapp')
AND (`testapp_redwidget__django_content_type`.`model` = 'redwidget')

.. or, even better (and because I don't quite understand the reason for the LEFT OUTER join):

SELECT DISTINCT `testapp_redwidget`.`id`,`testapp_redwidget`.`created`,`testapp_redwidget`.`name` 
FROM `testapp_redwidget` 
INNER JOIN `testapp_alert` AS `testapp_redwidget__alerts` ON `testapp_redwidget`.`id` = `testapp_redwidget__alerts`.`object_id` 
WHERE (`testapp_redwidget__alerts`.`name` LIKE '%alert%')
AND (`testapp_redwidget__alerts`.`content_type_id` = 9)

The only current workaround for this is to explicitly filter on content type id - this works:

>>> ctype = ContentType.objects.get_for_model(RedWidget)
>>> reds = RedWidget.objects.filter(alerts__name__contains='alert', alerts__content_type=ctype).distinct()
>>> reds
[<RedWidget: RedWidget Red 1>]

Attachments

django-generic-relation-query.patch (2.4 kB) - added by Joshua 'jag' Ginsberg <jag@flowtheory.net> on 11/20/07 21:08:07.

Change History

11/15/07 03:18:08 changed by mtredinnick

  • keywords changed from generic relation to generic relation, qs-rf.
  • needs_better_patch changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • needs_docs changed.

11/20/07 21:07:12 changed by Joshua 'jag' Ginsberg <jag@flowtheory.net>

  • cc set to jag@flowtheory.net.
  • has_patch set to 1.

I believe the attached patch fixes the issue.

-jag

11/20/07 21:08:07 changed by Joshua 'jag' Ginsberg <jag@flowtheory.net>

  • attachment django-generic-relation-query.patch added.

01/08/08 15:22:03 changed by korpios

  • cc changed from jag@flowtheory.net to jag@flowtheory.net, korpios@korpios.com.

02/27/08 20:23:47 changed by anonymous

  • cc changed from jag@flowtheory.net, korpios@korpios.com to jag@flowtheory.net, korpios@korpios.com, drackett@mac.com.

07/06/08 04:01:21 changed by mtredinnick

  • owner changed from nobody to mtredinnick.

Somehow I forgot to grab this. It's still being worked on.

07/06/08 19:18:27 changed by anonymous

  • cc changed from jag@flowtheory.net, korpios@korpios.com, drackett@mac.com to jag@flowtheory.net, korpios@korpios.com, drackett@mac.com, aaron@earthman.ca.

07/24/08 16:01:50 changed by mattgrayson

  • cc changed from jag@flowtheory.net, korpios@korpios.com, drackett@mac.com, aaron@earthman.ca to jag@flowtheory.net, korpios@korpios.com, drackett@mac.com, aaron@earthman.ca, matt@eitheror.org.

08/09/08 16:29:50 changed by mtredinnick

  • milestone set to 1.0.

08/11/08 02:02:28 changed by anonymous

  • cc changed from jag@flowtheory.net, korpios@korpios.com, drackett@mac.com, aaron@earthman.ca, matt@eitheror.org to jag@flowtheory.net, korpios@korpios.com, drackett@mac.com, aaron@earthman.ca, matt@eitheror.org, wonlay@gmail.com.

08/21/08 11:05:14 changed by korpios

  • cc changed from jag@flowtheory.net, korpios@korpios.com, drackett@mac.com, aaron@earthman.ca, matt@eitheror.org, wonlay@gmail.com to jag@flowtheory.net, drackett@mac.com, aaron@earthman.ca, matt@eitheror.org, wonlay@gmail.com.

08/27/08 00:23:18 changed by mtredinnick

Fixed in [8608].

08/27/08 00:23:26 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

08/27/08 21:13:23 changed by mtredinnick

  • status changed from closed to reopened.
  • resolution deleted.

This wasn't properly fixed by the above commit. exclude() causes an infinite loop and there are some problems with the efficiency (number of table joins) with the resulting query. I'm working on this now, but wanted to reopen in case anybody rediscovered the problem.

08/28/08 00:00:24 changed by mtredinnick

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [8644]) Improvements to [8608] to fix an infinite loop (for exclude(generic_relation)). Also comes with approximately 67% less stupidity in the table joins for filtering on generic relations.

Fixed #5937, hopefully for good, this time.


Add/Change #5937 (Generic Relations backward lookups generate invalid SQL)




Change Properties
Action