Error when trying to ignore reverse relationships with related_name using the "+"

[Troy Grosfield]

The setup and failing test can be seen below. It's trying to use the related name as an actual field. I didn't see this issue until I just upgraded to django 1.6 yesterday. Am I missing something here?

# models.py

from django.db import models
from django.contrib.auth.models import User

class ModelA(models.Model):
    created_user = models.ForeignKey(
                    User,
                    related_name='%(app_label)s_%(class)s_created_user+')
    last_modified_user = models.ForeignKey(
                    User,
                    related_name='%(app_label)s_%(class)s_last_modified_user+')

# tests.py

from tests.models import ModelA
from django.contrib.auth.models import User


class Django16PrefetchBug(TestCase):

    def test_prefetch_bug(self):
        u1 = User.objects.get_or_create(username='hello1',
                                        email='hello@world.com',
                                        password='hi')[0]
        u2 = User.objects.get_or_create(username='hello2',
                                        email='hello@world.com',
                                        password='hi')[0]
        u3 = User.objects.get_or_create(username='hello3',
                                        email='hello@world.com',
                                        password='hi')[0]

        m1 = ModelA.objects.create(created_user=u1,
                                   last_modified_user=u2)
        m2 = ModelA.objects.create(created_user=u2,
                                   last_modified_user=u3)
        m3 = ModelA.objects.create(created_user=u3,
                                   last_modified_user=u2)
        x = ModelA.objects.all().prefetch_related('created_user')
        # The line below fails when evaluating the query.
        y = list(x)

======================================================================
ERROR: test_prefetch_bug (tests.prefetch_bug.Django16PrefetchBug)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/troy/github/django-recurrence/tests/prefetch_bug.py", line 249, in test_prefetch_bug
    y = list(x)
  File "/path/to/site-packages/django/db/models/query.py", line 96, in __iter__
    self._fetch_all()
  File "/path/to/site-packages/django/db/models/query.py", line 856, in _fetch_all
    self._prefetch_related_objects()
  File "/path/to/site-packages/django/db/models/query.py", line 517, in _prefetch_related_objects
    prefetch_related_objects(self._result_cache, self._prefetch_related_lookups)
  File "/path/to/site-packages/django/db/models/query.py", line 1598, in prefetch_related_objects
    obj_list, additional_prl = prefetch_one_level(obj_list, prefetcher, attr)
  File "/path/to/site-packages/django/db/models/query.py", line 1697, in prefetch_one_level
    prefetcher.get_prefetch_queryset(instances)
  File "/path/to/site-packages/django/db/models/fields/related.py", line 277, in get_prefetch_queryset
    qs = self.get_queryset(instance=instances[0]).filter(**query)
  File "/path/to/site-packages/django/db/models/query.py", line 590, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "/path/to/site-packages/django/db/models/query.py", line 608, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "/path/to/site-packages/django/db/models/sql/query.py", line 1198, in add_q
    clause = self._add_q(where_part, used_aliases)
  File "/path/to/site-packages/django/db/models/sql/query.py", line 1232, in _add_q
    current_negated=current_negated)
  File "/path/to/site-packages/django/db/models/sql/query.py", line 1100, in build_filter
    allow_explicit_fk=True)
  File "/path/to/site-packages/django/db/models/sql/query.py", line 1351, in setup_joins
    names, opts, allow_many, allow_explicit_fk)
  File "/path/to/site-packages/django/db/models/sql/query.py", line 1274, in names_to_path
    "Choices are: %s" % (name, ", ".join(available)))
FieldError: Cannot resolve keyword u'tests_modela_created_user+' into field. Choices are: date_joined, email, first_name, groups, id, is_active, is_staff, is_superuser, last_login, last_name, password, user_permissions, username

[loic84]

I'm pretty sure this is a regression.

Bisected to 97774429aeb54df4c09895c07cd1b09e70201f7d.

I'll need to investigate some more but for now I'll mark it as a release blocker.

[loic84]

That should more or less do it ​https://gist.github.com/loic/7382392 (quick experiment, not commit material).

For prefetching we probably shouldn't rely on the existence of reverse managers.

The other prefetchers may be suffering from the same issue as well, we'll need to investigate that.

Another problem is that my fix is not going to play well with composite fields...

[loic84]

PR ​https://github.com/django/django/pull/1898.

[loic84]

We test directly ForeignObject ​https://github.com/django/django/blob/master/tests/foreign_object/models.py#L25 which cause a 'ForeignObject' object has no attribute 'related_field' failure.

We may need to use something else than related_field, I'll get back to this later on.

[loic84]

ForeignKey().related_field is a shortcut for ForeignObject().foreign_related_fields[0], I've update the patch to use the latter so it works with plain ForeignObject.

This should be ready for reviews.

[anonymous]

Tested the change from loic and all my tests are passing again. Thanks for the quick turnaround loic84!

[Anssi Kääriäinen]

There is a problem with multicolumn fields here. I think I will go with an approach that looks something like this:

if field.rel.is_hidden():
    # code like in patch
else:
    # code pre-patch

The reason is that we do want to use the fk__in lookup if at all possible. Especially for multicolumn lookups what fk__in=instances does can be quite complex. We don't want to duplicate all the logic needed here. For 1.7 we will likely need to invent something better, as otherwise we will end up having this exact same problem for hidden multicolumn foreign keys.

[loic84]

Updated the PR with @akaariai's suggestion.

[Anssi Kääriäinen <akaariai@…>]

In cb83448891f2920c926f61826d8eae4051a3d8f2:

Fixed #21410 -- prefetch_related() for ForeignKeys with related_name='+'

Regression introduced by commit 9777442.

Thanks to trac username troygrosfield for the report and test case.

[Anssi Kääriäinen <akaariai@…>]

In b107421acfc4046ffaa799aceef2c3b4877207f2:

[1.6.x] Fixed #21410 -- prefetch_related() for ForeignKeys with related_name='+'

Regression introduced by commit 9777442.

Thanks to trac username troygrosfield for the report and test case.

Backpatch of cb83448891f2920c926f61826d8eae4051a3d8f2 from master.

Conflicts:

tests/prefetch_related/models.py

[Loic Bistuer <loic.bistuer@…>]

In d3b71b976dee4b02908235b8007db254a9795268:

Fixed #21760 -- prefetch_related used an inefficient query for reverse FK.

Regression introduced by commit 9777442. Refs #21410.

[Loic Bistuer <loic.bistuer@…>]

In 6b3a8d27052bb5b242ab6c1c2fb0d6da5b49681d:

[1.7.x] Fixed #21760 -- prefetch_related used an inefficient query for reverse FK.

Regression introduced by commit 9777442. Refs #21410.

Backport of d3b71b976d from master

[Loic Bistuer <loic.bistuer@…>]

In 1252b77824639fba398cf70586bdd75c42f86fdf:

[1.6.x] Fixed #21760 -- prefetch_related used an inefficient query for reverse FK.

Regression introduced by commit 9777442. Refs #21410.

Conflicts:

tests/prefetch_related/tests.py

Backport of d3b71b976d from master