Filtering on reverse ForeignKey relation uses get_db_prep_value from a wrong field.

[Michal Petrucha]

Let's start off with a minimal test case.

from django.db import models


class Local(models.Model):
    id = models.DateTimeField(primary_key=True, auto_now_add=True)


class Remote(models.Model):
    fk = models.ForeignKey(Local, on_delete=models.CASCADE)

and

from django.test import TestCase

from . import models


class ReverseFilterTestCase(TestCase):
    def setUp(self):
        self.local = models.Local.objects.create()
        self.remote = models.Remote.objects.create(fk=self.local)

    def test_reverse_filter(self):
        obj = models.Local.objects.filter(remote=self.remote).get()
        self.assertEqual(obj, self.local)

    def test_reverse_filter_direct_field_reference(self):
        obj = models.Local.objects.filter(remote__pk=self.remote.pk).get()
        self.assertEqual(obj, self.local)

I would expect both tests to have the same result. However, what happens is that test_reverse_filter fails with the following exception:

======================================================================
ERROR: test_reverse_filter (reverse_fk_test_case.tests.ReverseFilterTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/koniiiik/env-django/reverse_fk_test_case/reverse_fk_test_case/tests.py", line 12, in test_reverse_filter
    obj = models.Local.objects.filter(remote=self.remote).get()
  File "/home/koniiiik/repos/git/django/django/db/models/query.py", line 408, in get
    num = len(clone)
  File "/home/koniiiik/repos/git/django/django/db/models/query.py", line 258, in __len__
    self._fetch_all()
  File "/home/koniiiik/repos/git/django/django/db/models/query.py", line 1240, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/home/koniiiik/repos/git/django/django/db/models/query.py", line 57, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "/home/koniiiik/repos/git/django/django/db/models/sql/compiler.py", line 1068, in execute_sql
    sql, params = self.as_sql()
  File "/home/koniiiik/repos/git/django/django/db/models/sql/compiler.py", line 481, in as_sql
    where, w_params = self.compile(self.where) if self.where is not None else ("", [])
  File "/home/koniiiik/repos/git/django/django/db/models/sql/compiler.py", line 397, in compile
    sql, params = node.as_sql(self, self.connection)
  File "/home/koniiiik/repos/git/django/django/db/models/sql/where.py", line 81, in as_sql
    sql, params = compiler.compile(child)
  File "/home/koniiiik/repos/git/django/django/db/models/sql/compiler.py", line 397, in compile
    sql, params = node.as_sql(self, self.connection)
  File "/home/koniiiik/repos/git/django/django/db/models/fields/related_lookups.py", line 130, in as_sql
    return super().as_sql(compiler, connection)
  File "/home/koniiiik/repos/git/django/django/db/models/lookups.py", line 162, in as_sql
    rhs_sql, rhs_params = self.process_rhs(compiler, connection)
  File "/home/koniiiik/repos/git/django/django/db/models/lookups.py", line 257, in process_rhs
    return super().process_rhs(compiler, connection)
  File "/home/koniiiik/repos/git/django/django/db/models/lookups.py", line 94, in process_rhs
    return self.get_db_prep_lookup(value, connection)
  File "/home/koniiiik/repos/git/django/django/db/models/lookups.py", line 186, in get_db_prep_lookup
    [get_db_prep_value(value, connection, prepared=True)]
  File "/home/koniiiik/repos/git/django/django/db/models/fields/related.py", line 942, in get_db_prep_value
    return self.target_field.get_db_prep_value(value, connection, prepared)
  File "/home/koniiiik/repos/git/django/django/db/models/fields/__init__.py", line 1429, in get_db_prep_value
    return connection.ops.adapt_datetimefield_value(value)
  File "/home/koniiiik/repos/git/django/django/db/backends/sqlite3/operations.py", line 218, in adapt_datetimefield_value
    if timezone.is_aware(value):
  File "/home/koniiiik/repos/git/django/django/utils/timezone.py", line 248, in is_aware
    return value.utcoffset() is not None
AttributeError: 'int' object has no attribute 'utcoffset'

----------------------------------------------------------------------

The problem is that when compiling a RelatedExact node to SQL, the rhs is processed with get_db_prep_value of the ForeignKey field, which defers to the target field, which in this case is the primary key of the local model. However, the correct field to process the value would be the remote primary key field.

FWIW, the lhs gets compiled to the correct SQL expression, "reverse_fk_test_case_remote"."id". The difference appears to be that when compiling the lhs, the Col's target attribute is used, which is a reference to the primary key of the remote model, but when processing the rhs, the output_field attribute of the lhs is used instead, which is a reference to the ForeignKey field.

Unfortunately, I don't quite understand all of the logic here, but it seems to me that whenever the lhs is a Col instance, the correct field to use would be target, and in other cases, i.e. results of more complex expressions, it would be output_field. I'm just guessing here, though, but at least a quick run through the test suite with a modified django.db.models.lookups.FieldGetDbPrepValueMixin seems to agree, though I don't really find that to be sufficient evidence. At the same time, I don't really get why output_field and target reference different fields in this case; could be that that is the root cause.

[Mariusz Felisiak]

Thanks for this report. I was able to reproduce this issue on all backends except PostgreSQL. It looks that using foreign keys to models with custom primary keys that require preparing DB value is not a common use case.

Reproduced at 7619a33665ccc138c1583a2cce5a5de6d86c9cb4.

[Michal Petrucha]

FWIW, the way I discovered this was with a UUID primary key with a Postgres backend, which might not be that unusual, but in Django 1.11. However, I had to reproduce it with a different field on master, because UUIDField.to_python has become a bit less strict since 1.11, and it doesn't error out on integers anymore.

[Can Sarıgöl]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 325d5d64:

Fixed #30477 -- Made reverse lookup use Field.get_db_prep_value() from the target field.