Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#11052 closed Bug (duplicate)

Q-Object disjunction join promotion .. bug

Reported by: emulbreh Owned by: mtredinnick
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: m2m, join, null, q
Cc: bronger, dcwatson 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 emulbreh 6 years ago.
first stab

Download all attachments as: .zip

Change History (16)

comment:1 Changed 6 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to mtredinnick
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 6 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

Changed 6 years ago by emulbreh

first stab

comment:3 Changed 6 years ago by emulbreh

  • 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 4 years ago by ramiro (previous) (diff)

comment:4 Changed 5 years ago by emulbreh

  • milestone set to 1.2

comment:5 Changed 5 years ago by russellm

  • milestone changed from 1.2 to 1.3

Not critical for 1.2

comment:6 Changed 5 years ago by russellm

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

comment:7 Changed 5 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 5 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 5 years ago by 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 5 years ago by bronger

  • Cc bronger added

comment:11 Changed 5 years ago by dcwatson

  • Cc dcwatson added

comment:12 Changed 4 years ago by dcwatson

  • Severity set to Normal
  • Type set to 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 4 years ago by dcwatson

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

  • Easy pickings unset
  • Resolution set to duplicate
  • Status changed from assigned to closed
  • UI/UX unset

The problem is solved by the patch for #14876.

comment:15 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