Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#12851 closed (fixed)

Queryset "defer" won't work properly with select_related

Reported by: ruosteinen Owned by: ruosteinen
Component: Database layer (models, ORM) Version: 1.2-beta
Severity: Keywords: select_related, defer, only, nesting, query
Cc: Carl Meyer, ruosteinen, Gregor Müllegger Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Select_related and defer don't seem to work properly together. Consider following models in APP buggy.

from django.db import models

class ProblemModel(models.Model):
    name = models.CharField(max_length=100)
    target = models.ForeignKey('Target', null=True)

class Target(models.Model):
    location = models.CharField(max_length=100)

Even though Queryset API document the use of underscore in deferring, the following ends up in assertion error.

from buggy.models import Target, ProblemModel
Target.objects.all().delete(); ProblemModel.objects.all().delete()

tgt = Target.objects.create(location='Earth')
pm = ProblemModel.objects.create(name='bug', target=tgt)

pm = ProblemModel.objects.select_related('target').defer('target__location').get(name='bug')

assert pm.target.location == 'bug'


At least according to Queryset API docs, defer() is supposed to allow access to deferred fields even if they're not loaded initially.

In addition pm.target is not of type Target_Deferred_location_deferred as it should be(?). There seem to be some other weirdness as well with select_related and defer but they'll probably be fixed once nesting works properly.

This in Django 1.2 beta 1 SVN-12416

I'll start working on a patch right away.

-Tommi Penttinen-

Attachments (2)

12851_inheritance_regression_test.patch (1.0 KB) - added by Gregor Müllegger 6 years ago.
The current fix for this bug does not work if you use model inheritance. Here is a failing testcase.
12851_inheritance_regression_test_2.patch (550 bytes) - added by Gregor Müllegger 6 years ago.
Here is another testcase using "only" instead of "defer" which fails with the current trunk.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 7 years ago by ruosteinen

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Seems to be related to #10733.

comment:2 Changed 7 years ago by Carl Meyer

Cc: Carl Meyer added

comment:3 Changed 7 years ago by Carl Meyer

I haven't played with this yet, but your repro code doesn't look right. I would expect an assertion error with that code: pm.target.location should be 'Earth' not 'bug'.

comment:4 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2
Triage Stage: UnreviewedAccepted

@carljm - Agreed: "Earth" should be the expected result. Either way, it's not working - pm.target.location is returning an empty string.

@ruosteinen - Although #10733 also deals with select_related() + defer()/only() problems, I'm not immediately convinced that it is the same problem (at least, not enough to close this as a duplicate).#10733 deals with having multiple joins to the same table; this ticket is about traversing a join correctly. I suspect the problem here is something to do with populating the related object cache correctly - the query is doing the join as it should, but because the query defers the only data field on the related object, and it is being deferred, the deferred class isn't being instantiated correctly.

comment:5 Changed 7 years ago by ruosteinen

@carljm, sorry. Changed the code a bit on the fly. But as russellm said, it still doesn't work.

@russellm True. Although in the proposed patch to #10733 only_load=only_load get passed properly to django.db.models.query.get_cached_row while recursing and after that relationships with deferred fields work properly i.e. type(pm.target) == <class 'buggy.models.Target_Deferred_location'>.

comment:6 Changed 7 years ago by ruosteinen

Cc: ruosteinen added
Has patch: set
Owner: changed from nobody to ruosteinen
Status: newassigned

I'll mark this as "has patch" since one is available at #10733.

comment:7 Changed 7 years ago by Russell Keith-Magee

Resolution: fixed
Status: assignedclosed

(In [12817]) Fixed #12851 -- Corrected the interaction of defer() with select_related(). Thanks to ruosteinen for the report.

comment:8 Changed 7 years ago by Russell Keith-Magee

(In [12818]) [1.1.X] Fixed #12851 -- Corrected the interaction of defer() with select_related(). Thanks to ruosteinen for the report.

Backport of r12817 from trunk.

Changed 6 years ago by Gregor Müllegger

The current fix for this bug does not work if you use model inheritance. Here is a failing testcase.

comment:9 Changed 6 years ago by Gregor Müllegger

Cc: Gregor Müllegger added
Keywords: only added
Resolution: fixed
Status: closedreopened

Unfortunatelly this bug is still valid in trunk if you use model inheritance.

My history with this bug was the following:

  • Discovered it with 1.2-beta-1
  • Saw that it was fixed in trunk and updated (and worked)
  • Changed my models to use inheritance which broke things again.

I added a testcase which fails for me with the following backtrace:

Failed example:
    markus.value
Exception raised:
    Traceback (most recent call last):
      File "C:\Python26\lib\site-packages\django\test\_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest issue12851.models.__test__.API_TESTS[45]>", line 1, in <module>
        markus.value
      File "C:\Python26\lib\site-packages\django\db\models\query_utils.py", line 208, in __get__
        cls._base_manager.filter(pk=instance.pk).only(name).using(
      File "C:\Python26\lib\site-packages\django\db\models\manager.py", line 141, in filter
        return self.get_query_set().filter(*args, **kwargs)
      File "C:\Python26\lib\site-packages\django\db\models\query.py", line 550, in filter
        return self._filter_or_exclude(False, *args, **kwargs)
      File "C:\Python26\lib\site-packages\django\db\models\query.py", line 568, in _filter_or_exclude
        clone.query.add_q(Q(*args, **kwargs))
      File "C:\Python26\lib\site-packages\django\db\models\sql\query.py", line 1131, in add_q
        can_reuse=used_aliases)
      File "C:\Python26\lib\site-packages\django\db\models\sql\query.py", line 1071, in add_filter
        connector)
      File "C:\Python26\lib\site-packages\django\db\models\sql\where.py", line 66, in add
        value = obj.prepare(lookup_type, value)
      File "C:\Python26\lib\site-packages\django\db\models\sql\where.py", line 299, in prepare
        return self.field.get_prep_lookup(lookup_type, value)
      File "C:\Python26\lib\site-packages\django\db\models\fields\related.py", line 134, in get_prep_lookup
        return self._pk_trace(value, 'get_prep_lookup', lookup_type)
      File "C:\Python26\lib\site-packages\django\db\models\fields\related.py", line 196, in _pk_trace
        v = getattr(field, prep_func)(lookup_type, v, **kwargs)
      File "C:\Python26\lib\site-packages\django\db\models\fields\__init__.py", line 292, in get_prep_lookup
        return self.get_prep_value(value)
      File "C:\Python26\lib\site-packages\django\db\models\fields\__init__.py", line 476, in get_prep_value
        return int(value)
    ValueError: invalid literal for int() with base 10: 'Markus McDonalds'

IMO the SQL query is correct and retrieves the correct fields from the database, but the fields are populated in the wrong way.

comment:10 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: reopenedclosed

(In [13055]) [1.1.X] Fixed #12851 -- Corrected the loading of values when select_related() is used on inherited models. Thanks to phxx for the report and test case.

Backport of r13054 from trunk.

Changed 6 years ago by Gregor Müllegger

Here is another testcase using "only" instead of "defer" which fails with the current trunk.

comment:11 Changed 6 years ago by Gregor Müllegger

Resolution: fixed
Status: closedreopened

I'm very sorry for reopening the ticket *again*.

But the issue is still there if you use the "only" method instead of "defer". I've attached a new testcase which fails with the current trunk.

Here is the traceback:

Failed example:
    troy.state.name
Exception raised:
    Traceback (most recent call last):
      File "C:\Python26\lib\site-packages\django\test\_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest issue12851.models.__test__.API_TESTS[56]>", line 1, in <module>
        troy.state.name
      File "C:\Python26\lib\site-packages\django\db\models\query_utils.py", line 208, in __get__
        cls._base_manager.filter(pk=instance.pk).only(name).using(
      File "C:\Python26\lib\site-packages\django\db\models\manager.py", line 141, in filter
        return self.get_query_set().filter(*args, **kwargs)
      File "C:\Python26\lib\site-packages\django\db\models\query.py", line 550, in filter
        return self._filter_or_exclude(False, *args, **kwargs)
      File "C:\Python26\lib\site-packages\django\db\models\query.py", line 568, in _filter_or_exclude
        clone.query.add_q(Q(*args, **kwargs))
      File "C:\Python26\lib\site-packages\django\db\models\sql\query.py", line 1131, in add_q
        can_reuse=used_aliases)
      File "C:\Python26\lib\site-packages\django\db\models\sql\query.py", line 1071, in add_filter
        connector)
      File "C:\Python26\lib\site-packages\django\db\models\sql\where.py", line 66, in add
        value = obj.prepare(lookup_type, value)
      File "C:\Python26\lib\site-packages\django\db\models\sql\where.py", line 299, in prepare
        return self.field.get_prep_lookup(lookup_type, value)
      File "C:\Python26\lib\site-packages\django\db\models\fields\related.py", line 134, in get_prep_lookup
        return self._pk_trace(value, 'get_prep_lookup', lookup_type)
      File "C:\Python26\lib\site-packages\django\db\models\fields\related.py", line 196, in _pk_trace
        v = getattr(field, prep_func)(lookup_type, v, **kwargs)
      File "C:\Python26\lib\site-packages\django\db\models\fields\__init__.py", line 292, in get_prep_lookup
        return self.get_prep_value(value)
      File "C:\Python26\lib\site-packages\django\db\models\fields\__init__.py", line 476, in get_prep_value
        return int(value)
    ValueError: invalid literal for int() with base 10: 'Troy Buswell'

comment:12 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: reopenedclosed

(In [13059]) Fixed #12851 -- Another attempt at fixing select_related() with inherited models, this time with only(). Thanks to phxx for the test case.

comment:13 Changed 6 years ago by Russell Keith-Magee

(In [13060]) [1.1.X] Fixed #12851 -- Another attempt at fixing select_related() with inherited models, this time with only(). Thanks to phxx for the test case.

Backport of r13059 from trunk.

comment:14 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top