[Django] #31486: Related filters with unsaved objects

Django

unread,

Apr 20, 2020, 7:55:48 AM4/20/20

to django-...@googlegroups.com

#31486: Related filters with unsaved objects
-------------------------------------+-------------------------------------
Reporter: Mapiarz | Owner: nobody
Type: | Status: new
Uncategorized |
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Consider this filter:
{{{
Foo.objects.filter(related_obj=bar)
}}}
Where 'bar' is an unsaved object instance. In Django 1.11, this would
always return an empty QuerySet (since no Foo object is related to unsaved
'bar'). In Django 2.0 through 2.2, this is equivalent to doing (which can
return a non-empty QuerySet):
{{{
Foo.objects.filter(related_obj=None)
}}}

I found a somewhat related issue that touches on this subject:
https://code.djangoproject.com/ticket/27985

My questions:
1. What is the intended behaviour? In the aforementioned issue Simon
Charette suggests that unsaved objects should be prevented from being used
in related filters. I agree with that.
2. Is this documented anywhere? I couldn't find anything. At the very
least this should be documented somewhere.

--
Ticket URL: <https://code.djangoproject.com/ticket/31486>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Apr 20, 2020, 9:47:22 AM4/20/20

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.

-------------------------------------+-------------------------------------
Reporter: Mapiarz | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* type: Uncategorized => Bug
* version: 2.2 => master
* stage: Unreviewed => Accepted

Comment:

Regarding [https://code.djangoproject.com/ticket/27985#comment:14 this
comment] I still think that deprecating passing unsaved objects to related
filters is worth doing so I'll accept this ticket on this basis.

Mapiarz, would you be interested in
[https://docs.djangoproject.com/en/3.0/internals/contributing/writing-code
/submitting-patches/#deprecating-a-feature submitting a patch doing so]?
It might require a bit of adjustments in the suite but I think that
warning on `obj.pk is None` is the way to go as it's less controversial
than `obj._state.adding`.

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:1>

Django

unread,

Apr 21, 2020, 8:40:14 AM4/21/20

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.
-------------------------------------+-------------------------------------

Reporter: Mapiarz | Owner: Hasan
| Ramezani
Type: Bug | Status: assigned

Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Hasan Ramezani):

* owner: nobody => Hasan Ramezani
* status: new => assigned
* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:2>

Django

unread,

Apr 22, 2020, 4:04:14 AM4/22/20

to django-...@googlegroups.com

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* cc: Simon Charette (added)
* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:3>

Django

unread,

May 2, 2020, 4:39:44 PM5/2/20

to django-...@googlegroups.com

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Hasan Ramezani):

As I [https://github.com/django/django/pull/12768#discussion_r412815335
commented on PR]:

I don't know how to distinguish between `p2.choice_set.all()` and
`Choice.objects.filter(poll=p2)`, It seems `get_normalized_value` receives
the same (value and lhs) for these both cases.

I've also tried to implement it in `RelatedLookupMixin.get_prep_lookup`
based on
[https://github.com/django/django/pull/12768#discussion_r412882870 Mariusz
comment on PR], but still same problem. the `lhs` and `rhs` have the same
value in `p2.choice_set.all()` and `Choice.objects.filter(poll=p2)` cases.

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:4>

unread,

Jan 23, 2022, 3:14:25 PM1/23/22

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.
-------------------------------------+-------------------------------------
Reporter: Mapiarz | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by raydeal):

PR was closed because of
([https://github.com/django/django/pull/12768#discussion_r412762330
Mariusz comment on PR]) ticket #7488 - admin panel might use filtering
with not saved model. But it is 14 years old ticket and it is worth to
check, in current version, if admin still use that.

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:7>

Django

unread,

Feb 3, 2022, 11:51:37 AM2/3/22

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.
-------------------------------------+-------------------------------------

Reporter: Mapiarz | Owner: raydeal
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by raydeal):

* owner: (none) => raydeal

* status: new => assigned

Comment:

After investigation of this ticket and #7488 conclusion is:
Issue in the #7488 ticket is obsolete. It is about {{{ get_querystet }}}
code that haven't existed since 2008.
Time line is as follow:
19-07-2008 newform-admin was merged with main branch
23-07-2008 #7488 ticket was closed
14-11-2008 the {{{ BaseInlineFormSet.get_queryset }}} code
(forms/models.py) mentioned in #7488 was deleted in #9076 (not closed yet
???) (https://code.djangoproject.com/ticket/9076#comment:22) and queryset
moved to
{{{
BaseInlineFormSet.__init__
}}}
https://github.com/django/django/commit/bca14cd3c8685f0c8d6a24583e3de33f94f8910b
#diff-360e00ebf46ef996c61729321d5a59992d78be2ad6913fb546394c2817b3837a
28-12-2012 this code was changed #19524
20-11-2013 this code was changed #21472
Now the code is

{{{
if self.instance.pk is not None:
qs = queryset.filter(**{self.fk.name: self.instance})
else:
qs = queryset.none()
}}}
It means that passing of unsaved object to related filter in admin Inlines
is already solved. Ticket #9076 might be closed.

Apart from this, the test
https://github.com/django/django/blob/4f8c7fd9d91b35e2c2922de4bb50c8c8066cbbc6/tests/model_inheritance_regress/tests.py#L185
could be renamed and used as it was in
[https://github.com/django/django/pull/12768/files/fe4eab0a800cb9bd061d14fb58adea82ef74ef5c#r412762330
PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:8>

Django

unread,

Feb 3, 2022, 5:10:24 PM2/3/22

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.
-------------------------------------+-------------------------------------
Reporter: Mapiarz | Owner: raydeal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by raydeal):

Replying to [comment:4 Hasan Ramezani]:

> As I [https://github.com/django/django/pull/12768#discussion_r412815335
commented on PR]:
>
> I don't know how to distinguish between `p2.choice_set.all()` and
`Choice.objects.filter(poll=p2)`, It seems `get_normalized_value` receives
the same (value and lhs) for these both cases.
>
> I've also tried to implement it in `RelatedLookupMixin.get_prep_lookup`
based on
[https://github.com/django/django/pull/12768#discussion_r412882870 Mariusz
comment on PR], but still same problem. the `lhs` and `rhs` have the same
value in `p2.choice_set.all()` and `Choice.objects.filter(poll=p2)` cases.

After looking at code and trying implement solution, my conclusion is: it
is connected with #19580, if FK raises ValueError like M2M does it will
open door to fix this without a struggle how to distinguish between
`p2.choice_set.all()` and `Choice.objects.filter(poll=p2)` because first
case will raise ValueError before `filter` is evaluated.

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:9>

Django

unread,

Feb 5, 2022, 3:28:15 PM2/5/22

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.
-------------------------------------+-------------------------------------
Reporter: Mapiarz | Owner: raydeal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by raydeal):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:10>

Django

unread,

Feb 24, 2022, 4:30:49 AM2/24/22

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.
-------------------------------------+-------------------------------------
Reporter: Mapiarz | Owner: raydeal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Agreed, `test_issue_7488` is obsolete, removed in
18245b948bf7032a0b50d92a743eff822f5bc6a6.

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:11>

Django

unread,

Feb 25, 2022, 1:52:13 AM2/25/22

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.
-------------------------------------+-------------------------------------
Reporter: Mapiarz | Owner: raydeal
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:12>

Django

unread,

Feb 25, 2022, 7:39:38 AM2/25/22

to django-...@googlegroups.com

#31486: Deprecate passing unsaved objects to related filters.
-------------------------------------+-------------------------------------
Reporter: Mapiarz | Owner: raydeal

Type: Bug | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"2b6a3baebe70e4d481dd256bdb774ac15e248540" 2b6a3bae]:
{{{
#!CommitTicketReference repository=""
revision="2b6a3baebe70e4d481dd256bdb774ac15e248540"
Fixed #31486 -- Deprecated passing unsaved objects to related filters.

Co-Authored-By: Hasan Ramezani <hasa...@gmail.com>
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31486#comment:13>

Django

unread,

Jan 17, 2023, 5:49:46 AM1/17/23

to django-...@googlegroups.com

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"4d78d7338cc5cfaa0afcfc9d240d753328bbf399" 4d78d73]:
{{{
#!CommitTicketReference repository=""
revision="4d78d7338cc5cfaa0afcfc9d240d753328bbf399"
Refs #31486 -- Removed ability to pass unsaved model instances to related
filters.