Opened 8 years ago

Closed 5 years ago

Last modified 5 years ago

#11052 closed Bug (duplicate)

Q-Object disjunction join promotion .. bug

Reported by: Johannes Dollinger Owned by: Malcolm Tredinnick
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: m2m, join, null, q
Cc: Torsten Bronger, Dan Watson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

(someone who understands why this fails should fix the summary)

Here's a minimal test:

from unittest import TestCase
from django.db import models

class A(models.Model):
    name = models.CharField(max_length=30)
    bs = models.ManyToManyField('B')
        
class B(models.Model):
    name = models.CharField(max_length=30)
    c = models.ForeignKey('C')
    
class C(models.Model):
    name = models.CharField(max_length=30)

class ABCTest(TestCase):
    def test(self):
        c = C.objects.create(name='the c')
        b = B.objects.create(c=c, name='the b')
        a_with_b = A.objects.create(name='a with b')
        a_with_b.bs.add(b)
        a_without_b = A.objects.create(name='a without b')
        
        q0 = models.Q(name__contains='without')
        q1 = models.Q(bs__name__contains='foo')
        q2 = models.Q(bs__c__name__contains='foo')
        
        self.assertEqual([a_without_b], list(A.objects.filter(q0)))
        self.assertEqual([a_without_b], list(A.objects.filter(q0 | q1)))
        self.assertEqual([a_without_b], list(A.objects.filter(q0 | q2)))

        # this fails:
        self.assertEqual([a_without_b], list(A.objects.filter(q0 | q1 | q2)))
        

The offensive query (edited for readabiliy):

SELECT a.id, a.name
FROM a 
  LEFT OUTER JOIN a_bs ON (a.id = a_bs.a_id) 
  LEFT OUTER JOIN b ON (a_bs.b_id = b.id)
  INNER JOIN c ON (b.c_id = c.id) 
WHERE (a.name LIKE %without% OR b.name LIKE %foo% OR c.name LIKE %foo%)

Obviously this INNER JOIN should be a LEFT OUTER JOIN.

Attachments (1)

11052.promote_alias_lhs.diff (763 bytes) - added by Johannes Dollinger 7 years ago.
first stab

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by Malcolm Tredinnick

Owner: changed from nobody to Malcolm Tredinnick
Status: newassigned

comment:2 Changed 7 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

Changed 7 years ago by Johannes Dollinger

first stab

comment:3 Changed 7 years ago by Johannes Dollinger

Has patch: set
Needs tests: set
Patch needs improvement: set

The patch changes Query.promote_alias() to actually look at the LHS of the join as its docstring suggests.
I'm not sure if this is the right approach or even the right place to handle this bug.

Observation:

>>> A.objects.filter(q0 | q2 | q1)
[<A: A object>]
>>> A.objects.filter(q0 | q1 | q2)
[]
>>> # probably a different bug:
>>> A.objects.exclude(Q(bs__name__contains="foo"))
[<A: A object>]
>>> A.objects.exclude(bs__name__contains="foo")
[<A: A object>, <A: A object>]


Last edited 6 years ago by Ramiro Morales (previous) (diff)

comment:4 Changed 7 years ago by Johannes Dollinger

milestone: 1.2

comment:5 Changed 7 years ago by Russell Keith-Magee

milestone: 1.21.3

Not critical for 1.2

comment:6 Changed 7 years ago by Russell Keith-Magee

#13198 is another report, with two test cases for the problem (or, at least a very similar problem).

comment:7 Changed 7 years ago by mucisland

#13198 describes the behaviour which emulbreh named "probably a different bug" in his observation:

>>> # probably a different bug:
>>> A.objects.exclude(Q(bs__name__contains="foo"))
[<A: A object>]
>>> A.objects.exclude(bs__name__contains="foo")
[<A: A object>, <A: A object>]

Well, #13198 has been closed and I learned that this is terminal. I just wanted to point out a last time that exclude() does not work as one would expect when using it with Q.

comment:8 Changed 6 years ago by rcarton

I am unsure this is related but:

In [81]: PullPending.objects.exclude(~Q(id__in=[])).count()
Out[81]: 53

In [82]: PullPending.objects.exclude(~Q(id__in=[]) & Q(id__gt=-1)).count()
Out[82]: 0

Correct me if I'm wrong:

  1. should return 0 (exclude any object whose id is not in an empty dict, hence exclude all the objects)
  2. should return 0 too, the only difference is like q1 & q2 where q2 is always true.

As I understand it:

  • exclude('True') -> does not exclude
  • exclude('True' and 'True') -> does exclude.

comment:9 Changed 6 years ago by Torsten Bronger

I confirm the observation of comment:7 by mucisland.

Apparently, if a foreign key can't be followed because it's NULL, the Q object returns True if used in an "exclude".

comment:10 Changed 6 years ago by Torsten Bronger

Cc: Torsten Bronger added

comment:11 Changed 6 years ago by Dan Watson

Cc: Dan Watson added

comment:12 Changed 6 years ago by Dan Watson

Severity: Normal
Type: Bug

Another description of the problem and code to reproduce it is here:
https://groups.google.com/d/msg/django-developers/4wMNF61oQNM/hqspS-Jp5rwJ

comment:13 Changed 6 years ago by Dan Watson

I think the problem I described in the post above is actually slightly different than this issue, so I've opened a new ticket: #15823

comment:14 Changed 5 years ago by Aleksandra Sendecka

Easy pickings: unset
Resolution: duplicate
Status: assignedclosed
UI/UX: unset

The problem is solved by the patch for #14876.

comment:15 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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