Opened 8 years ago

Closed 7 years ago

#6154 closed (fixed)

Q objects don't work properly when using spanned relationships

Reported by: Thomas Steinacher <tom@…> Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qs-rf-fixed
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

>>> from wcenter.models import Website, Domain
>>> from django.db.models import Q
>>> websites = Website.objects.filter(pk__in=[182,183])
>>> d = Domain.objects.get(name='mydomain')
>>> websites
[<Website: 182>, <Website: 183]

Until now, that's fine. Now let's try the following:

>>> q = websites.filter(Q(domain=d) | Q(websitealias__domain=d))
>>> q
[<Website: 182>, <Website: 182>]

This is the bug. Why is website 183 gone? Here's the SQL (I am not an SQL expert, but maybe Django should use a LEFT JOIN instead of an INNER JOIN):

>>> def show_sql(q):
...     cols, sql, args = q._get_sql_clause()
...     return "SELECT %s %s" % (', '.join(cols), sql % tuple(args))
...
>>> show_sql(q)
'SELECT `wcenter_website`.`id`, `wcenter_website`.`domain_id`, [...]  FROM `wcenter_website` INNER JOIN `wcenter_websitealias` AS `wcenter_website__websitealias` ON `wcenter_website`.`id` = `wcenter_website__websitealias`.`website_id` WHERE (`wcenter_website`.`id` IN (182,183) AND (`wcenter_website`.`domain_id` = 2 OR `wcenter_website__websitealias`.`domain_id` = 2))'
>>> show_sql(websites)
'SELECT `wcenter_website`.`id`, `wcenter_website`.`domain_id`, [...]  FROM `wcenter_website` WHERE (`wcenter_website`.`id` IN (182,183))'

I think the second website is missing because it doesn't have any website aliases:

>>> websites[0].websitealias_set.all()
[<WebsiteAlias>, <WebsiteAlias>]
>>> websites[1].websitealias_set.all()
[]

It looks like a LEFT JOIN would solve the problem:

mysql> SELECT `wcenter_website`.`id`, `wcenter_website`.`domain_id` FROM `wcenter_website` LEFT JOIN `wcenter_websitealias` AS `wcenter_website__websitealias` ON `wcenter_website`.`id` = `wcenter_website__websitealias`.`website_id` WHERE (`wcenter_website`.`id` IN (182,183) AND (`wcenter_website`.`domain_id` = 2));
+-----+-----------+
| id  | domain_id |
+-----+-----------+
| 182 |         2 | 
| 182 |         2 | 
| 183 |         2 | 
+-----+-----------+
3 rows in set (0.00 sec)

mysql> SELECT `wcenter_website`.`id`, `wcenter_website`.`domain_id` FROM `wcenter_website` INNER JOIN `wcenter_websitealias` AS `wcenter_website__websitealias` ON `wcenter_website`.`id` = `wcenter_website__websitealias`.`website_id` WHERE (`wcenter_website`.`id` IN (182,183));
+-----+-----------+
| id  | domain_id |
+-----+-----------+
| 182 |         2 | 
| 182 |         2 | 
+-----+-----------+
2 rows in set (0.00 sec)

Change History (8)

comment:1 Changed 8 years ago by Thomas Steinacher <tom@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Note: Of course, lines 5 and 6 should read as follows:

>>> websites.filter(domain=d)
[<Website: 182>, <Website: 183>]

comment:2 Changed 8 years ago by mtredinnick

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

This is #2080, which has now been fixed on the queryset-refactor branch and will be merged into trunk when that work is finished.

comment:3 Changed 8 years ago by Thomas Steinacher <tom@…>

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Sorry for reopening it, but it still doesn't work properly in the queryset-refactor branch:

>>> Website.objects.filter(pk__in=[182,183]).filter(Q(domain=d) | Q(websitealias__domain=d)).distinct().count()
41L
>>> Website.objects.filter(Q(domain=d) | Q(websitealias__domain=d)).filter(pk__in=[182,183]).distinct().count()                        
2L

comment:4 Changed 8 years ago by mtredinnick

  • Keywords qs-rf added
  • Triage Stage changed from Unreviewed to Accepted

In that case, please construct a small, complete example that demonstrates the problem. At the moment, all I have to go on is the "shape" of the query, which is why it looks like #2080, which did have problems.

Since it's apparently not that issue, what is a small model and data set that can be used to repeat the bug for testing purposes (I have no idea what your Website contains and which of the relations in your setup are significant here).

comment:5 Changed 8 years ago by Thomas Steinacher <tom@…>

from django.db import models

class A(models.Model):
    def __unicode__(self):
        return '%d' % self.id

class B(models.Model):
    a = models.ForeignKey(A)
    def __unicode__(self):
        return '%d' % self.id

class C(models.Model):
    a = models.ForeignKey(A)
    b = models.ForeignKey(B)
    def __unicode__(self):
        return '%d' % self.id
>>> from django.db.models import Q
>>> from qs.models import A,B,C
>>> a1 = A.objects.create()
>>> a2 = A.objects.create()
>>> b1 = B.objects.create(a=a1)
>>> b2 = B.objects.create(a=a1)
>>> b3 = B.objects.create(a=a2)
>>> c1 = C.objects.create(a=a1, b=b1)
>>> a1, a2, b1, b2, b3, c1
(<A: 1>, <A: 2>, <B: 1>, <B: 2>, <B: 3>, <C: 1>)
>>> B.objects.filter(pk__in=[b1.id,b2.id]).filter(Q(a=a1.id) | Q(c__a=a1.id))           
[<B: 1>, <B: 2>]

Until now, everything is okay. Now let's create another C object:

>>> c2 = C.objects.create(b=b3, a=a1)
>>> B.objects.filter(pk__in=[b1.id,b2.id]).filter(Q(a=a1.id) | Q(c__a=a1.id))
[<B: 1>, <B: 2>, <B: 3>]
>>> B.objects.filter(Q(a=a1.id) | Q(c__a=a1.id)).filter(pk__in=[b1.id,b2.id])
[<B: 1>, <B: 2>]

b3 shouldn't appear here, because it is filtered. It works correctly when filtering the PK at the end.

comment:6 Changed 8 years ago by mtredinnick

(In [6957]) queryset-refactor: Fixed a problem when adding certain additional filters to a queryset that has precisely one filter attached already.

Refs #6154.

comment:7 Changed 8 years ago by mtredinnick

  • Keywords qs-rf-fixed added; qs-rf removed

comment:8 Changed 7 years ago by mtredinnick

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

(In [7477]) Merged the queryset-refactor branch into trunk.

This is a big internal change, but mostly backwards compatible with existing
code. Also adds a couple of new features.

Fixed #245, #1050, #1656, #1801, #2076, #2091, #2150, #2253, #2306, #2400, #2430, #2482, #2496, #2676, #2737, #2874, #2902, #2939, #3037, #3141, #3288, #3440, #3592, #3739, #4088, #4260, #4289, #4306, #4358, #4464, #4510, #4858, #5012, #5020, #5261, #5295, #5321, #5324, #5325, #5555, #5707, #5796, #5817, #5987, #6018, #6074, #6088, #6154, #6177, #6180, #6203, #6658

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