Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#7076 closed (fixed)

qs.exclude(something=123) should not exclude objects with something=None

Reported by: Thomas Steinacher <tom@…> Owned by:
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qsrf-cleanup
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description

from django.db import models
class A(models.Model):
    str = models.CharField(null=True, blank=True, max_length=20)
    def __unicode__(self):
        return self.str or 'None'
In [1]: from test import models

In [2]: a = models.A.objects.create(str='a')

In [3]: b = models.A.objects.create(str=None)

In [4]: models.A.objects.exclude(str='a')
Out[4]: []

In [5]: models.A.objects.filter(str__isnull=True).exclude(str='a')
Out[5]: []

In [6]: models.A.objects.all()                                    
Out[6]: [<A: a>, <A: None>]

Expected output:

Out[4]: [<A: None>]  
Out[5]: [<A: None>]  

Tested with trunk & queryset-refactor.

Attachments (1)

7076.1.diff (1.6 KB) - added by emulbreh 6 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 6 years ago by gav

  • Keywords qsrf-cleanup added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Changed 6 years ago by emulbreh

comment:2 Changed 6 years ago by emulbreh

  • Has patch set
  • Needs documentation set
  • Needs tests set
  • Owner changed from nobody to anonymous
  • Patch needs improvement set
  • Status changed from new to assigned

I can reproduce this (r7573 + postgres).

It could be solved easyly by making __exact NULL-safe:

But I couldn't find something comparable for sqlite and oracle (except for (a=b OR (a IS NULL AND b IS NULL))) - and I don't know if anything depends on NULL == 1=NULL == NOT (1=NULL) and would break with FALSE == 1<=>NULL != NOT(1<=>NULL) == TRUE.

While trying to come up with a patch I found some dead code that should handle the issue for related tables (see patch, missing [JOIN_TYPE] in if final > 1 ...) and just threw in a similar weakening for the unjoined case.

This will be a backwards incompatible change.

comment:3 Changed 6 years ago by anonymous

  • Owner anonymous deleted
  • Status changed from assigned to new
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 6 years ago by jacob

  • milestone set to 1.0

comment:5 Changed 6 years ago by mtredinnick

Thanks for the patch, but ... one way to tell that a patch isn't quite correct is when the existing test suite doesn't pass when it's applied. :-)

This patch breaks the exclude(field__in=[]) case. I'll fix it before committing and add some tests for the issues in this ticket, but please check the test suite and include tests for any new functionality. It makes things easier.

comment:6 Changed 6 years ago by mtredinnick

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

Fixed in [7760].

comment:7 Changed 6 years ago by mtredinnick

(In [7812]) Modified [7760] to not include a "col is not NULL" fragment for non-nullable fields.

This avoids any use of "pk is not NULL" fragment, which behave inconsistently
in MySQL. Thanks to Russell Keith-Magee for diagnosing the problem and
suggesting the easy fix.

Refs #7076.

comment:8 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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.