Using exclude on a queryset with an annotate field gives attribute error.

[joseph.helfer@…]

Generating a MySQL query with an annotate followed by an exclude which follows two foreignkey relations results in:

AttributeError: 'NoneType' object has no attribute 'startswith'

To reproduce this, you can do the following:
Create a new project, set the backend to mysql (and make a database), and create an app foo with this models.py:

from django.db import models

class Model1(models.Model):
    pass

class Model2(models.Model):
    model1 = models.ForeignKey(Model1)

class Model3(models.Model):
    model2 = models.ForeignKey(Model2)

Put foo in INSTALLED_APPS, run syncdb, and then run this in the django shell

from foo.models import *
from django.db.models import Count

print Model3.objects.annotate(bar=Count('id')).exclude(model2__model1__id=4).query

I'm running django 1.3.1 and python 2.6.7 on Mac OSX 10.7.1

This seems to be very similar to
https://code.djangoproject.com/ticket/12687
but, although that ticket is closed, the bug persists.

[glen.nelson.1@…]

I have hit a very similar case. I cannot upload and attach files due to IT restrcitions where I am. However I can give you the following source:

managers.py

from django.db import models
from django.db.models import Count

class TestRunAggregator(models.Manager):
    def get_query_set(self):
        return super(TestRunAggregator, self).get_query_set().annotate(tests_run=Count("testcaserun"))

models.py

from django.db import models
from managers import TestRunAggregator


class Location(models.Model):
    """ Represents the various locations a system
        can be found.
    """
    name = models.CharField(max_length=200, unique=True)
    description = models.TextField(null=True, blank=True)

    def __unicode__(self):
        return self.name

class System(models.Model):
    name = models.CharField(max_length=100, unique=True)
    description = models.TextField(null=True, blank=True)
    ipaddress = models.IPAddressField('IP address')
    owners = models.ManyToManyField(SystemOwner)
    location = models.ForeignKey(Location)
    objects = TestRunAggregator()

    def __unicode__(self):
        return self.name

    def all_owners(self):
        return ', '.join([owner.email for owner in self.owners.all()])

    @models.permalink
    def get_absolute_url(self):
        return ('system_view', [str(self.id)])

    def latest_version(self):
        if self.testcase_runs():
            return self.testcaserun_set.latest('end_date').build
        else:
            return 'unknown'

    def testcase_runs(self):
        return self.testcaserun_set.count()

class TestCaseRun(models.Model):
    start_date = models.DateTimeField('date started')
    end_date = models.DateTimeField('date finished')
    status = models.ForeignKey(TestCaseStatus)
    exception = models.TextField('exception message', null=True, blank=True)
    traceback = models.TextField(null=True, blank=True)
    cycles = models.IntegerField('number of cycles')
    build = models.TextField(null=True, blank=True)

    class Meta:
        unique_together = ('testcase', 'system', 'start_date', 'end_date')
        ordering = ['-start_date']

    def __unicode__(self):
        return self.testcase.name

    @classmethod
    def latest(cls, count=10):
        return cls.objects.all()[:count]

    @models.permalink
    def get_absolute_url(self):
        return ('testrun_view', [str(self.id)])

Example in the shell:

>>> import models as m

>>> m.System.objects.all()
[<System System1>, <System System2>, <System System3>]
>>> m.System.objects.all().exclude(location__name="City1")
[]
>>> # Not Shown here, but if you include connection, the exclude above never ran any SQL

>>> m.System.objects.all().filter(location__name="City1")
[<System System1>, <System System2>]
>>> m.SystemObjects.all().exclude(location__id=1)
[<System System3>]

[Aleksandra Sendecka]

Patch without tests (done mostly by lrekucki, I was just helping)

[Łukasz Rekucki]

Previous patch with regression test added.

[glen.nelson.1@…]

Tested this patch on my django project - it seems to have worked.

[community@…]

This is still an issue with 1.4.5 (not tested 1.5.x).

The patch provided fixes this exact problem, but is there any side effect I have to expect? Anybody verified this using tests?

And a last one: Will this patch be committed (or even backported to 1.4.x)?

Regards,
Marc

[Łukasz Rekucki]

Replying to community@…:

The patch provided fixes this exact problem, but is there any side effect I have to expect? Anybody verified this using tests?

I'm pretty sure it has no side effects. At the time of writing it, the whole Django test suite passed.

And a last one: Will this patch be committed (or even backported to 1.4.x)?

I hope so. What this issue needs is someone to test it with master (patch, run the test suite) and if everything is OK, mark it as "Ready for check-in". Sadly, only regressions and security fixes go to maintenance branches, so this has a chance to be included in 1.6.

[Tim Graham]

Patch no longer applies cleanly.

[Łukasz Rekucki]

Testcase rebased to master.

[Łukasz Rekucki]

The issue seems to be fixed by refactoring in changeset:01b9c3d5193fe61b82ae8b26242a13fdec22f211. On master the condition in promote_joins() still only checks if join_type is not LOUTER instead of explicitly checking for INNER (which could result in promoting non-join to join). I would recommend adding an explicit assertion there or at least a comment why the author assumes that join_type is not None.

[Tim Graham]

Test looks good. Marking as RFC for @akaariai to take a look.

[Anssi Kääriäinen]

I am going to investigate the join_type issue. Maybe there are other cases where None could be promoted accidentally, like combining two querysets with |.

[Anssi Kääriäinen <akaariai@…>]

In c7739e30b20f55c2b055b12a628bfb5c2228ba4e:

Fixed #17424 -- annotate() + exclude() bug

The bug was already fixed by 01b9c3d5193fe61b82ae8b26242a13fdec22f211,
so only tests added.

At the same time promote_joins()'s uncoditional flag is gone, it isn't
needed for anything any more.