Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#17991 closed Bug (fixed)

Prefetch_related fails with GenericRelation

Reported by: okke@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords:
Cc: lemaire.adrien@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

While greatly anticipating prefetch_related, I am sorry to say that bug #17838 is not yet entirely fixed. I am running into empty result sets when I perform a prefetch_related.

I have a GenericRelation on my model to Comments. My model uses an integer primary key. This combination makes the prefetch_related return empty embedded result sets.

In the latest django, django/db/models/query.py, line 1791, the _prefetched_objects_cache is accessed with (possibly) an integer as value. However, the _prefetched_objects_cache is always populated with unicode values (lines 1770 - 1774)

The quick fix is to replace (line 1791)

            obj._prefetched_objects_cache[cache_name] = qs

with

            obj._prefetched_objects_cache[unicode(cache_name)] = qs

Can someone create a patch for this, and write a test to see if the queryset is actually populated with related items?

Attachments (2)

fix_issue_17991.patch (515 bytes) - added by Okke 3 years ago.
generic_foreign_key_char_test.diff (2.4 KB) - added by carmandrew@… 3 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 3 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I'm sorry, I made a mistake in the line in which the error is.

It's line 1778:

        vals = rel_obj_cache.get(instance_attr_val, [])

which should be

        vals = rel_obj_cache.get(unicode(instance_attr_val), [])

Changed 3 years ago by Okke

comment:2 Changed 3 years ago by Okke

  • Has patch set
  • Needs tests set

comment:3 Changed 3 years ago by Fandekasp

  • Cc lemaire.adrien@… added
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 3 years ago by tonnzor

I got the same bug -- see #18200 . The fix in #comment:1 fixed the issue for me

comment:5 Changed 3 years ago by stephanejais

There's more to patch if you want "relations of relations" to also work:

> diff /Users/stephh/.virtualenv/azap/lib/python2.7/site-packages/django/db/models/query.py django/db/models/query.py
1773,1774c1773,1774
<             rel_obj_cache[rel_attr_val] = []
<         rel_obj_cache[rel_attr_val].append(rel_obj)
---
>             rel_obj_cache[unicode(rel_attr_val)] = []
>         rel_obj_cache[unicode(rel_attr_val)].append(rel_obj)
1778c1778
<         vals = rel_obj_cache.get(instance_attr_val, [])
---
>         vals = rel_obj_cache.get(unicode(instance_attr_val), [])

comment:6 Changed 3 years ago by lukeplant

  • Easy pickings unset
  • Patch needs improvement set

I think at this point a test demonstrating the problem would be the most useful thing. Blindly converting things to unicode seems like altogether the wrong approach. If there is any type conversion to be done, it is surely something that the model fields will dictate.

comment:7 Changed 3 years ago by carmandrew@…

  • Needs tests unset

I ran into the same issue. There is a bug with the way that prefetch_related is handling GenericRelations where the GenericForeignKey's object_id is a different type than the primary key of the model it's pointing to. I've attached a test case.

Changed 3 years ago by carmandrew@…

comment:8 Changed 3 years ago by Luke Plant <L.Plant.98@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 4c4d08502cbef6e0ed85470b2a5aade7fafa099f:

Fixed #17991 - prefetch_related fails with GenericRelation and varchar ID field

Thanks to okke@… for the report, and carmandrew@… for the tests.

comment:9 Changed 3 years ago by Luke Plant <L.Plant.98@…>

In d7d7ad2881b66d2c6be45ab0b055f492d3b589e9:

[1.5.x] Fixed #17991 - prefetch_related fails with GenericRelation and varchar ID field

Thanks to okke@… for the report, and carmandrew@… for the tests.

Backport of ccd14ff25b7642678bf3c9ed8a12643f04853144 from master

Note: See TracTickets for help on using tickets.
Back to Top