Opened 3 years ago

Last modified 12 months ago

#24612 new Cleanup/optimization

Confusing error message when using only/defer through deleted related field

Reported by: Richard Eames Owned by:
Component: Database layer (models, ORM) Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

# 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="+"

Change History (10)

comment:1 Changed 3 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:2 Changed 3 years ago by lukawoj

Owner: changed from nobody to lukawoj
Status: newassigned

comment:3 Changed 3 years ago by lukawoj

I'm starting work on this.

comment:4 Changed 3 years ago 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'

comment:5 Changed 3 years ago by lukawoj

Owner: lukawoj deleted
Status: assignednew

comment:6 Changed 3 years ago by Richard Eames

@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.

comment:7 Changed 12 months ago by Eduardo

Owner: set to Eduardo
Status: newassigned

comment:8 Changed 12 months ago 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)

comment:9 Changed 12 months ago 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.

comment:10 Changed 12 months ago by Eduardo

Owner: Eduardo deleted
Status: assignednew
Note: See TracTickets for help on using tickets.
Back to Top