#14694 closed Bug (fixed)
defer() doesn't work with reverse relations
Reported by: | Sayane | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | defer only OneToOneField reverse relationship |
Cc: | Sayane, anssi.kaariainen@…, real.human@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
It's impossible to defer REVERSED relation. I'm not able to reproduce whole queryset, because it's generated automatically.
Traceback: File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/core/handlers/base.py" in get_response 109. response = callback(request, *callback_args, **callback_kwargs) File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/views/generic/base.py" in view 52. return self.dispatch(request, *args, **kwargs) File "/home/sayane/Programowanie/qcr2/unit/../qcr/core/ajax/views.py" in dispatch 55. resp = super(AjaxView, self).dispatch(request, *args, **kwargs) File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/views/generic/base.py" in dispatch 73. return handler(request, *args, **kwargs) File "/home/sayane/Programowanie/qcr2/unit/../qcr/core/ajax/store.py" in post 101. total = self.queryset.count() File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/query.py" in count 327. return self.query.get_count(using=self.db) File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/sql/query.py" in get_count 391. obj.add_subquery(subquery, using=using) File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/sql/subqueries.py" in add_subquery 222. self.subquery, self.sub_params = query.get_compiler(using).as_sql(with_col_aliases=True) File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/sql/compiler.py" in as_sql 58. self.pre_sql_setup() File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/sql/compiler.py" in pre_sql_setup 29. self.fill_related_selections() File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/sql/compiler.py" in fill_related_selections 578. opts=f.rel.to._meta, as_pairs=True) File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/sql/compiler.py" in get_default_columns 237. only_load = self.deferred_to_columns() File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/sql/compiler.py" in deferred_to_columns 666. self.query.deferred_to_data(columns, self.query.deferred_to_columns_cb) File "/home/sayane/Programowanie/virtenv/dj13/lib/python2.6/site-packages/django/db/models/sql/query.py" in deferred_to_data 540. cur_model = opts.get_field_by_name(name)[0].rel.to Exception Type: AttributeError at /a/case/stores/case/ Exception Value: 'RelatedObject' object has no attribute 'rel'
When trying with simple query, it doesn't return any results:
In [7]: Case.objects.select_related('info') Out[7]: [<Case: test/01>] In [8]: Case.objects.select_related('info').defer('info__requirements') Out[8]: [] In [9]: Case.objects.select_related('info').defer('info__requirements').count() Out[9]: 1
Attachments (4)
Change History (25)
comment:1 by , 14 years ago
milestone: | 1.3 |
---|---|
Resolution: | → invalid |
Status: | new → closed |
comment:2 by , 13 years ago
Easy pickings: | unset |
---|---|
Resolution: | invalid |
Severity: | → Normal |
Status: | closed → reopened |
Type: | → Bug |
UI/UX: | unset |
Version: | SVN → 1.3 |
I can provide some example code, I've run into the exact same problem. It's actually a bit more complicated than that.
Here are some models:
class Paper(models.Model): title = models.CharField(max_length=200) abstract = models.TextField() def __unicode__(self): return self.title class Presentation(models.Model): starts = models.DateTimeField() ends = models.DateTimeField() notes = models.TextField() paper = models.OneToOneField("Paper", related_name="presentation")
Then, in the shell:
>>> from papers.models import * >>> for p in range(0,83): ... paper = Paper.objects.create(title="Sample Title %d" % p, abstract="This is paper %d" % p) ... if p / 6 == 0: ... presentation = Presentation.objects.create(starts='2011-01-01 00:00:00', ends='2011-01-01 01:00:00', paper=paper, notes="This is the presentation for %s" % paper) ... >>> qs_no_related = Paper.objects.all().defer('abstract') >>> qs_no_related [<Paper_Deferred_abstract: Sample Title 0>, <Paper_Deferred_abstract: Sample Title 1>, <Paper_Deferred_abstract: Sample Title 2>, <Paper_Deferred_abstract: Sample Title 3>, <Paper_Deferred_abstract: Sample Title 4>, <Paper_Deferred_abstract: Sample Title 5>, <Paper_Deferred_abstract: Sample Title 6>, <Paper_Deferred_abstract: Sample Title 7>, <Paper_Deferred_abstract: Sample Title 8>, <Paper_Deferred_abstract: Sample Title 9>, <Paper_Deferred_abstract: Sample Title 10>, <Paper_Deferred_abstract: Sample Title 11>, <Paper_Deferred_abstract: Sample Title 12>, <Paper_Deferred_abstract: Sample Title 13>, <Paper_Deferred_abstract: Sample Title 14>, <Paper_Deferred_abstract: Sample Title 15>, <Paper_Deferred_abstract: Sample Title 16>, <Paper_Deferred_abstract: Sample Title 17>, <Paper_Deferred_abstract: Sample Title 18>, <Paper_Deferred_abstract: Sample Title 19>, '...(remaining elements truncated)...'] >>> print qs_no_related.query SELECT "papers_paper"."id", "papers_paper"."title" FROM "papers_paper" >>> len(qs_no_related) 83 >>> qs_no_defer = Paper.objects.all().select_related("presentation__pk") >>> qs_no_defer [<Paper: Sample Title 0>, <Paper: Sample Title 1>, <Paper: Sample Title 2>, <Paper: Sample Title 3>, <Paper: Sample Title 4>, <Paper: Sample Title 5>, <Paper: Sample Title 6>, <Paper: Sample Title 7>, <Paper: Sample Title 8>, <Paper: Sample Title 9>, <Paper: Sample Title 10>, <Paper: Sample Title 11>, <Paper: Sample Title 12>, <Paper: Sample Title 13>, <Paper: Sample Title 14>, <Paper: Sample Title 15>, <Paper: Sample Title 16>, <Paper: Sample Title 17>, <Paper: Sample Title 18>, <Paper: Sample Title 19>, '...(remaining elements truncated)...'] >>> print qs_no_defer.query SELECT "papers_paper"."id", "papers_paper"."title", "papers_paper"."abstract", "papers_presentation"."id", "papers_presentation"."starts", "papers_presentation"."ends", "papers_presentation"."notes", "papers_presentation"."paper_id" FROM "papers_paper" LEFT OUTER JOIN "papers_presentation" ON ("papers_paper"."id" = "papers_presentation"."paper_id") >>> len(qs_no_defer) 83 >>> qs_related_defer=Paper.objects.all().defer('abstract','presentation__notes').select_related("presentation__pk") >>> qs_related_defer [] >>> print qs_related_defer.query Traceback (most recent call last): File "<console>", line 1, in <module> File "/path/to/django/db/models/sql/query.py", line 162, in __str__ sql, params = self.get_compiler(DEFAULT_DB_ALIAS).as_sql() File "/path/to/django/db/models/sql/compiler.py", line 58, in as_sql self.pre_sql_setup() File "/path/to/django/db/models/sql/compiler.py", line 29, in pre_sql_setup self.fill_related_selections() File "/path/to/django/db/models/sql/compiler.py", line 653, in fill_related_selections opts=model._meta, as_pairs=True, local_only=True) File "/path/to/django/db/models/sql/compiler.py", line 237, in get_default_columns only_load = self.deferred_to_columns() File "/path/to/django/db/models/sql/compiler.py", line 670, in deferred_to_columns self.query.deferred_to_data(columns, self.query.deferred_to_columns_cb) File "/path/to/django/db/models/sql/query.py", line 555, in deferred_to_data cur_model = opts.get_field_by_name(name)[0].rel.to AttributeError: 'RelatedObject' object has no attribute 'rel' >>> len(qs_related_defer) Traceback (most recent call last): File "<console>", line 1, in <module> File "/path/to/django/db/models/query.py", line 82, in __len__ self._result_cache = list(self.iterator()) File "/path/to/django/db/models/query.py", line 229, in iterator only_load = self.query.get_loaded_field_names() File "/path/to/django/db/models/sql/query.py", line 1761, in get_loaded_field_names self.deferred_to_data(collection, self.get_loaded_field_names_cb) File "/path/to/django/db/models/sql/query.py", line 555, in deferred_to_data cur_model = opts.get_field_by_name(name)[0].rel.to AttributeError: 'RelatedObject' object has no attribute 'rel' >>>
Note that the reason I was using defer
in this case is because I was also using a distinct()
which can't be used on Text fields (at least, not on the DB server I'm using). However, I found that this error occurs whether or not you are using distinct()
.
This is 100% repeatable in Django 1.3. The example shell output came from a project I created from scratch for the purpose of this ticket. It uses the SQLite backend so it is not specific to a backend either.
by , 13 years ago
Attachment: | ticket14694.tgz added |
---|
To save you time I've attached the sample app to recreate the error
comment:3 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 13 years ago
Has patch: | set |
---|---|
Keywords: | defer only OneToOneField reverse relationship added |
Version: | 1.3 → SVN |
I think this is a simple fix, and have attached a patch with tests. Using get_field_by_name()
with the name of a reverse relation returns a RelatedObject
, which has a .model
attribute, instead of rel.to
, for the target model.
comment:5 by , 13 years ago
Patch needs improvement: | set |
---|
On closer inspection, this fix doesn't fully resolve the problem. deferred_to_columns_cb()
still tries to access field.column
, which doesn't exist on RelatedObject
. I tried adding the actual field instead of the RelatedObject
to seen
and must_include
, but I still get problems. There are probably ORM complexities that I'm missing here. One thing that has probably helped this bug to slip under the radar is that exceptions raised in deferred_to_columns_cb()
and deferred_to_data()
appear to be silenced in some cases. Evaluating a queryset could return no results when .count()
shows that there are items in the set, but converting querysey.query
to str
raises an exception.
by , 13 years ago
Attachment: | 14694-defer-reverse-relations-r17008.2.diff added |
---|
comment:6 by , 13 years ago
Patch needs improvement: | unset |
---|
Updated the patch to avoid adding RelatedObject
fields (not actual DB fields), which fixes the error mentioned above (trying to access field.column
).
Also tracked down the cause of problems when using select_related()
with defer()
for reverse relations. Line 1400 of query.py
in get_cached_row()
was trying to get the value of foreign key fields on the related object by looking up the attname
of the foreign key field on the target object. Obviously this would never work. I'm not sure why it wasn't picked up until now. It seems to fail silently under normal circumstances, but the exception was raised only when running the test suite.
comment:7 by , 13 years ago
Patch needs improvement: | set |
---|
Still problems. I don't think my change to get_cached_row()
is correct, but I don't think what's there is correct, either. At least, not for objects with deferred fields.
For objects with deferred fields, it seems that NO fields are local and _meta.get_fields_with_model()
returns a model for what should be local fields.
This causes the rel_model is not None
condition to fail and all attributes on rel_obj
are set to the same value as the matching attribute on obj
.
For example: if Profile
has user = OneToOneField(User, reverse_name='profile')
, and you try User.objects.select_related('profile').defer('profile__nickname')
, then when get_cached_row()
is called, rel_obj
will be a Profile_Deferred_nickname
object with no local fields and Profile
as the model for all fields, and obj
will be a User
object. It will do setattr(rel_obj, rel_field.attname, getattr(obj, rel_field.attname))
for all fields including the auto PK field, which will end up setting the PK of your Profile to the same PK as your User. If Profile has any fields that don't exist, an exception is raised when running tests. The exception appears to be silenced and an empty queryset returned under normal circumstances.
comment:8 by , 13 years ago
I think I've finally cracked it. When working with model instances that have deferred fields, you need to call get_fields_with_model()
on the original model to get non-local fields correctly. You can access the opts for the original model with rel_obj._meta.proxy_for_model._opts
. I have just updated the patch. The full test suite passes, so I think this is RFC now, but I will wait for review by more a set of eyes more experienced with the ORM.
by , 13 years ago
Attachment: | 14694-defer-reverse-relations-r17462.diff added |
---|
comment:9 by , 13 years ago
Updated and re-tested against trunk.
Fixed (and tested) one other case where passing in the related_name
for a reverse OneToOneField
to .only()
(e.g. Item.objects.only('one_to_one_item')
) would result in a silenced exception (triggered by attempting to access field.column
when field
is a RelatedObject
) and an empty queryset when evaluated.
This problem wasn't obvious with .defer()
, or when referencing fields on a reverse related model with .only()
(e.g. Item.objects.only('one_to_one_item__name')
).
I think this is a clear bug, and the patch RFC. Just awaiting review by another set of eyes more familiar with the ORM :)
comment:10 by , 13 years ago
Cc: | added |
---|
I think this part could lead to some bugs:
if getattr(rel_obj, '_deferred'): opts = opts.proxy_for_model._meta
The reason is, proxy_for_model relates to the concrete model behind the proxy chain. So, if you have
class A: pass class ProxyA(A): proxy = True
And you query ProxyA.objects.defer('some_f'), now you will be using the options of A instead of ProxyA. I don't know if this will cause any bugs but it is wrong in any case.
There are many places in the code assuming that proxy_for_model generates a chain. However, this is not true. I think I have seen issues related to this on at least couple of tickets. Fixing this would be nice, but I haven't gotten much feedback from the community in the tickets where I have tried to solve this (#16128 for example).
There are some isinstance() calls added to query.py. Is it at all possible to return something consistent from get_field_by_name(), so that query.py would not need this complication?
comment:11 by , 12 years ago
I've updated this patch on a branch at GitHub to apply cleanly again.
https://github.com/thirstydigital/django/tree/tickets/14694-defer-reverse-relations
I don't think there is a problem with the opts = opts.proxy_for_model._meta
line that you questioned.
In the case of an automatic proxy model with deferred fields, we are intentionally accessing the opts from the concrete model in order to get and populate non-local fields correctly. Those fields don't exist on the automatic proxy model. We can only get them from the concrete model.
I still think this patch is RFC, and that this is a clear bug. Maybe you can improve the test cases to confirm your suspicions (or confirm that the patch is good)?
comment:12 by , 12 years ago
Cc: | added |
---|
comment:13 by , 12 years ago
Patch needs improvement: | unset |
---|
The proxy_for_model actually works correctly now, it was fixed some time ago to actually be the next model in the chain, and there is a new concrete_model opts parameter which is the concrete model in the end of the chain.
So, I am pretty sure that part of the patch is correct.
comment:15 by , 12 years ago
There is one other issue - I think the patch will allow silent .only()/.defer() calls to reverse foreign keys, while before that should have errored out in .iterator(). This is hard to see as the error is silenced and an empty list is returned in the erroneous case.
As in the patch, a .defer/.only call to reverse FK is silently discarded. At least I think that is the case...
The correct approach would be to do a check on the .only/.defer values when called, not when iterating the queryset. Another ticket's problem.
So, I do think the patch is RFC, although the logic can be a little bit complex to follow in the deferred_to_data.
I have one idea for making the hidden exceptions bug a little less annoying. I will see if I can make it work. If yes, it should make seeing the above error easier.
comment:16 by , 12 years ago
akaariai do you want to create another ticket before we close/commit this one so it doesn't get forgotten?
comment:17 by , 12 years ago
The idea about iterator ended up not working without altering the queryset iterator behavior. See #18702.
I will try to find some time to get this one committed.
comment:18 by , 12 years ago
Please forgive the lack of quality of this report (it's my first) but I want to comment that this issue affects me too. The following piece of code is where the exception 'RelatedObject' object has no attribute 'rel'
is raised.
values = [ o.caracteristicastelefono.resolucion_camara_trasera.get('total') for o in self.con_caracteristicas().\ only('caracteristicastelefono__resolucion_total_camara_trasera', 'caracteristicastelefono__resolucion_x_camara_trasera', 'caracteristicastelefono__resolucion_y_camara_trasera') ]
where models.py contains
class Telefono(mo.Model): # some irrelevant fields pass class CaracteristicasTelefono(mo.Model): telefono = mo.OneToOneField(Telefono) # # more fields # resolucion_total_camara_trasera = mo.FloatField('Resolución total (MP)', blank=True, null=True, help_text='(e.g. 1.5) Definir SÓLO si están vacías res. horizontal y vertical') resolucion_x_camara_trasera = mo.IntegerField('Resolución horizontal (px)', blank=True, null=True, help_text='(e.g. 1920)') resolucion_y_camara_trasera = mo.IntegerField('Resolución vertical (px)', blank=True, null=True, help_text='(e.g. 1080)') # # more fields and methods # @property def resolucion_camara_trasera(self): total = self.resolucion_total_camara_trasera x = self.resolucion_x_camara_trasera y = self.resolucion_y_camara_trasera return get_resolucion(total, x, y)
and managers.py contains
class TelefonoQuerySet(models.query.QuerySet): def con_caracteristicas(self): return self.exclude(caracteristicastelefono__exact=None) def activos(self): return self.filter(status=STATUS_TELEFONOS_ACTIVOS) class TelefonoManager(models.Manager): def get_query_set(self): model = models.get_model('telefonos', 'Telefono') return TelefonoQuerySet(model)
The class CaracteristicasTelefono
has lots of fields. Since I only need to get the result of resolucion_camara_trasera
and that method only uses 3 fields, it made sense to use Queryset
's only()
method.
I hope this info helps to diagnose/fix the problem. Thanks
PS: I'm using Django==1.4.1 / virtualenv==1.7.1.2 / Python 2.7.3 / Ubuntu 12.04 amd64
comment:19 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Closing invalid because of the complete lack of helpful information in narrowing down this problem. You've provided a stack trace... with no example code that generated it. You've provided three sample queries... without any details of the models that are involved.
Feel free to reopen if can provide a complete test case that would allow others to verify the bug without resorting to guesswork.