Code

Opened 3 years ago

Closed 6 months ago

#14511 closed Bug (fixed)

exclude() generates wrong query for ManyToManyField with a 'through' relationship

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

Description

Consider the following sample models:

from django.db import models

class Person(models.Model):
    name = models.CharField(max_length=128)

class Company(models.Model):
    name = models.CharField(max_length=128)
    employees = models.ManyToManyField(Person, related_name='employers', through='Employment')

class Employment(models.Model):
    employer = models.ForeignKey(Company)
    employee = models.ForeignKey(Person)
    title = models.CharField(max_length=128)

The generated query for finding all the companies that 'alex' has worked for as an engineer or developer looks good:

>>> alex = Person.objects.get(name='alex')
>>> alex.employers.filter(employment__title__in=('Engineer', 'Developer')).distinct()
SELECT DISTINCT `lib_company`.`id`,
                `lib_company`.`name`
FROM `lib_company`
INNER JOIN `lib_employment` ON (`lib_company`.`id` = `lib_employment`.`employer_id`)
WHERE (`lib_employment`.`employee_id` = 1
       AND `lib_employment`.`title` IN (Engineer, Developer))

However the inverse -- finding all the companies that alex has worked for but not as an engineer or developer -- is wrong:

>>> alex.employers.exclude(employment__title__in=('Engineer', 'Developer')).distinct()
SELECT DISTINCT `lib_company`.`id`,
                `lib_company`.`name`
FROM `lib_company`
INNER JOIN `lib_employment` ON (`lib_company`.`id` = `lib_employment`.`employer_id`)
WHERE (`lib_employment`.`employee_id` = 1
       AND NOT (`lib_company`.`id` IN
                  (SELECT U1.`employer_id`
                   FROM `lib_employment` U1
                   WHERE U1.`title` IN (Engineer, Developer))))

So basically this returns all the companies that alex has worked for that don't have any engineer or developer employee, not just alex. Here's a test that reproduces the problem:

import unittest

class ExcludeJoinTest(unittest.TestCase):
    def test(self):
        alex = Person.objects.get_or_create(name='Alex')[0]
        jane = Person.objects.get_or_create(name='Jane')[0]

        oracle = Company.objects.get_or_create(name='Oracle')[0]
        google = Company.objects.get_or_create(name='Google')[0]
        microsoft = Company.objects.get_or_create(name='Microsoft')[0]
        intel = Company.objects.get_or_create(name='Intel')[0]

        def employ(employer, employee, title):
            Employment.objects.get_or_create(employee=employee, employer=employer, title=title)

        employ(oracle, alex, 'Engineer')
        employ(oracle, alex, 'Developer')
        employ(google, alex, 'Engineer')
        employ(google, alex, 'Manager')
        employ(microsoft, alex, 'Manager')
        employ(intel, alex, 'Manager')

        employ(microsoft, jane, 'Developer')
        employ(intel, jane, 'Manager')

        alex_tech_employers = list(alex.employers.filter(employment__title__in=('Engineer', 'Developer')).distinct())
        self.assertEqual(alex_tech_employers, [oracle, google])

        alex_nontech_employers = list(alex.employers.exclude(employment__title__in=('Engineer', 'Developer')).distinct())
        self.assertEqual(alex_nontech_employers, [microsoft, intel])

======================================================================
FAIL: test (lib.models.ExcludeJoinTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/george/lib/models.py", line 53, in test
    self.assertEqual(alex_nontech_employers, [microsoft, intel])
AssertionError: [<Company: Intel>] != [<Company: Microsoft>, <Company: Intel>]

Attachments (0)

Change History (5)

comment:1 Changed 3 years ago by russellm

  • milestone 1.3 deleted
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:3 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:4 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:5 Changed 6 months ago by Anssi Kääriäinen <akaariai@…>

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

In bec0b2a8c637ba2dc3745ad0c1e68ccc6c2f78bd:

Fixed #14511 -- bug in .exclude() query

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.