[Django] #24612: Confusing error message when using only/defer through deleted related field
Django
----------------------------------------------+--------------------
Reporter: Naddiseo | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Database layer (models, ORM) | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
If one tries to use `defer`/`only` for a field in a model that has the
reverse relationship explicitly removed, the resulting error message is
confusing.
{{{
#!python
# models.py
from django.db import models
from django.contrib.auth.models import User
class Message(models.Model):
content = models.CharField(max_length=10)
class MessageParticipant(models.Model):
msg = models.ForeignKey(Message, related_name='participants')
user = models.ForeignKey(User, related_name='+')
}}}
Query to reproduce:
{{{
list(Message.objects.all().only('participants__user__username')
}}}
Error:
{{{
Traceback (most recent call last):
File "/usr/lib/python3.4/unittest/loader.py", line 312, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python3.4/unittest/loader.py", line 290, in
_get_module_from_name
__import__(name)
File "~/tmp/sr/test.py", line 11, in <module>
print(q.query)
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/query.py", line 213, in __str__
sql, params = self.sql_with_params()
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/query.py", line 221, in sql_with_params
return self.get_compiler(DEFAULT_DB_ALIAS).as_sql()
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 367, in as_sql
extra_select, order_by, group_by = self.pre_sql_setup()
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 48, in pre_sql_setup
self.setup_query()
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 39, in setup_query
self.select, self.klass_info, self.annotation_col_map =
self.get_select()
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 184, in get_select
for c in self.get_default_columns():
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 487, in
get_default_columns
only_load = self.deferred_to_columns()
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/compiler.py", line 755, in
deferred_to_columns
self.query.deferred_to_data(columns,
self.query.get_loaded_field_names_cb)
File "~/.virtualenvs/dj18_sr/lib/python3.4/site-
packages/django/db/models/sql/query.py", line 622, in deferred_to_data
cur_model = source.rel.to
AttributeError: 'ManyToOneRel' object has no attribute 'rel'
}}}
Even though the error makes sense once you know why it's happening, I
think it would be more helpful to the programmer if the message stated
that one cannot use `only`/`defer` on a table/field that has had the
reverse relationship removed via `related_name="+"`
--
Ticket URL: <https://code.djangoproject.com/ticket/24612>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Naddiseo | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(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
Changes (by timgraham):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:1>
Django
Reporter: Naddiseo | Owner: lukawoj
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by lukawoj):
* owner: nobody => lukawoj
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:2>
Django
Reporter: Naddiseo | Owner: lukawoj
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by lukawoj):
I'm starting work on this.
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:3>
Django
Reporter: Naddiseo | Owner: lukawoj
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by lukawoj):
On version 1.9.dev20150602231717 I get this exception:
{{{
Traceback (most recent call last):
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\tests\defer\tests.py",
line 280, in test_defer_only
list(Message.objects.all().only('participants__user__username'))
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\query.py",
line 258, in __iter__
self._fetch_all()
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\query.py",
line 1063, in _fetch_all
self._result_cache = list(self.iterator())
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\query.py",
line 52, in __iter__
results = compiler.execute_sql()
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\compiler.py",
line 835, in execute_sql
sql, params = self.as_sql()
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\compiler.py",
line 384, in as_sql
extra_select, order_by, group_by = self.pre_sql_setup()
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\compiler.py",
line 48, in pre_sql_setup
self.setup_query()
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\compiler.py",
line 39, in setup_query
self.select, self.klass_info, self.annotation_col_map =
self.get_select()
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\compiler.py",
line 196, in get_select
for c in self.get_default_columns():
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\compiler.py",
line 504, in get_default_columns
only_load = self.deferred_to_columns()
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\compiler.py",
line 772, in deferred_to_columns
self.query.deferred_to_data(columns,
self.query.get_loaded_field_names_cb)
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\query.py",
line 684, in deferred_to_data
callback(target, model, values)
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\query.py",
line 1804, in get_loaded_field_names_cb
target[model] = {f.attname for f in fields}
File
"C:\dev\django_con\django_sprint_2015\django_sprint\django\django\db\models\sql\query.py",
line 1804, in <setcomp>
target[model] = {f.attname for f in fields}
AttributeError: 'ManyToOneRel' object has no attribute 'attname'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:4>
Django
Reporter: Naddiseo | Owner:
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by lukawoj):
* owner: lukawoj =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:5>
Django
Reporter: Naddiseo | Owner:
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Naddiseo):
@lukawoj, exactly. The error message is still confusing since it doesn't
tell you why it occurred. Something more useful would be: For model '%s',
`defer`/`only` cannot be used with deleted reverse relationships.
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:6>
Django
Reporter: Richard Eames | Owner: Eduardo
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Eduardo):
* status: new => assigned
* owner: (none) => Eduardo
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:7>
Django
Reporter: Richard Eames | Owner: Eduardo
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Eduardo):
It's ok to change /django/db/models/sql/query.py?
I added a try/except, which kind of exception we need to raise?
{{{
try:
target[model] = {f.attname for f in fields}
except AttributeError:
msg = "For model '%s' defer/only cannot be used with deleted
reverse relationships." % (model)
raise FieldError(msg)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:8>
Django
Reporter: Richard Eames | Owner: Eduardo
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
It's difficult to say if a deleted reverse relation is the only way to hit
an error there. Something along the lines of `"Cannot resolve keyword %r
into field. Choices are: %s"` might be more generic. Is there any other
validation for only/deferred fields? Might be interesting to see if this
case could be covered there.
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:9>
Django
Reporter: Richard Eames | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Eduardo):
* status: assigned => new
* owner: Eduardo => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:10>
Django
Reporter: no | Owner: (none)
Type: | Status: closed
Component: Database layer | Version: 1.8
Severity: Normal | worksforme
Has patch: 0 | Needs documentation: 0
Changes (by JaeHyuckSa):
* resolution: => worksforme
* status: new => closed
Comment:
I rechecked this on the current main branch and can no longer reproduce
the issue.
Using the exact model setup from the ticket, the following query now works
as expected:
{{{
#!python
list(Publisher.objects.only("books__author__name"))
}}}
The previously reported
{{{
AttributeError: 'ManyToOneRel' object has no attribute 'attname'
}}}
no longer occurs.
Looking at the code, this seems to have been fixed as a side effect of the
changes in #21204, which rewrote the only() / defer() machinery.
The old deferred_to_data() logic only treated reverse OneToOne relations
specially, while reverse ForeignKey relations (ManyToOneRel) still went
through remote_field.model. That logic was replaced entirely in #21204, so
the original failure mode doesn’t appear to be reachable anymore.
--
Ticket URL: <https://code.djangoproject.com/ticket/24612#comment:11>