Opened 14 years ago

Closed 13 years ago

Last modified 13 years ago

#13815 closed (fixed)

Excluding isnull=False testing failing on reverse optional relations

Reported by: Bas Peschier Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Keywords: isnull, reverse relation
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

This one is related to #13768.

When executing <queryset>.exclude(<reverse relation>__isnull=False) SQL is generated which can contain NULL-values in a subquery when the relation is optional (null=True), always resulting in an empty resultset, irregardless of other values in the subquery.

This is the SQL generated:

class Outer(models.Model):
    name    = models.CharField(max_length=1)

class Inner(models.Model):
    outer   = models.ForeignKey(Outer, null=True)

Outer.objects.exclude(inner__isnull=False):

SELECT "test_outer"."id", "test_outer"."name" FROM "test_outer" WHERE NOT (("test_outer"."id" IN (SELECT U1."outer_id" FROM "test_inner" U1 WHERE U1."id" IS NOT NULL) AND NOT ("test_outer"."id" IS NULL))) LIMIT 21

The subquery could include a test for the relation-field (U1."outer_id" IS NOT NULL) to exclude unusable NULL-values, which will make this work. SQL-generation in Django is one of the few areas I am not comfortable with, so I have no relation to *where* to fix this.

I have attached a unittest to check all isnull-tests, just to be sure.

Attachments (4)

tests.py (2.7 KB ) - added by Bas Peschier 14 years ago.
unittest for checking isnull on reverse relations
models.py (180 bytes ) - added by Bas Peschier 14 years ago.
models for unittest
13815.patch (1.4 KB ) - added by Bas Peschier 14 years ago.
Patch + extension of null_queries doctest
13815_2.patch (1.6 KB ) - added by Bas Peschier 13 years ago.
Updated patch with unittest instead of doctest

Download all attachments as: .zip

Change History (10)

by Bas Peschier, 14 years ago

Attachment: tests.py added

unittest for checking isnull on reverse relations

by Bas Peschier, 14 years ago

Attachment: models.py added

models for unittest

by Bas Peschier, 14 years ago

Attachment: 13815.patch added

Patch + extension of null_queries doctest

comment:1 by anonymous, 14 years ago

Has patch: set

Found to time to dive into Django's SQL generation and created patch for this case.

comment:2 by anonymous, 13 years ago

Tested it with django 1.1.2 and this patch fixes alot of issues for me! (I have a stack of workarounds for this bug).
Thank you!

Hope this gets applied for the next release.

by Bas Peschier, 13 years ago

Attachment: 13815_2.patch added

Updated patch with unittest instead of doctest

comment:3 by Marcos Moyano, 13 years ago

Triage Stage: UnreviewedReady for checkin

I can confirm the patch worked against revision 14551

comment:4 by Daishiman, 13 years ago

Confirmed here as well.

comment:5 by Russell Keith-Magee, 13 years ago

Resolution: fixed
Status: newclosed

In [15458]:

Fixed #13815 -- Ensure that reverse exclude lookups on nullable foreign keys exclude null values. Thanks to bpeschier for the report and patch.

comment:6 by Russell Keith-Magee, 13 years ago

In [15460]:

[1.2.X] Fixed #13815 -- Ensure that reverse exclude lookups on nullable foreign keys exclude null values. Thanks to bpeschier for the report and patch.

Backport of r15458 from trunk.

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