[Django] #22382: ManyRelatedManager's get_prefetch_queryset doesn't validate the prefetch types
Django
types
----------------------------------------------+--------------------
Reporter: Keryn Knight <django@…> | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
[https://github.com/django/django/blob/a2407c9577c400bf9931ff1db1d7757afa378162/django/db/models/fields/related.py#L898
the return value] for `get_prefetch_queryset` includes two lambda
functions, one for
[https://github.com/django/django/blob/a2407c9577c400bf9931ff1db1d7757afa378162/django/db/models/query.py#L1909
setting the resulting tuple] as a key in the `rel_obj_cache` and another
for
[https://github.com/django/django/blob/a2407c9577c400bf9931ff1db1d7757afa378162/django/db/models/query.py#L1913
getting it back later].
The
[https://github.com/django/django/blob/a2407c9577c400bf9931ff1db1d7757afa378162/django/db/models/fields/related.py#L900
one which is used for retrieving] the data uses `getattr` while the other
pulls the data from the `_prefetch_related_val_...` attribute returned as
part of the `QuerySet.extra` call. The prefetched values do not
necessarily correlate to the intended types, however, and so values can
end up mismatched.
By way of example, and how I discovered this, [https://github.com/dcramer
/django-uuidfield django-uuidfield] returns a `StringUUID` in it's
[https://github.com/dcramer/django-
uuidfield/blob/master/uuidfield/fields.py#L138 to_python] method, which is
what ends up being returned
[https://github.com/django/django/blob/a2407c9577c400bf9931ff1db1d7757afa378162/django/db/models/fields/related.py#L900
as instance_attr] - something like
`(UUID('7d917781c54e4fdfa551a693c3782380'),)` but the
[https://github.com/django/django/blob/a2407c9577c400bf9931ff1db1d7757afa378162/django/db/models/fields/related.py#L899
rel_obj_attr] doesn't take into account the `to_python` for the relation,
and so returns `(u'7d917781c54e4fdfa551a693c3782380',)` - the naive
strings from the database.
Thus, the key `(u'7d917781c54e4fdfa551a693c3782380',)` never exists in the
`rel_obj_cache`, and Django assumes everything went well (because it just
sets a default of `[]`). Future queries to the relation
(`x.relation.all()`) will yield nothing - they won't trigger a query
because they've been prefetched, but they won't be populated because of
the type mismatch in the dictionary keys.
Arguably the problem exists downstream in `django-uuidfield`, but the
problem *also* exists in Django, and may exist as an edge-case in other
third party fields - I've marked it as master, but the type-mismatch issue
exists from 1.4 (attrgetter version) to 1.6 (list comprehension version)
and looks to still be there in master (generator expression version)
The fix that seems to work for me is transforming:
{{{
lambda result: tuple(getattr(result, '_prefetch_related_val_%s' %
f.attname) for f in fk.local_related_fields),
}}}
into:
{{{
lambda result: tuple(f.rel.get_related_field().to_python(getattr(result,
'_prefetch_related_val_%s' % f.attname)) for f in
fk.local_related_fields),
}}}
I'm not sure if any of the equivalent methods on other classes would be
affected.
--
Ticket URL: <https://code.djangoproject.com/ticket/22382>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
types
Reporter: Keryn Knight | Owner: nobody
<django@…> | Status: new
Type: Bug | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/22382#comment:1>
Django
types
Reporter: Keryn Knight | Owner: mjtamlyn
<django@…> | Status: assigned
Type: Bug | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* owner: nobody => mjtamlyn
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/22382#comment:2>
Django
types
Reporter: Keryn Knight | Owner: Marc
<django@…> | Tamlyn
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Paulo):
I think this is a duplicate of https://code.djangoproject.com/ticket/24912
and got fixed by
https://github.com/django/django/commit/c58755d8757d6d6ad1ab2c52a631aff1b9bae2da.
--
Ticket URL: <https://code.djangoproject.com/ticket/22382#comment:3>
Django
types
Reporter: Keryn Knight | Owner: Marc
<django@…> | Tamlyn
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => duplicate
Comment:
Thanks Paulo.
--
Ticket URL: <https://code.djangoproject.com/ticket/22382#comment:4>