Opened 14 years ago

Closed 14 years ago

Last modified 12 years ago

#12870 closed (worksforme)

ORM bug with using exclude in conjunction with Q objects

Reported by: Yeago Owned by: nobody
Component: Database layer (models, ORM) Version: 1.1
Severity: Keywords: exclude Q
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Alex Gaynor)

Here's a shell session which illustrates the problem.

>>> from tapped.deck.models import Deck
>>> from django.db.models import Q
>>> Deck.objects.filter(user__username='nammertime')
[<Deck: Walled Out (Legacy) *Private*>, <Deck: Reign of Fire (Legacy) *Private*>, <Deck: Wake of the Cataclysm (Legacy) *Private*>, <Deck: WMDs (Extended) *Private*>, <Deck: Chernabog (Legacy) *Private*>, <Deck: Dark Alliance (Legacy) *Private*>, <Deck: Elf Horde (Legacy) *Private*>, <Deck: Natural Selection *Private*>, <Deck: Sliver Rush (Legacy) *Private*>, <Deck: Rath's Last Conscripts (Legacy) *Private*>, <Deck: Furious Assault (Legacy) *Private*>, <Deck: Urza's Venom (Legacy) *Private*>, <Deck: Sapling EDH (Legacy) *Private*>, <Deck: Ashling EDH (Legacy) *Private*>]
>>> Deck.objects.filter(user__username='nammertime').count()
14
>>> Deck.objects.filter(user__username='nammertime').exclude(category='pro').exclude(category='precon')
[<Deck: Walled Out (Legacy) *Private*>, <Deck: Reign of Fire (Legacy) *Private*>, <Deck: Wake of the Cataclysm (Legacy) *Private*>, <Deck: WMDs (Extended) *Private*>, <Deck: Chernabog (Legacy) *Private*>, <Deck: Dark Alliance (Legacy) *Private*>, <Deck: Elf Horde (Legacy) *Private*>, <Deck: Natural Selection *Private*>, <Deck: Sliver Rush (Legacy) *Private*>, <Deck: Rath's Last Conscripts (Legacy) *Private*>, <Deck: Furious Assault (Legacy) *Private*>, <Deck: Urza's Venom (Legacy) *Private*>, <Deck: Sapling EDH (Legacy) *Private*>, <Deck: Ashling EDH (Legacy) *Private*>]
>>> Deck.objects.filter(user__username='nammertime').exclude(category='pro').exclude(category='precon').count()
14
>>> Deck.objects.filter(user__username='nammertime').exclude(Q(category='pro')|Q(category='precon'))


>>> Deck.objects.filter(user__username='nammertime').exclude(Q(category='pro')|Q(category='precon'))
[]
>>> Deck.objects.filter(user__username='nammertime').exclude(Q(category='pro')|Q(category='precon')).count()
0
>>> Deck.objects.filter(user__username='nammertime').filter(category='pro')
[]
>>> Deck.objects.filter(user__username='nammertime').filter(category='precon')
[]
>>> Deck.objects.filter(user__username='nammertime').exclude(Q(category='pro')|Q(category='precon')).query.as_sql()
('SELECT `deck_deck`.`id`, `deck_deck`.`format_id`, `deck_deck`.`name`, `deck_deck`.`slug`, `deck_deck`.`user_id`, `deck_deck`.`description`, `deck_deck`.`profile_cardinality`, `deck_deck`.`date_added`, `deck_deck`.`date_updated`, `deck_deck`.`date_refreshed`, `deck_deck`.`is_private`, `deck_deck`.`category`, `deck_deck`.`preconstructed_set_id`, `deck_deck`.`cost`, `deck_deck`.`view_count_cache`, `deck_deck`.`comment_count_cache`, `deck_deck`.`data_generated`, `deck_deck`.`image_token`, `deck_deck`.`similar_due`, `deck_deck`.`rating_votes`, `deck_deck`.`rating_score` FROM `deck_deck` INNER JOIN `auth_user` ON (`deck_deck`.`user_id` = `auth_user`.`id`) WHERE (`auth_user`.`username` = %s  AND NOT ((`deck_deck`.`category` = %s  OR `deck_deck`.`category` = %s )))', ('nammertime', 'pro', 'precon'))
>>> Deck.objects.filter(user__username='nammertime').exclude(category='pro').exclude(category='precon').query.as_sql()
('SELECT `deck_deck`.`id`, `deck_deck`.`format_id`, `deck_deck`.`name`, `deck_deck`.`slug`, `deck_deck`.`user_id`, `deck_deck`.`description`, `deck_deck`.`profile_cardinality`, `deck_deck`.`date_added`, `deck_deck`.`date_updated`, `deck_deck`.`date_refreshed`, `deck_deck`.`is_private`, `deck_deck`.`category`, `deck_deck`.`preconstructed_set_id`, `deck_deck`.`cost`, `deck_deck`.`view_count_cache`, `deck_deck`.`comment_count_cache`, `deck_deck`.`data_generated`, `deck_deck`.`image_token`, `deck_deck`.`similar_due`, `deck_deck`.`rating_votes`, `deck_deck`.`rating_score` FROM `deck_deck` INNER JOIN `auth_user` ON (`deck_deck`.`user_id` = `auth_user`.`id`) WHERE (`auth_user`.`username` = %s  AND NOT (`deck_deck`.`category` = %s  AND NOT (`deck_deck`.`category` IS NULL)) AND NOT (`deck_deck`.`category` = %s  AND NOT (`deck_deck`.`category` IS NULL)))', ('nammertime', 'pro', 'precon'))

Attachments (1)

django-t12870.diff (3.4 KB ) - added by Alex Gaynor 14 years ago.
I'm unable to reproduce with these tests.

Download all attachments as: .zip

Change History (12)

comment:2 by Alex Gaynor, 14 years ago

Description: modified (diff)

comment:3 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:4 by Yeago, 14 years ago

milestone: 1.2

by Alex Gaynor, 14 years ago

Attachment: django-t12870.diff added

I'm unable to reproduce with these tests.

comment:5 by Alex Gaynor, 14 years ago

Please provide some more information so we can reproduce this, otherwise I'm going to close as worksforme.

comment:6 by mucisland, 14 years ago

Sorry for blending in, but I got the same problem.

Using kwargs, exclude() works just fine:

>>> units.exclude(cpe_model__chipset__name__icontains="ac5")

...returns the units which do not contain "ac5" in the chipset name of their related cpe_model.

When using Q, exclude() has a problem:

>>> units.exclude(Q(cpe_model__chipset__name__icontains="ac5")

...also excludes units which have an empty chipset field in their related cpe_model.

Sorry for not providing a reproducable session log, but my app is quite complex and I didn't manage to extract just the important models and relations. But as user subsume already showed, the SQL query of exclude(Q()) is missing the AND NOT (deck_deck.category IS NULL) in the WHERE clause.

comment:7 by Yeago, 14 years ago

I verified that this bug still exists in latest svn. updated console:

>>> Deck.objects.filter(user__username='nammertime').exclude(Q(category='pro')|Q(category='precon')).query.get_compiler('default').as_sql()
('SELECT `deck_deck`.`id`, `deck_deck`.`format_id`, `deck_deck`.`name`, `deck_deck`.`slug`, `deck_deck`.`user_id`, `deck_deck`.`description`, `deck_deck`.`profile_cardinality`, `deck_deck`.`date_added`, `deck_deck`.`date_updated`, `deck_deck`.`date_refreshed`, `deck_deck`.`is_private`, `deck_deck`.`category`, `deck_deck`.`preconstructed_set_id`, `deck_deck`.`cost`, `deck_deck`.`view_count_cache`, `deck_deck`.`comment_count_cache`, `deck_deck`.`data_generated`, `deck_deck`.`image_token`, `deck_deck`.`similar_due`, `deck_deck`.`rating_votes`, `deck_deck`.`rating_score` FROM `deck_deck` INNER JOIN `auth_user` ON (`deck_deck`.`user_id` = `auth_user`.`id`) WHERE (`auth_user`.`username` = %s  AND NOT ((`deck_deck`.`category` = %s  OR `deck_deck`.`category` = %s )))', ('nammertime', 'pro', 'precon'))

>>> Deck.objects.filter(user__username='nammertime').exclude(category='pro').exclude(category='precon').query.get_compiler('default').as_sql()
('SELECT `deck_deck`.`id`, `deck_deck`.`format_id`, `deck_deck`.`name`, `deck_deck`.`slug`, `deck_deck`.`user_id`, `deck_deck`.`description`, `deck_deck`.`profile_cardinality`, `deck_deck`.`date_added`, `deck_deck`.`date_updated`, `deck_deck`.`date_refreshed`, `deck_deck`.`is_private`, `deck_deck`.`category`, `deck_deck`.`preconstructed_set_id`, `deck_deck`.`cost`, `deck_deck`.`view_count_cache`, `deck_deck`.`comment_count_cache`, `deck_deck`.`data_generated`, `deck_deck`.`image_token`, `deck_deck`.`similar_due`, `deck_deck`.`rating_votes`, `deck_deck`.`rating_score` FROM `deck_deck` INNER JOIN `auth_user` ON (`deck_deck`.`user_id` = `auth_user`.`id`) WHERE (`auth_user`.`username` = %s  AND NOT (`deck_deck`.`category` = %s  AND NOT (`deck_deck`.`category` IS NULL)) AND NOT (`deck_deck`.`category` = %s  AND NOT (`deck_deck`.`category` IS NULL)))', ('nammertime', 'pro', 'precon'))
>>>

I realize that the tests you can seemed to come up clean, however, there is still some burden to explain the extraneous JOINS. Obviously the test case isn't close enough to the actual problem. Indeed, the fact that such a basic bug can live this long must mean there is a nuance missing. If filter(Q()|Q()) was broken, it would have been found in minutes.

comment:8 by mucisland, 14 years ago

This is a complete app which reproduces the error.

models.py:

from django.db import models

class CarVendor(models.Model):
    name = models.CharField(max_length=128)
    def __unicode__(self):
        return u'%s' % self.name

class CarModel(models.Model):
    name = models.CharField(max_length=128)
    car_vendor = models.ForeignKey(CarVendor, blank=True, null=True)
    def __unicode__(self):
        return u'%s %s' % (self.name, self.car_vendor)

class Car(models.Model):
    owner = models.CharField(max_length=128)
    car_model = models.ForeignKey(CarModel)
    def __unicode__(self):
        return u'%s %s' % (self.owner, self.car_model)

views.py:

from django.db.models import Q
from iop.q.models import *

def init():
    v = CarVendor(name="BMW")
    v.save()
    m = CarModel(name="M3", car_vendor=v)
    m.save()
    c = Car(owner="Bob", car_model=m)
    c.save()
    m = CarModel(name="Trabbi") # Unknown vendor!
    m.save()
    c = Car(owner="Kevin", car_model=m)
    c.save()
    print "All cars:",
    print Car.objects.all()

def plain_exclude():
    print 'No BMWs:',
    print 'Car.objects.exclude(car_model__car_vendor__name="BMW")',
    print Car.objects.exclude(car_model__car_vendor__name="BMW"),
    print '<- Correctly leaves the car with the empty vendor foreign key in its related car_model.'
    print Car.objects.exclude(car_model__car_vendor__name="BMW").query

def q_exclude():
    print 'No BMWs (using Q()):',
    print 'Car.objects.exclude(Q(car_model__car_vendor__name="BMW"))',
    print Car.objects.exclude(Q(car_model__car_vendor__name="BMW")),
    print '<- excludes not only the BMWs, but cars of unknown vendors as well!'
    print Car.objects.exclude(Q(car_model__car_vendor__name="BMW")).query

Interactive session (Django 1.1.1):

>>> from q import views
>>> views.init()
All cars: [<Car: Bob M3 BMW>, <Car: Kevin Trabbi None>]

>>> views.plain_exclude()
No BMWs: Car.objects.exclude(car_model__car_vendor__name="BMW") [<Car: Kevin Trabbi None>] <- Correctly leaves the car with the empty vendor foreign key in its related car_model.
SELECT "q_car"."id", "q_car"."owner", "q_car"."car_model_id" FROM "q_car" INNER JOIN "q_carmodel" ON ("q_car"."car_model_id" = "q_carmodel"."id") LEFT OUTER JOIN "q_carvendor" ON ("q_carmodel"."car_vendor_id" = "q_carvendor"."id") WHERE NOT ("q_carvendor"."name" = BMW  AND NOT ("q_carvendor"."id" IS NULL))

>>> views.q_exclude()
No BMWs (using Q()): Car.objects.exclude(Q(car_model__car_vendor__name="BMW")) [] <- excludes not only the BMWs, but cars of unknown vendors as well!
SELECT "q_car"."id", "q_car"."owner", "q_car"."car_model_id" FROM "q_car" INNER JOIN "q_carmodel" ON ("q_car"."car_model_id" = "q_carmodel"."id") INNER JOIN "q_carvendor" ON ("q_carmodel"."car_vendor_id" = "q_carvendor"."id") WHERE NOT ("q_carvendor"."name" = BMW )

The differences between using .exclude with or without Q() are that .exclude(Q()) uses an INNER JOIN where .exclude() uses a LEFT OUTER JOIN and, more important here, in this INNER JOIN .exclude(Q()) forgets the 'AND NOT ("q_carvendor"."id" IS NULL)' in the WHERE NOT clause.

Sorry for this long entry, but I wasn't able to reproduce the bug in a more compact way.

comment:9 by Yeago, 14 years ago

I misspoke above. Its not a matter of extraneous JOINS, but funky and contradictive NOT syntax.

comment:10 by Yeago, 14 years ago

Resolution: worksforme
Status: newclosed

I'm going to have to retract the above. Apparently I misunderstood the SQL in question, and that those records with category = NULL were not going to be returned.

comment:11 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top