Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#12252 closed (fixed)

Queryset unions are sometimes incorrect.

Reported by: benreynwar Owned by: benreynwar
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Queryset unions are in this case incorrect. It appears that an 'inner join' is
not being converted into an 'outer join' when it should be. See the example below.

import unittest
from django.db import models

class ObjectA(models.Model):
    name = models.CharField(max_length=50)

class ObjectB(models.Model):
    name = models.CharField(max_length=50)
    objecta = models.ForeignKey(ObjectA)
    number = models.PositiveSmallIntegerField()

class TestStuff(unittest.TestCase):

    def test_stuff(self):
        names = ['one', 'two', 'three', 'four', 'five', 'six', 'seven']
        obs = [ObjectA(name=name) for name in names]
        for o in obs:
            o.save()
        b_infos = [('fish', 5, obs[0]), ('turtle', 9, obs[0]),
                   ('fish', 6, obs[3])]
        ob_bs = [ObjectB(name=name, objecta=objecta, number=number)
                 for name, number, objecta in b_infos]
        for o in ob_bs:
            o.save()
        all_query = ObjectA.objects.all()
        filter1_query = ObjectA.objects.filter(objectb__name='fish')
        filter2_query = ObjectA.objects.filter(objectb__name='fish',
                                               objectb__number=6)
        # The tests with filter1_query pass for me.
	self.assertEqual(set(all_query | filter1_query),
                         set(all_query) | set(filter1_query))
	self.assertEqual(set(filter1_query | all_query),
                         set(all_query) | set(filter1_query))
        # The second test with filter2_query fails for me.
        self.assertEqual(set(all_query | filter2_query),
                         set(all_query) | set(filter2_query))
        self.assertEqual(set(filter2_query | all_query),
                         set(all_query) | set(filter2_query))

Attachments (5)

12252a.diff (5.8 KB) - added by benreynwar 5 years ago.
12252b.diff (5.8 KB) - added by benreynwar 5 years ago.
second attempt to get the patch to display correctly
12252.diff (5.7 KB) - added by kmtracey 5 years ago.
Updated patch to apply cleanly
12252.1.diff (5.3 KB) - added by ramiro 5 years ago.
Karen, no enough ORM fu here, but updated the patch removing unneeded code in tests (init(), setUp())
12252_1.2.diff (5.7 KB) - added by PhiR 4 years ago.
patch for 1.2.5 in case anyone needs it (fix is commited in 1.2.X branch)

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by emulbreh

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

comment:2 Changed 5 years ago by benreynwar

  • Owner changed from nobody to benreynwar

Changed 5 years ago by benreynwar

comment:3 Changed 5 years ago by benreynwar

  • Has patch set

I've uploaded a patch (although it's not displaying properly) which appears to correct this problem, and a regression test to go with it.

Changed 5 years ago by benreynwar

second attempt to get the patch to display correctly

Changed 5 years ago by kmtracey

Updated patch to apply cleanly

comment:4 Changed 5 years ago by mlavin

  • milestone set to 1.3

I'm seeing a similar issue in my project where queryset OR is not commutative. One way produces a LEFT OUTER JOIN and the other produces an INNER JOIN. This patch resolves this issue for me.

comment:5 Changed 5 years ago by kmtracey

  • Triage Stage changed from Accepted to Ready for checkin

Near as I can tell, this is good to go, though I'd be happier if someone with more ORM clues than me would give it look...

Changed 5 years ago by ramiro

Karen, no enough ORM fu here, but updated the patch removing unneeded code in tests (init(), setUp())

comment:6 Changed 4 years ago by russellm

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

In [15726]:

Fixed #12252 -- Ensure that queryset unions are commutative. Thanks to benreynwar for the report, and draft patch, and to Karen and Ramiro for the review eyeballs and patch updates.

comment:7 Changed 4 years ago by russellm

In [15727]:

[1.2.X] Fixed #12252 -- Ensure that queryset unions are commutative. Thanks to benreynwar for the report, and draft patch, and to Karen and Ramiro for the review eyeballs and patch updates.

Backport of r15726 from trunk.

Changed 4 years ago by PhiR

patch for 1.2.5 in case anyone needs it (fix is commited in 1.2.X branch)

comment:8 Changed 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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