Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#12870 closed (worksforme)

ORM bug with using exclude in conjunction with Q objects

Reported by: subsume 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: UI/UX:

Description (last modified by Alex)

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 5 years ago.
I'm unable to reproduce with these tests.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by subsume

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by Alex

  • Description modified (diff)

comment:3 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 5 years ago by subsume

  • milestone set to 1.2

Changed 5 years ago by Alex

I'm unable to reproduce with these tests.

comment:5 Changed 5 years ago by Alex

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

comment:6 Changed 5 years ago by mucisland

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 Changed 5 years ago by subsume

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 Changed 5 years ago by mucisland

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 Changed 5 years ago by subsume

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

comment:10 Changed 5 years ago by subsume

  • Resolution set to worksforme
  • Status changed from new to closed

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 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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