[Django] #23270: select_related on fields pointing to subclasses does not work when using defer
Django
defer
----------------------------------------------+----------------------------
Reporter: islavov | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.7-rc-2
Severity: Normal | Keywords: select_related
Triage Stage: Unreviewed | defer
Easy pickings: 0 | Has patch: 0
| UI/UX: 0
----------------------------------------------+----------------------------
I have the following models
{{{
#!python
from django.db import models
class Base(models.Model):
text = models.TextField()
class SubClassA(Base):
name = models.CharField(max_length=32)
}}}
In shell
{{{
#!python
In [3]: from defertest.models import SubClassA, Base
# Select related + defer
In [4]: c=Base.objects.select_related("subclassa").defer("text")[0]
(0.001) SELECT "defertest_base"."id", "defertest_subclassa"."base_ptr_id",
"defertest_subclassa"."name" FROM "defertest_base" LEFT OUTER JOIN
"defertest_subclassa" ON ("defertest_base"."id" =
"defertest_subclassa"."base_ptr_id") LIMIT 1; args=()
# We get an extra query when accessing the subclass
In [5]: c.subclassa
(0.000) SELECT "defertest_base"."id", "defertest_base"."text",
"defertest_subclassa"."base_ptr_id", "defertest_subclassa"."name" FROM
"defertest_subclassa" INNER JOIN "defertest_base" ON
("defertest_subclassa"."base_ptr_id" = "defertest_base"."id") WHERE
"defertest_subclassa"."base_ptr_id" = 1 ; args=(1,)
Out[5]: <SubClassA: SubClassA object>
# If no deferred fields
In [6]: c=Base.objects.select_related("subclassa")[0]
(0.000) SELECT "defertest_base"."id", "defertest_base"."text",
"defertest_subclassa"."base_ptr_id", "defertest_subclassa"."name" FROM
"defertest_base" LEFT OUTER JOIN "defertest_subclassa" ON
("defertest_base"."id" = "defertest_subclassa"."base_ptr_id") LIMIT 1;
args=()
# select related works fine ( no extra query )
In [7]: c.subclassa
Out[7]: <SubClassA: SubClassA object>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23270>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
defer
Reporter: islavov | Owner: nobody
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 1
defer | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by jarshwah):
* needs_docs: => 1
* stage: Unreviewed => Accepted
* needs_tests: => 1
* needs_better_patch: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:1>
Django
defer
Reporter: islavov | Owner: nobody
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 1
Has patch: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by jarshwah):
Verified that there is an extra query. Tried the patch from #23001
thinking it may be related, but same problem there.
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:2>
Django
defer
Reporter: islavov | Owner: nobody
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 1
Has patch: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Vladimiroff):
The reason behind this is that `subclassa` (in the given example) is not
technically a **field**. Which means it won't be returned by
`Base._meta.get_concrete_fields_with_model`. The only solution I could
think of is creating `get_all_concrete_fields_with_model` (which should
use `get_all_field_names` instead of `local_fields`).
Anyone with better ideas?
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:3>
Django
defer
Reporter: islavov | Owner: nobody
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 1
Has patch: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Vladimiroff):
Ok, my idea was completely wrong. I had to dig a little bit deeper than I
thought.
https://github.com/django/django/pull/3067
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:4>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-2
(models, ORM) | Resolution:
Keywords: select_related | checkin
defer | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_docs: 1 => 0
* has_patch: 0 => 1
* type: Uncategorized => Bug
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
Ready for review by ORM experts.
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:5>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-2
(models, ORM) | Resolution:
defer | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
The patch doesn't seem to work correctly. The obj.derived.base_ptr_id
doesn't have correct value of 1 in the test case.
{{{
>>> obj.derived.base_ptr_id
'bar'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:6>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-2
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
defer | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Vladimiroff):
@akaariai, you're right. I was building the field_names wrong. The issue
was not only that `base_ptr_id` was with the wrong value, but *all* of the
values was getting mapped to the wrong field.
The only solution that I found was completely rebuilding the whole list
from scratch. Was that a good idea?
https://github.com/Vladimiroff/django/commit/cfd87b8157bf38ce145758e0251f5c14dfb5e60f
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:7>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-2
(models, ORM) | Resolution:
Keywords: select_related | checkin
defer | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
Ready for another review.
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:8>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-2
(models, ORM) | Resolution:
Keywords: select_related | Needs documentation: 0
defer | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
See PR for comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:9>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-2
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_better_patch: 1 => 0
Comment:
Patch received another update.
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:10>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-2
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Marking as "patch needs improvement" per latest comment from Anssi on the
PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:11>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-2
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
defer | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
Pull request https://github.com/django/django/pull/3669 fixes this issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:12>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: select_related | Needs documentation: 0
defer | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* version: 1.7-rc-2 => 1.7
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:13>
Django
defer
Reporter: islavov | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.7
(models, ORM) |
Keywords: select_related | Triage Stage: Accepted
defer |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/23270#comment:14>