[Django] #17143: select_related makes base __init__ unsafe
Django
----------------------------------------------+--------------------
Reporter: Leo | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: SVN
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
For the following models setup:
{{{#!python
class A(models.Model):
foo = models.CharField(max_length=1, default='N')
def __init__(self, *args, **kwargs):
super(A, self).__init__(*args, **kwargs)
print self.foo
class B(A):
bar = models.CharField(max_length=1, default='X')
}}}
The `print` in in class `A` is just a proxy for doing something else with
the field. My specific use case is that I'm trying to save the field off
so I can detect whether it has been changed on save or not - but that
doesn't impact the bug.
{{{#!python
>>> b = B(foo='Y', bar='Z')
Y
>>> b.save()
>>> A.objects.select_related('b')
Y
N
[<A: A object>]
}}}
The second call to `A.__init__` is to create the B object, however, it
doesn't have all of the fields from the original A so while in
`A.__init__`, `self.foo` evaluates to the wrong value.
This is either something broken with what fields `select_related` loads on
related objects, or it needs to be documented as a really really nasty
gotcha for inherited classes/`select_related`.
--
Ticket URL: <https://code.djangoproject.com/ticket/17143>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Leo | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by aaugustin):
* needs_better_patch: => 0
* component: Database layer (models, ORM) => Documentation
* needs_tests: => 0
* needs_docs: => 0
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
To the best of my knowledge, that's the expected behavior. I agree the
docs could be improved. Basically, `B` carries only the `bar` field and a
one-to-one relation to `A`.
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:1>
Django
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by carljm):
* type: Cleanup/optimization => Bug
* component: Documentation => Database layer (models, ORM)
Comment:
I'm not sure I agree, aaugustin. I got further clarification from Leo that
this only happens with select_related, not when selecting directly on Bs.
IMO that indicates that it is a bug, and might be fixable. I might be
wrong; would have to sit down and look at it in more detail to see what a
fix would entail.
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:2>
Django
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by aaugustin):
Indeed, if the problem happens with `select_related` but not elsewhere, it
could be a bug. This needs a test case, for instance with an assertion on
the arguments received in `__init__`.
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:3>
Django
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by calvinspealman):
I broke this down into a few test conditions and found the report to be
accurate. Only through select_related is the default value used, where in
all other instantiations of the B the correct value is found.
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:4>
Django
Reporter: Leo | Owner:
Type: Bug | calvinspealman
Component: Database layer | Status: assigned
(models, ORM) | Version: SVN
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Changes (by calvinspealman):
* owner: nobody => calvinspealman
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:5>
Django
Reporter: Leo | Owner:
Type: Bug | calvinspealman
Component: Database layer | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by calvinspealman):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:6>
Django
Reporter: Leo | Owner:
Type: Bug | calvinspealman
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
Comment:
I'm not sure if this is still an issue, but the test in `17143.diff`
passes as far back as I tested (stable/1.3.x) so it doesn't appear to be a
proper regression test for the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:7>
Django
Reporter: Leo | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: assigned => new
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
* owner: calvinspealman =>
Comment:
Based on the test case in the description, it looks like this was fixed in
Django 1.6 by 1194a9699932088385f9f88869be28a251597f45. The result is now:
{{{
>>> A.objects.select_related('b')
Y
Y
}}}
However, no tests were added in that patch, so it seems a good idea to try
to add some before closing this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:8>
Django
Reporter: Leo | Owner:
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => fixed
Comment:
Actually the tests were added in f51e409a5fb34020e170494320a421503689aea0
-- some didn't pass until the follow up commit.
--
Ticket URL: <https://code.djangoproject.com/ticket/17143#comment:9>