Re: [Django] #14645: Exclude query with multiple conditions for the same multi-value relation not correct
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
Type: Bug | Version: 1.2
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* ui_ux: => 0
* easy: => 0
Comment:
#17315 was a duplicate. It contains a thorough demonstration of the
problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
Type: Bug | Version: 1.2
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anonymous):
I can confirm this Bug on Django 1.3.1 on Linux.
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:5>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* version: 1.2 => 1.3
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:6>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* version: 1.3 => master
Comment:
Still present in master.
You can get SQL which returns what you would expect using this:
{{{
Song.objects.exclude(id__in=Release.objects.filter(format='cd',released=1).values('song'))
}}}
This gives the query you would expect originally:
{{{
SELECT "songs_song"."id", "songs_song"."name" FROM "songs_song"
WHERE NOT (
"songs_song"."id" IN (
SELECT U0."song_id"
FROM "songs_release" U0
WHERE (U0."released" = True AND U0."format" = cd)
)
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:7>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
Type: Bug | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by chriskrusz):
Please note that this behaviour contradicts the documentation.
According to the documentation (from
[https://docs.djangoproject.com/en/1.5/topics/db/queries/#spanning-multi-
valued-relationships]):
"All of this behavior also applies to exclude(): all the conditions in a
single exclude() statement apply to a single instance (if those conditions
are talking about the same multi-valued relation). Conditions in
subsequent filter() or exclude() calls that refer to the same relation may
end up filtering on different linked objects."
But exclude actually filters each condition on a different linked object.
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:8>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
Type: Bug | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: chriskrusz (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:9>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
Type: Bug | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
This one will be hard to fix correctly. Consider case:
{{{
Song.objects.exclude((Q(release_set__format='cd') | Q(pk=1)) &
(Q(release_set__format='lp') | Q(pk=2)))
}}}
As far as I understand the only sensible way to write this is to have a
query:
{{{
SELECT * FROM "song" WHERE NOT EXISTS (
SELECT 1
FROM "release_set"
WHERE ((format = 'cd' OR "song"."pk" = 1)
AND
(format = 'lp' OR "song"."pk" = 2)
AND "song"."pk" = "release_set"."song_id"
}}}
that is, the whole condition must be pushed down into EXISTS query. The
condition should be pushed down from the lowest connector that contains
all the references to the same subquery.
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:10>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
Type: Bug | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: PhiR_42 (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:11>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> | Status: new
Type: Bug | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: exclude manytomany | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: benkraft (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:12>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by seddonym):
I've submitted a pull request for some documentation to draw attention to
this behaviour. Let me know if you'd like it adjusted.
https://github.com/django/django/pull/3898
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:13>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: bugs@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:14>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by seddonym):
Just following up on the pull request, is this something that would be
useful to be added to the documentation?
https://github.com/django/django/pull/3898
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:15>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by seddonym):
I've been looking at correcting the documentation and have just realised
that one of the statements in the original bug report is wrong: "This
works correctly for filter() but for exclude() it is operating the same as
if it was 2 separate exclude() calls."
Actually, the behaviour is a bit different:
`Song.objects.exclude(release_set__format='cd',release_set__released=1)`
would exclude Songs that have ''both'' CD releases ''and'' releases that
have gone out.
`Song.objects.exclude(release_set__format='cd').exclude(release_set__released=1)`,
on the other hand, would exclude Songs that have ''either'' CD releases
''or'' releases that have gone out.
I've made a gist that sums this up in the form of tests:
https://gist.github.com/seddonym/84407891a11389419c14
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:16>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by akaariai):
I think it is time to start working on this. I think we have two major
bugs in Django ORM:
1. This one.
2. Aggregation over multiple different multivalued relations, or
aggregation after filtering over m2m relation produces wrong results.
In addition, we have the problem that filtering over multivalued relation
doesn't use a subquery, instead you have to use distinct.
These all are mixed together. Unfortunately this isn't easy to fix. But
that doesn't mean we shouldn't try.
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:17>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"6770b7ecd208a0746f181e54202fb829460c6490" 6770b7ec]:
{{{
#!CommitTicketReference repository=""
revision="6770b7ecd208a0746f181e54202fb829460c6490"
Refs #14645 -- Documented bug with exclude() and multi-value relations
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:18>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"744d9a10ef46decb632a07f9a4e5fb0df43eb0e6" 744d9a10]:
{{{
#!CommitTicketReference repository=""
revision="744d9a10ef46decb632a07f9a4e5fb0df43eb0e6"
[1.8.x] Refs #14645 -- Documented bug with exclude() and multi-value
relations
Backport of 6770b7ecd208a0746f181e54202fb829460c6490 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:20>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"b46643a47e82fb56d589b9584877f823dabc3fdf" b46643a4]:
{{{
#!CommitTicketReference repository=""
revision="b46643a47e82fb56d589b9584877f823dabc3fdf"
[1.7.x] Refs #14645 -- Documented bug with exclude() and multi-value
relations
Backport of 6770b7ecd208a0746f181e54202fb829460c6490 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:19>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"48f5adf308644c1f03b763b22b39c5058c1840b8" 48f5adf]:
{{{
#!CommitTicketReference repository=""
revision="48f5adf308644c1f03b763b22b39c5058c1840b8"
[1.6.x] Refs #14645 -- Documented bug with exclude() and multi-value
relations
Backport of 6770b7ecd208a0746f181e54202fb829460c6490 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:21>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Can Sarıgöl (added)
* has_patch: 0 => 1
Comment:
I tried to fix this issue with
[https://github.com/django/django/pull/11528 PR] as much as the sample
that's in the description. Does the approach and solution place make
sense?
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:22>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> According to [http://docs.djangoproject.com/en/1.2/topics/db/queries
> /#spanning-multi-valued-relationships]: "all the conditions in a single
> exclude() statement apply to a single instance (if those conditions are
> talking about the same multi-valued relation)." This works correctly for
> filter() but for exclude() it is operating the same as if it was 2
> separate exclude() calls. Here's an example set of models:
>
> {{{
> class Song(models.Model):
> name = models.CharField(max_length=30)
>
> class Release(models.Model):
> song = models.ForeignKey(Song)
> format =
> models.CharField(max_length=3,choices=(('cd',"CD"),('mp3',"MP3")))
> released = models.BooleanField()
> class Meta:
> unique_together = ['song','format']
> }}}
>
> If I want to ask for all of the songs that have a CD release that has
> already gone out this filter works:
>
> {{{Song.objects.filter(release_set__format='cd',release_set__released=1)}}}
>
> But if I want to find all the songs that don't have a CD release that
> has gone out (either it has one that hasn't been released yet, or it
> doesn't have a release record at all), this exclude statement is not
> working:
>
> {{{Song.objects.exclude(release_set__format='cd',release_set__released=1)}}}
>
> It produces this SQL:
>
> {{{
> ('SELECT U0.`id` FROM `songs_song` U0
> WHERE NOT (U0.`id` IN (SELECT U1.`id` FROM `songs_release` U1
> WHERE U1.`format` = %s) AND
> U0.`id` IN (SELECT U1.`id` FROM `songs_release` U1
> WHERE U1.`released` = %s ))',
> ('cd', True))
> }}}
>
> Instead of what I would expect (and give me the right results):
>
> {{{
> ('SELECT U0.`id` FROM `songs_song` U0
> WHERE NOT (U0.`id` IN (SELECT U1.`id` FROM `songs_release` U1
> WHERE U1.`format` = %s AND
> U1.`released` = %s ))',
> ('cd', True))
> }}}
>
> {{{Song.objects.filter(~Q(release_set__format='cd',release_set__released=1))}}}
> produces the same result, but
> {{{Song.objects.exclude(Q(release_set__format='cd',release_set__released=1))}}}
> produces the even more wrong:
>
> {{{
> ('SELECT U0.`id` FROM `songs_song` U0 INNER JOIN `songs_release` U1 ON
> (U0.`id` = U1.`song_id`)
> WHERE NOT ((U1.`format` = %s AND U1.`released` = %s ))',
> ('cd', True))
> }}}
New description:
According to [http://docs.djangoproject.com/en/dev/topics/db/queries
/#spanning-multi-valued-relationships]: "all the conditions in a single
exclude() statement apply to a single instance (if those conditions are
talking about the same multi-valued relation)." This works correctly for
filter() but for exclude() it is operating the same as if it was 2
separate exclude() calls. Here's an example set of models:
{{{
class Song(models.Model):
name = models.CharField(max_length=30)
class Release(models.Model):
song = models.ForeignKey(Song)
format =
models.CharField(max_length=3,choices=(('cd',"CD"),('mp3',"MP3")))
released = models.BooleanField()
class Meta:
unique_together = ['song','format']
}}}
If I want to ask for all of the songs that have a CD release that has
already gone out this filter works:
{{{Song.objects.filter(release_set__format='cd',release_set__released=1)}}}
But if I want to find all the songs that don't have a CD release that has
gone out (either it has one that hasn't been released yet, or it doesn't
have a release record at all), this exclude statement is not working:
{{{Song.objects.exclude(release_set__format='cd',release_set__released=1)}}}
It produces this SQL:
{{{
('SELECT U0.`id` FROM `songs_song` U0
WHERE NOT (U0.`id` IN (SELECT U1.`id` FROM `songs_release` U1
WHERE U1.`format` = %s) AND
U0.`id` IN (SELECT U1.`id` FROM `songs_release` U1
WHERE U1.`released` = %s ))',
('cd', True))
}}}
Instead of what I would expect (and give me the right results):
{{{
('SELECT U0.`id` FROM `songs_song` U0
WHERE NOT (U0.`id` IN (SELECT U1.`id` FROM `songs_release` U1
WHERE U1.`format` = %s AND
U1.`released` = %s ))',
('cd', True))
}}}
{{{Song.objects.filter(~Q(release_set__format='cd',release_set__released=1))}}}
produces the same result, but
{{{Song.objects.exclude(Q(release_set__format='cd',release_set__released=1))}}}
produces the even more wrong:
{{{
('SELECT U0.`id` FROM `songs_song` U0 INNER JOIN `songs_release` U1 ON
(U0.`id` = U1.`song_id`)
WHERE NOT ((U1.`format` = %s AND U1.`released` = %s ))',
('cd', True))
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:23>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:24>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
I just had a look at this ticket. The surprising exclude() behaviour and
the workaround are now documented in
https://docs.djangoproject.com/en/6.0/topics/db/queries/#spanning-multi-
valued-relationships. Could this ticket actually be closed? Or is there a
wish to change the exclude() behaviour?
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:26>
Django
not correct
-------------------------------------+-------------------------------------
Reporter: Ben Buchwald | Owner: nobody
<bb2@…> |
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exclude manytomany | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
I think a solution to make `exclude()` work like `filter()` would still be
welcome here. comment:10 has a hint, so although it sounds involved, I
wouldn't say we should give up just yet.
--
Ticket URL: <https://code.djangoproject.com/ticket/14645#comment:27>