Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#21748 closed Bug (fixed)

Invalid exclude result for related models

Reported by: Sergey Tereschenko Owned by: nobody
Component: Database layer (models, ORM) Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When making exclude query on fields added as reverse relations results are incorrect.
Here is an example:
models.py

class Author(models.Model):
    name = models.CharField(max_length=120)
    book = models.OneToOneField('Book', null=True, blank=True)

    def __unicode__(self):
        return self.name

class Reader(models.Model):
    name = models.CharField(max_length=120)
    book = models.OneToOneField('Book', null=True, blank=True)

    def __unicode__(self):
        return self.name

class Book(models.Model):
    name = models.CharField(max_length=120)

    def __unicode__(self):
        return self.name

class SimpleBook(models.Model):
    name = models.CharField(max_length=120)
    author = models.CharField(max_length=120, null=True, blank=True)
    reader = models.CharField(max_length=120, null=True, blank=True)

    def __unicode__(self):
        return "%s" % self.name  

Test data:

Book                    | Author        | Reader
The Lord of the Rings	| JRR Tolkien	| John
Pride and Prejudice	| Jane Austen	| x
His Dark Materials	| x     	| x

Correct query results (django 1.5):

One book without author and reader, two books, excluding books without author and reader.

In [2]: Book.objects.exclude(author=None, reader=None)
Out[2]: [<Book: The Lord of the Rings>, <Book: Pride and Prejudice>]

In [3]: Book.objects.filter(author=None, reader=None)
Out[3]: [<Book: His Dark Materials>]

Incorrect query results (django 1.6):
One book without without author and reader. If excluded - only one book is selected instead of two.

In [2]: Book.objects.exclude(author=None, reader=None)
Out[2]: [<Book: The Lord of the Rings>]

In [3]: Book.objects.filter(author=None, reader=None)
Out[3]: [<Book: His Dark Materials>]

Expected results: When exluding books without author and reader query must return 2 books. Just like it does with simple fields(which is not reverse relations), django 1.6:

In [2]: SimpleBook.objects.exclude(author=None, reader=None)
Out[2]: [<SimpleBook: The Lord of the Rings>, <SimpleBook: Pride and Prejudice>]

In [3]: SimpleBook.objects.filter(author=None, reader=None)
Out[3]: [<SimpleBook: His Dark Materials>]

Attachments (1)

testproject.tar.gz (7.8 KB ) - added by Sergey Tereschenko 10 years ago.
This is test project to reproduce bug

Download all attachments as: .zip

Change History (8)

by Sergey Tereschenko, 10 years ago

Attachment: testproject.tar.gz added

This is test project to reproduce bug

comment:1 by Sergey Tereschenko, 10 years ago

testproject.tar.gz Contains all data to reproduce bug, including initial_data.json with test data, and

testprojects/testapp/tests.py (simple test to check correct behavior, qs1 = filtered items, qs2 = excluded items, qs1.count() + qs2.count() must be equal to full queryset count).

comment:2 by Anssi Kääriäinen, 10 years ago

Likely a regression due to join promotion changes. I'll take a look.

comment:3 by Anssi Kääriäinen, 10 years ago

The problem is in join promotion logic. The join promotion logic doesn't consider NOT (A AND B) to be equivalent to (NOT A OR NOT B). I have a first-draft patch for master. It works for the attached project's tests and passes all current tests, but I will have to check it also works for more complicated expressions containing multiple NOT clauses.

For 1.6 I will need to write a separate patch, the join promotion code has been largely rewritten for 1.7.

See https://github.com/django/django/pull/2153 for proposed solution.

comment:4 by Baptiste Mispelon, 10 years ago

Has patch: set
Triage Stage: UnreviewedAccepted

(Marking this as accepted per akaariai's comment)

comment:5 by Anssi Kääriäinen, 10 years ago

Triage Stage: AcceptedReady for checkin

I added some more tests, the new logic seems to work for complex cases, too.

comment:6 by Anssi Kääriäinen <akaariai@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 35cecb1ebd0ccda0be7a518d1b7273333d26fbae:

Fixed #21748 -- join promotion for negated AND conditions

Made sure Django treats case .filter(NOT (a AND b)) the same way as
.filter((NOT a OR NOT b)) for join promotion.

comment:7 by Anssi Kääriäinen <akaariai@…>, 10 years ago

In fd3fa851b592700a8b04af46f626454db0db02e4:

[1.6.x] Fixed #21748 -- join promotion for negated AND conditions

Made sure Django treats case .filter(NOT (a AND b)) the same way as
.filter((NOT a OR NOT b)) for join promotion.

Heavily modified backpatch of 35cecb1ebd0ccda0be7a518d1b7273333d26fbae
from master.

Conflicts:

django/db/models/sql/query.py
tests/queries/tests.py

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