Code

Opened 3 months ago

Closed 2 months ago

Last modified 2 months ago

#21748 closed Bug (fixed)

Invalid exclude result for related models

Reported by: partizan 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 partizan 3 months ago.
This is test project to reproduce bug

Download all attachments as: .zip

Change History (8)

Changed 3 months ago by partizan

This is test project to reproduce bug

comment:1 Changed 3 months ago by partizan

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

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 Changed 3 months ago by akaariai

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

comment:3 Changed 3 months ago by akaariai

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 Changed 3 months ago by bmispelon

  • Has patch set
  • Triage Stage changed from Unreviewed to Accepted

(Marking this as accepted per akaariai's comment)

comment:5 Changed 3 months ago by akaariai

  • Triage Stage changed from Accepted to Ready for checkin

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

comment:6 Changed 2 months ago by Anssi Kääriäinen <akaariai@…>

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

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 Changed 2 months ago by Anssi Kääriäinen <akaariai@…>

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.