[Django] #30477: Filtering on reverse ForeignKey relation uses get_db_prep_value from a wrong field
Django
wrong field
-------------------------------------+-------------------------------------
Reporter: Michal | Owner: nobody
Petrucha |
Type: Bug | Status: new
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30477>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Michal Petrucha | Owner: nobody
Type: Bug | Status: new
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
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30477#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Petrucha | Owner: nobody
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30477#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Petrucha | Owner: Sergey
| Golovchenko
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Sergey Golovchenko
--
Ticket URL: <https://code.djangoproject.com/ticket/30477#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Petrucha | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => new
* owner: Sergey Golovchenko => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/30477#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Petrucha | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Can Sarıgöl
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/11510 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30477#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Petrucha | Owner: Can
| Sarıgöl
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"325d5d64452118d89624d8b4c8679f0d2cb3d6d6" 325d5d64]:
{{{
#!CommitTicketReference repository=""
revision="325d5d64452118d89624d8b4c8679f0d2cb3d6d6"
Fixed #30477 -- Made reverse lookup use Field.get_db_prep_value() from the
target field.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30477#comment:4>