Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#13815 closed (fixed)

Excluding isnull=False testing failing on reverse optional relations

Reported by: bpeschier Owned by: nobody
Component: Database layer (models, ORM) Version: master
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: UI/UX:

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 bpeschier 5 years ago.
unittest for checking isnull on reverse relations
models.py (180 bytes) - added by bpeschier 5 years ago.
models for unittest
13815.patch (1.4 KB) - added by bpeschier 5 years ago.
Patch + extension of null_queries doctest
13815_2.patch (1.6 KB) - added by bpeschier 5 years ago.
Updated patch with unittest instead of doctest

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by bpeschier

unittest for checking isnull on reverse relations

Changed 5 years ago by bpeschier

models for unittest

Changed 5 years ago by bpeschier

Patch + extension of null_queries doctest

comment:1 Changed 5 years ago by anonymous

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 5 years ago by anonymous

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.

Changed 5 years ago by bpeschier

Updated patch with unittest instead of doctest

comment:3 Changed 4 years ago by marcosmoyano

  • Triage Stage changed from Unreviewed to Ready for checkin

I can confirm the patch worked against revision 14551

comment:4 Changed 4 years ago by Daishiman

Confirmed here as well.

comment:5 Changed 4 years ago by russellm

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

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 Changed 4 years ago by russellm

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