Code

Opened 4 years ago

Closed 3 years ago

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

Download all attachments as: .zip

Change History (10)

Changed 4 years ago by bpeschier

unittest for checking isnull on reverse relations

Changed 4 years ago by bpeschier

models for unittest

Changed 4 years ago by bpeschier

Patch + extension of null_queries doctest

comment:1 Changed 4 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 4 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 4 years ago by bpeschier

Updated patch with unittest instead of doctest

comment:3 Changed 3 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 3 years ago by Daishiman

Confirmed here as well.

comment:5 Changed 3 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 3 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.

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.