[Django] #26983: models.Manager query "filter(field__isnull=False)" not working in 1.10
Django
----------------------------------------------+----------------------
Reporter: weidwonder | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.10
Severity: Normal | Keywords: queryset
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+----------------------
models Manager:
{{{
class ModelManager(models.Manager):
def get_queryset(self):
return super(ModelManager,
self).get_queryset().filter(parent__isnull=False)
}}}
models:
{{{
class Category(models.Model):
parent = models.ForeignKey('Category', to_field='slug',
db_column='parent',
max_length=32, blank=True, null=True,
related_name='models')
name = models.CharField(max_length=50, blank=True, null=True)
....
vehicle_models = ModelManager()
class Meta:
db_table = 'open_category'
ordering = ['pinyin']
}}}
query:
{{{
Category.vehicle_models.filter(slug=model_slug).values('name').query
}}}
query's sql output is:
{{{
SELECT `open_category`.`name` FROM `open_category` WHERE
(`open_category`.`parent` IS NULL AND `open_category`.`slug` = "suteng")
ORDER BY `open_category`.`pinyin` ASC
}}}
it should be '''`open_category`.`parent` IS NOT NULL'''
--
Ticket URL: <https://code.djangoproject.com/ticket/26983>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: weidwonder | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: queryset | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by weidwonder):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
> models Manager:
>
> {{{
> class ModelManager(models.Manager):
> def get_queryset(self):
> return super(ModelManager,
> self).get_queryset().filter(parent__isnull=False)
> }}}
>
> models:
>
> {{{
> class Category(models.Model):
> parent = models.ForeignKey('Category', to_field='slug',
> db_column='parent',
> max_length=32, blank=True, null=True,
> related_name='models')
> name = models.CharField(max_length=50, blank=True, null=True)
> ....
> vehicle_models = ModelManager()
>
> class Meta:
> db_table = 'open_category'
> ordering = ['pinyin']
>
> }}}
>
> query:
>
> {{{
> Category.vehicle_models.filter(slug=model_slug).values('name').query
> }}}
>
> query's sql output is:
>
> {{{
> SELECT `open_category`.`name` FROM `open_category` WHERE
> (`open_category`.`parent` IS NULL AND `open_category`.`slug` = "suteng")
> ORDER BY `open_category`.`pinyin` ASC
> }}}
>
> it should be '''`open_category`.`parent` IS NOT NULL'''
New description:
models Manager:
{{{
class ModelManager(models.Manager):
def get_queryset(self):
return super(ModelManager,
self).get_queryset().filter(parent__isnull=False)
}}}
models:
{{{
class Category(models.Model):
parent = models.ForeignKey('Category', to_field='slug',
db_column='parent',
max_length=32, blank=True, null=True,
related_name='models')
name = models.CharField(max_length=50, blank=True, null=True)
....
vehicle_models = ModelManager()
class Meta:
db_table = 'open_category'
ordering = ['pinyin']
}}}
query:
{{{
Category.vehicle_models.filter(slug=model_slug).values('name').query
}}}
query's sql output is:
{{{
SELECT `open_category`.`name` FROM `open_category` WHERE
(`open_category`.`parent` IS NULL AND `open_category`.`slug` = "suteng")
ORDER BY `open_category`.`pinyin` ASC
}}}
it should be '''`open_category`.`parent` IS NOT NULL''', I downgrade
django to 1.9.9, it works.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:1>
Django
Reporter: weidwonder | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* Attachment "testcase-26983.diff" added.
Reproduction testcase
Django
Reporter: weidwonder | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Hi,
Using the attached testcase based on your description, I managed to
confirm the issue and tracked it down to commit
388bb5bd9aa3cd43825cd8a3632a57d8204f875f.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:2>
Django
Reporter: weidwonder | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* Attachment "testcase-26983.diff" added.
Reproduction testcase (simplified)
Django
Reporter: weidwonder | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by bmispelon):
I simplified the testcase a bit to get rid of some "moving parts".
The key to the failure seems to be the `to_field` attribute of the
ForeignKey. Without it, the query seems to be generated correctly.
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:3>
Django
to_field
-------------------------------------+-------------------------------------
Reporter: weidwonder | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:4>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MatthewWilkes):
I don't believe that IsNull lookups should be converting their values to
the type of the parent field, however the RelatedLookupMixin has useful
looking code for ensuring that MultiColSource is handled correctly.
Would the fix for this be as simple as:
{{{
class RelatedIsNull(RelatedLookupMixin, IsNull):
def get_prep_lookup(self):
return self.rhs
}}}
?
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:5>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
Replying to [comment:5 MatthewWilkes]:
> I don't believe that IsNull lookups should be converting their values to
the type of the parent field, however the RelatedLookupMixin has useful
looking code for ensuring that MultiColSource is handled correctly.
>
> Would the fix for this be as simple as:
>
> {{{
> class RelatedIsNull(RelatedLookupMixin, IsNull):
> def get_prep_lookup(self):
> return self.rhs
> }}}
> ?
`return super(RelatedLookupMixin, self).get_prep_lookup()` would be more
appropriate.
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:6>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: queryset | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/7006 PR] resulting of discussions
with bmispelon on IRC.
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:7>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:8>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.10
(models, ORM) |
Keywords: queryset | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz <claude@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"272eccf7ff0ced609e5a5e3bb6b4a40e773e0e66" 272eccf7]:
{{{
#!CommitTicketReference repository=""
revision="272eccf7ff0ced609e5a5e3bb6b4a40e773e0e66"
Fixed #26983 -- Fixed isnull filtering on ForeignKey with to_field
Thanks weidwonder for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:9>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: queryset | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"6757c94662ef07e35eafcf3ac8bdae07a3d9326a" 6757c946]:
{{{
#!CommitTicketReference repository=""
revision="6757c94662ef07e35eafcf3ac8bdae07a3d9326a"
[1.10.x] Fixed #26983 -- Fixed isnull filtering on ForeignKey with
to_field
Thanks weidwonder for the report.
Backport of 272eccf7ff0ced609e5a5e3bb6b4a40e773e0e66 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:10>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by lamby):
* status: closed => new
* resolution: fixed =>
Comment:
This also happens for `field_isnull=False` where `field` is a `ForeignKey`
to a model with a `CharField` that is `primary_key=True`. Again,
`self.rhs` is converted from `False` to `'False``, ie a `bool` to a `str`.
Reopening as the pach does not fix this.
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:11>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: queryset | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by lamby):
After writing a testcase (available here:
https://gist.github.com/lamby/394b712e9e9ff1e868a20e67d4ba482b/raw) "git
bisect" told me that it was indeed fixed with the above change.
Investigating more, what happened was that I was applying the patch
against `RelatedIn`instead of `RelatedLookupMixin`. Apologies for the
noise there.
''However'', I am still correct in that the bug is not strictly `to_field`
specific as it affects this `primary_key` case. Therefore, I suggest:
* You update the release documentation, etc. to reflect this
* You add my test (or similar) to prevent a regression
In light of this, I am not resolving this ticket so that these action
items do not get overlooked.
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:12>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 1 => 0
* severity: Release blocker => Normal
* stage: Ready for checkin => Accepted
Comment:
Could you send a pull request with those items? You can use "Refs #26983
--" in the commit message.
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:13>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by lamby):
Replying to [comment:13 timgraham]:
> Could you send a pull request with those items? You can use "Refs #26983
--" in the commit message.
https://github.com/django/django/pull/7104
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:14>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"97513269d73520d10722bbd10404be6ac4d48d07" 9751326]:
{{{
#!CommitTicketReference repository=""
revision="97513269d73520d10722bbd10404be6ac4d48d07"
Refs #26983 -- Added test for isnull lookup to CharField with
primary_key=True.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:15>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"c24a47b3e6cfba88ee7792be25da5b5333266abe" c24a47b3]:
{{{
#!CommitTicketReference repository=""
revision="c24a47b3e6cfba88ee7792be25da5b5333266abe"
[1.10.x] Refs #26983 -- Added test for isnull lookup to CharField with
primary_key=True.
Backport of 97513269d73520d10722bbd10404be6ac4d48d07 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:16>
Django
to_field
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.10
(models, ORM) |
Keywords: queryset | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/26983#comment:17>