[Django] #32281: Prefetech Object - to_attr has ambiguous behavior when using a related lookup

13 views
Skip to first unread message

Django

unread,
Dec 17, 2020, 6:11:58 AM12/17/20
to django-...@googlegroups.com
#32281: Prefetech Object - to_attr has ambiguous behavior when using a related
lookup
-----------------------------------------+------------------------
Reporter: talolard | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
**Code Example**
{{{
from django.db.models import Prefetch

p = Prefetch("a_set", to_attr="my_a")
assert p.prefetch_to == "my_a"

p = Prefetch("b__a_set", to_attr="my_a")
assert p.prefetch_to == "my_a", p.prefetch_to

}}}

That last assertion fails because p.prefetch_to == "b__my_a"

**Description and use case**
We have a models X, Y and Z
Z has a foreign key to Y and Y has a foreign key to Z
We'd like to fetch X with all of it's Z's using prefetch related and to be
able to set the attribute to "z"

But as above, this doesn't seem to work.

The
[https://github.com/django/django/blob/bb64b99b78a579cb2f6178011a4cf9366e634438/django/db/models/query.py#L1570-L1587
relevant Django code] seems to ignore this use case (or discourage it?)
and instead puts to "z" attribute on each "Y"


{{{
self.prefetch_to =
LOOKUP_SEP.join(lookup.split(LOOKUP_SEP)[:-1] + [to_attr])

}}}

----

I assume "fixing this" is a breaking change, but could we do an explicit
parameter for prefetch_to ?At the moment we're setting it manually after
creating the prefetch object

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

Django

unread,
Dec 17, 2020, 7:06:57 AM12/17/20
to django-...@googlegroups.com
#32281: Prefetech Object - to_attr has ambiguous behavior when using a related
lookup
-------------------------------------+-------------------------------------
Reporter: Tal Perry | Owner: nobody
Type: New feature | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => wontfix
* component: Uncategorized => Database layer (models, ORM)
* type: Uncategorized => New feature


Old description:

> **Code Example**
> {{{
> from django.db.models import Prefetch
>
> p = Prefetch("a_set", to_attr="my_a")
> assert p.prefetch_to == "my_a"
>
> p = Prefetch("b__a_set", to_attr="my_a")
> assert p.prefetch_to == "my_a", p.prefetch_to
>
> }}}
>
> That last assertion fails because p.prefetch_to == "b__my_a"
>
> **Description and use case**
> We have a models X, Y and Z
> Z has a foreign key to Y and Y has a foreign key to Z
> We'd like to fetch X with all of it's Z's using prefetch related and to
> be able to set the attribute to "z"
>
> But as above, this doesn't seem to work.
>
> The
> [https://github.com/django/django/blob/bb64b99b78a579cb2f6178011a4cf9366e634438/django/db/models/query.py#L1570-L1587
> relevant Django code] seems to ignore this use case (or discourage it?)
> and instead puts to "z" attribute on each "Y"
>

> {{{
> self.prefetch_to =
> LOOKUP_SEP.join(lookup.split(LOOKUP_SEP)[:-1] + [to_attr])
>
> }}}
>

>
> ----
>
> I assume "fixing this" is a breaking change, but could we do an explicit
> parameter for prefetch_to ?At the moment we're setting it manually after
> creating the prefetch object

New description:

**Code Example**
{{{
from django.db.models import Prefetch

p = Prefetch("a_set", to_attr="my_a")
assert p.prefetch_to == "my_a"

p = Prefetch("b__a_set", to_attr="my_a")
assert p.prefetch_to == "my_a", p.prefetch_to

}}}

That last assertion fails because `p.prefetch_to == "b__my_a"`

**Description and use case**
We have a models X, Y and Z
Z has a foreign key to Y and Y has a foreign key to Z
We'd like to fetch X with all of it's Z's using prefetch related and to be
able to set the attribute to "z"

But as above, this doesn't seem to work.

The
[https://github.com/django/django/blob/bb64b99b78a579cb2f6178011a4cf9366e634438/django/db/models/query.py#L1570-L1587
relevant Django code] seems to ignore this use case (or discourage it?)
and instead puts to "z" attribute on each "Y"


{{{
self.prefetch_to =
LOOKUP_SEP.join(lookup.split(LOOKUP_SEP)[:-1] + [to_attr])

}}}

----

I assume "fixing this" is a breaking change, but could we do an explicit
parameter for prefetch_to ?At the moment we're setting it manually after
creating the prefetch object

--

Comment:

Thanks for this ticket, however IMO this is an expected behavior. In the
second example `a_set` is a field lookup on `b` so we can change its name
to `my_a` (in `b`) but we cannot assign it directly to the main object, it
will be misleading.

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

Django

unread,
Dec 17, 2020, 8:48:39 AM12/17/20
to django-...@googlegroups.com
#32281: Prefetech Object - to_attr has ambiguous behavior when using a related
lookup
-------------------------------------+-------------------------------------
Reporter: Tal Perry | Owner: nobody
Type: New feature | Status: new

Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tal Perry):

* status: closed => new
* resolution: wontfix =>


Comment:

I understand that we can't assign to the main object, but am reopening
because I think significant confusion remains.

In our use case, we're assigning a filtered queryset to the prefetch
object .


{{{
annotations = Prefetch(
"tokens__tagged_tokens", queryset=tagged_token_qs,
to_attr="annotations",
)
}}}

In the above example it's not clear whether the resulting prefetch will
bring all tokens, or only those tokens who have a tagged_token in the
supplied queryset.

Perhaps the solution is as simple as making this explicit in the docs.
Happy to contribute if that's the right way and someone can shed light on
the behavior and any edge cases.

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

Django

unread,
Dec 17, 2020, 11:34:58 AM12/17/20
to django-...@googlegroups.com
#32281: Prefetech Object - to_attr has ambiguous behavior when using a related
lookup
-------------------------------------+-------------------------------------
Reporter: Tal Perry | Owner: nobody
Type: New feature | Status: closed

Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => wontfix


Comment:

> In our use case, we're assigning a filtered queryset to the prefetch
object .
>
> {{{
> annotations = Prefetch(
> "tokens__tagged_tokens", queryset=tagged_token_qs,
to_attr="annotations",
> )
> }}}
>
> In the above example it's not clear whether the resulting prefetch will
bring all tokens, or only those tokens who have a tagged_token in the
supplied queryset.
>
> Perhaps the solution is as simple as making this explicit in the docs.
Happy to contribute if that's the right way and someone can shed light on
the behavior and any edge cases.

I'm not sure how this question is related with the ticket. It's explicitly
stated in
[https://docs.djangoproject.com/en/3.1/ref/models/querysets/#django.db.models.Prefetch
docs]:

> The queryset argument supplies a base QuerySet for the given lookup.

[https://docs.djangoproject.com/en/stable/internals/contributing/triaging-
tickets/#closing-tickets Please follow triaging guidelines with regards to
wontfix tickets.]

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

Reply all
Reply to author
Forward
0 new messages