Opened 16 years ago

Closed 13 years ago

Last modified 13 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: dev
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 15 years ago.
first stab

Download all attachments as: .zip

Change History (16)

comment:1 by Malcolm Tredinnick, 16 years ago

Owner: changed from nobody to Malcolm Tredinnick
Status: newassigned

comment:2 by Alex Gaynor, 15 years ago

Triage Stage: UnreviewedAccepted

by Johannes Dollinger, 15 years ago

first stab

comment:3 by Johannes Dollinger, 15 years ago

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>]


Version 0, edited 15 years ago by Johannes Dollinger (next)

comment:4 by Johannes Dollinger, 15 years ago

milestone: 1.2

comment:5 by Russell Keith-Magee, 15 years ago

milestone: 1.21.3

Not critical for 1.2

comment:6 by Russell Keith-Magee, 15 years ago

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

comment:7 by mucisland, 15 years ago

#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 by rcarton, 14 years ago

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 by Torsten Bronger, 14 years ago

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 by Torsten Bronger, 14 years ago

Cc: Torsten Bronger added

comment:11 by Dan Watson, 14 years ago

Cc: Dan Watson added

comment:12 by Dan Watson, 14 years ago

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 by Dan Watson, 14 years ago

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 by Aleksandra Sendecka, 13 years ago

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

The problem is solved by the patch for #14876.

comment:15 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

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