[Django] #34791: Issue when using Prefetch objects in prefetch_related
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
MaxDude132 |
Type: Bug | Status: new
Component: Database | Version: 4.2
layer (models, ORM) | Keywords: prefetch,
Severity: Normal | prefetch_related
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
There seems to be an issue when using a Prefetch object to fetch something
that has already been fetched. Here is an example: so let's say I have a
Pizza and some Toppings. Now I want to get all the toppings, but for some
reason I also want to separately fetch only the toppings that are out of
stock. I could do something like:
{{{
queryset = Pizza.objects.all().prefetch_related(
'toppings',
Prefetch('toppings',
queryset=Topping.objects.filter(is_in_stock=False),
to_attr='out_of_stock_toppings'),
)
}}}
This looks good, but if we run it, it will fail, saying
out_of_stock_toppings is not an attribute of Pizza. However, if I were to
do it like this instead:
{{{
queryset = Pizza.objects.all().prefetch_related(
Prefetch('toppings',
queryset=Topping.objects.filter(is_in_stock=False),
to_attr='out_of_stock_toppings'),
'toppings',
)
}}}
then all works fine. Looking at the code, this seems to be because the
name used by a field to validate the cache is not the name used to store
the data on the model, but rather simply the name of the field, so it
collides. I have not tested it, but I think in the second example, the
data returned will actually be the filtered data, not the full expected
queryset. Note that this is my first time reading that part of the code,
so there could be things I missed.
Now this is a bit of a nonsensical example, but when using rest_framework
with serializers, this type of situation could come up, where one
serializer needs it formatted a certain way and this issue could arise (it
has for me).
I am not sure what the best way to fix this would be, but I feel like
setting a to_attr should make the cache take that new field name into
account instead of the field name.
--
Ticket URL: <https://code.djangoproject.com/ticket/34791>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia Bidart):
Hello! Thank you for your report. Could you please post your models.py
definition (reduced to this example)? Also it would be useful if you can
paste the query each example generate.
This way, we can try to reproduce and triage accordingly. Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/34791#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Maxime Toussaint | Owner: nobody
Component: Database layer | Version: 4.2
Severity: Normal | worksforme
Keywords: prefetch, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => worksforme
Comment:
I have created a few models of my own and tested your queries. The queries
are correct in both cases and I get expected results. I have created many
toppings and `onion` and `tomato` are out of stock:
{{{
>>> def print_toppings(q):
... print([t.name for p in q for t in p.toppings.all()], [t.name for p
in q for t in p.out_of_stock_toppings])
...
>>> q1 =
Pizza.objects.all().prefetch_related('toppings',Prefetch('toppings',
queryset=Topping.objects.filter(is_in_stock=False),
to_attr='out_of_stock_toppings'))
>>> print_toppings(q1)
['sweet pepper', 'egg', 'ham', 'bacon', 'onion', 'cheese', 'tomato',
'onion', 'sweet pepper', 'egg', 'cheese', 'onion', 'tomato'] ['onion',
'tomato', 'onion', 'onion', 'tomato']
>>> print("\n\n".join(i["sql"] for i in connection.queries))
SELECT "ticket_34791_pizza"."id", "ticket_34791_pizza"."name" FROM
"ticket_34791_pizza"
SELECT ("ticket_34791_pizza_toppings"."pizza_id") AS
"_prefetch_related_val_pizza_id", "ticket_34791_topping"."id",
"ticket_34791_topping"."name", "ticket_34791_topping"."is_in_stock" FROM
"ticket_34791_topping" INNER JOIN "ticket_34791_pizza_toppings" ON
("ticket_34791_topping"."id" = "ticket_34791_pizza_toppings"."topping_id")
WHERE "ticket_34791_pizza_toppings"."pizza_id" IN (2, 3, 4, 1)
SELECT ("ticket_34791_pizza_toppings"."pizza_id") AS
"_prefetch_related_val_pizza_id", "ticket_34791_topping"."id",
"ticket_34791_topping"."name", "ticket_34791_topping"."is_in_stock" FROM
"ticket_34791_topping" INNER JOIN "ticket_34791_pizza_toppings" ON
("ticket_34791_topping"."id" = "ticket_34791_pizza_toppings"."topping_id")
WHERE (NOT "ticket_34791_topping"."is_in_stock" AND
"ticket_34791_pizza_toppings"."pizza_id" IN (2, 3, 4, 1))
}}}
The second query also works fine:
{{{
>>> q2 = Pizza.objects.all().prefetch_related(Prefetch('toppings',
queryset=Topping.objects.filter(is_in_stock=False),
to_attr='out_of_stock_toppings'),'toppings')
>>> print_toppings(q2)
['sweet pepper', 'egg', 'ham', 'bacon', 'onion', 'cheese', 'tomato',
'onion', 'sweet pepper', 'egg', 'cheese', 'onion', 'tomato'] ['onion',
'tomato', 'onion', 'onion', 'tomato']
>>> print("\n\n".join(i["sql"] for i in connection.queries))
SELECT "ticket_34791_pizza"."id", "ticket_34791_pizza"."name" FROM
"ticket_34791_pizza"
SELECT ("ticket_34791_pizza_toppings"."pizza_id") AS
"_prefetch_related_val_pizza_id", "ticket_34791_topping"."id",
"ticket_34791_topping"."name", "ticket_34791_topping"."is_in_stock" FROM
"ticket_34791_topping" INNER JOIN "ticket_34791_pizza_toppings" ON
("ticket_34791_topping"."id" = "ticket_34791_pizza_toppings"."topping_id")
WHERE (NOT "ticket_34791_topping"."is_in_stock" AND
"ticket_34791_pizza_toppings"."pizza_id" IN (2, 3, 4, 1))
SELECT ("ticket_34791_pizza_toppings"."pizza_id") AS
"_prefetch_related_val_pizza_id", "ticket_34791_topping"."id",
"ticket_34791_topping"."name", "ticket_34791_topping"."is_in_stock" FROM
"ticket_34791_topping" INNER JOIN "ticket_34791_pizza_toppings" ON
("ticket_34791_topping"."id" = "ticket_34791_pizza_toppings"."topping_id")
WHERE "ticket_34791_pizza_toppings"."pizza_id" IN (2, 3, 4, 1)
}}}
You should note that when using `to_attr`, the result is stored in a list
as documented here (see the Note):
https://docs.djangoproject.com/en/4.2/ref/models/querysets/#prefetch-
objects
--
Ticket URL: <https://code.djangoproject.com/ticket/34791#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Maxime Toussaint | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) | Resolution:
Severity: Normal | worksforme
Keywords: prefetch, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Maxime Toussaint):
Hi, thanks a lot for the quick answer. I seem to have misunderstood the
parameters of my issue, and the example I gave does work for me. In trying
to simplify it, I took out the part that was making it fail.
So the issue only seems to happen when there is depth in the prefetch.
Here is an example I made this morning that fails:
{{{
pizzas = Pizza.objects.all().prefetch_related(
"toppings__origin",
Prefetch(
"toppings__origin",
queryset=Country.objects.filter(label="China"),
to_attr="china",
),
)
china = pizzas[0].toppings.all()[0].china
}}}
Here, when trying to get china, I would assume it to either be a Country
object or None. However, I get the message: AttributeError: 'Topping'
object has no attribute 'china'
Here are the models I set up for my test:
{{{
class Country(models.Model):
label = models.CharField(max_length=50)
class Pizza(models.Model):
label = models.CharField(max_length=50)
class Topping(models.Model):
pizza = models.ForeignKey(Pizza, on_delete=models.CASCADE,
related_name="toppings")
label = models.CharField(max_length=50)
origin = models.ForeignKey(
Country, on_delete=models.CASCADE, related_name="toppings"
)
}}}
And here are the queries made when calling the queryset:
{{{
1. SELECT "tests_pizza"."id", "tests_pizza"."label",
"tests_pizza"."provenance_id" FROM "tests_pizza"
2. SELECT "tests_topping"."id", "tests_topping"."pizza_id",
"tests_topping"."label", "tests_topping"."origin_id" FROM "tests_topping"
WHERE "tests_topping"."pizza_id" IN (1, 2)
3. SELECT "tests_country"."id", "tests_country"."label",
"tests_country"."continent_id" FROM "tests_country" WHERE
"tests_country"."id" IN (2, 3, 4)
}}}
Note that the filter by label='china' has completely disappeared.
Now, if I switch the prefetches around like so:
{{{
pizzas = Pizza.objects.all().prefetch_related(
Prefetch(
"toppings__origin",
queryset=Country.objects.filter(label="China"),
to_attr="china",
),
"toppings__origin",
)
}}}
Fetching china now works, and here are the queries being made:
{{{
1. SELECT "tests_pizza"."id", "tests_pizza"."label",
"tests_pizza"."provenance_id" FROM "tests_pizza"
2. SELECT "tests_topping"."id", "tests_topping"."pizza_id",
"tests_topping"."label", "tests_topping"."origin_id" FROM "tests_topping"
WHERE "tests_topping"."pizza_id" IN (1, 2)
3. SELECT "tests_country"."id", "tests_country"."label",
"tests_country"."continent_id" FROM "tests_country" WHERE
("tests_country"."label" = 'China' AND "tests_country"."id" IN (2, 3, 4))
4. SELECT "tests_country"."id", "tests_country"."label",
"tests_country"."continent_id" FROM "tests_country" WHERE
"tests_country"."id" IN (2, 3, 4)
}}}
This time, both calls to Country were made.
I did follow the code a bit yesterday, and I believe it comes from the
fact that the ForeignKey field defines the cache name as being simply the
name of the field. I am not certain though, and it would likely require
someone with more knowledge than me to look into it.
Thanks again, hopefully we can figure out what is happening!
--
Ticket URL: <https://code.djangoproject.com/ticket/34791#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Maxime Toussaint | Owner: nobody
Component: Database layer | Version: 4.2
(models, ORM) |
Keywords: prefetch, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: worksforme =>
--
Ticket URL: <https://code.djangoproject.com/ticket/34791#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Maxime Toussaint | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> There seems to be an issue when using a Prefetch object to fetch
> something that has already been fetched. Here is an example: so let's say
> I have a Pizza and some Toppings. Now I want to get all the toppings, but
> for some reason I also want to separately fetch only the toppings that
> are out of stock. I could do something like:
>
> {{{
> queryset = Pizza.objects.all().prefetch_related(
> 'toppings',
> Prefetch('toppings',
> queryset=Topping.objects.filter(is_in_stock=False),
> to_attr='out_of_stock_toppings'),
> )
> }}}
>
> This looks good, but if we run it, it will fail, saying
> out_of_stock_toppings is not an attribute of Pizza. However, if I were to
> do it like this instead:
> {{{
> queryset = Pizza.objects.all().prefetch_related(
> Prefetch('toppings',
> queryset=Topping.objects.filter(is_in_stock=False),
> to_attr='out_of_stock_toppings'),
> 'toppings',
> )
> }}}
>
> then all works fine. Looking at the code, this seems to be because the
> name used by a field to validate the cache is not the name used to store
> the data on the model, but rather simply the name of the field, so it
> collides. I have not tested it, but I think in the second example, the
> data returned will actually be the filtered data, not the full expected
> queryset. Note that this is my first time reading that part of the code,
> so there could be things I missed.
>
> Now this is a bit of a nonsensical example, but when using rest_framework
> with serializers, this type of situation could come up, where one
> serializer needs it formatted a certain way and this issue could arise
> (it has for me).
>
> I am not sure what the best way to fix this would be, but I feel like
> setting a to_attr should make the cache take that new field name into
> account instead of the field name.
New description:
Note: Edited the description following the discussion
There seems to be an issue when using a Prefetch object to fetch something
that has already been fetched. The issue only seems to happen when there
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34791#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Maxime Toussaint | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Maxime Toussaint):
Reopened because I managed to create a test that shows the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/34791#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Maxime Toussaint | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
> I did follow the code a bit yesterday, and I believe it comes from the
fact that the ForeignKey field defines the cache name as being simply the
name of the field. I am not certain though, and it would likely require
someone with more knowledge than me to look into it.
As far as I'm aware this is an expected behavior, subsequent lookups in
`prefetch_related()` affect each other, see
[https://docs.djangoproject.com/en/stable/ref/models/querysets/#django.db.models.query.QuerySet.prefetch_related
docs]:
> ''"The ordering of lookups matters."''
You prefetched `toppings__origin` with a default queryset, so the second
`Prefetch()` ignores a custom queryset on the same relation. You should
use `Prefetch(..., to_attr="")` in both cases to have two independent
queries.
In such cases, we may consider raising `ValueError`.
--
Ticket URL: <https://code.djangoproject.com/ticket/34791#comment:7>