{{{#!python
@classmethod
def from_db(cls, db, field_names, values):
if len(values) != len(cls._meta.concrete_fields):
values_iter = iter(values)
values = [
next(values_iter) if f.attname in field_names else
DEFERRED
for f in cls._meta.concrete_fields
]
new = cls(*values)
new._state.adding = False
new._state.db = db
return new
}}}
This means that during the class initialization, the model instance state
is equal to `_state.adding = True` and `_state.db = None`. This can cause
troubles with related queries in the pre-/post-init methods. For example:
{{{#!python
class Parent(models.Model):
pass
class Child(models.Model):
parent = models.ForeignKey(Parent)
def foo(instance, **kwargs)
print(instance._state.adding)
print(instance._state.db)
# It could be that we initialize the model while it does not exist in
the
# database, so we check whether `instance.parent` exists first.
if getattr(instance, 'parent', None):
# Since we try to access instance.parent, it passes the `instance`
# as hint to the router, but `instance._state.db` is `None`, so it
defaults
# to the `DEFAULT_DB_ALIAS`.
assert instance._state.db == instance.parent._state.db
signals.post_init.connect(foo, sender=Child)
}}}
Trying to fetch an instance of `Child`, will now return:
{{{#!python
>>> child = Child.objects.using('second').get(pk=1)
True
None
AssertionError
}}}
The current behavior indicates that one should never do related queries in
the `pre_init` or `post_init` hooks, because it cannot be guaranteed that
the related queries are done on the same database you're retrieving the
instance from. Currently there's also no way of getting to know the on
which database the query was done for in the `pre_init` and `post_init`
hooks because the state is set after those signals are fired.
One related issue is #18532, which is only talking about the
`_state.adding` flag. There's a suggested solution over there, but I'm not
sure if that's the best way to go. I think there should be at least a note
in the Django documentation at the `pre_init` and `post_init` hooks saying
that related queries should be avoided, because it cannot be guaranteed
that they will be run on the same database you're getting the instance
from.
--
Ticket URL: <https://code.djangoproject.com/ticket/30083>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:1>
* status: new => assigned
* owner: nobody => Nasir Hussain
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:2>
* status: assigned => new
* owner: Nasir Hussain => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:3>
* owner: nobody => Nasir Hussain
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:2>
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:3>
* needs_better_patch: 0 => 1
Comment:
I left comments for improvements on the PR. After a read through of #18532
I'm still not convinced the addition in complexity is worth it. I never
add to use init signals though but performing queries in receivers seems
like a recipe for disaster.
The suggested implementation will also have the side effect of exposing
instances with a `._state` attribute in `pre_init` signal while it wasn't
the case before. If we wanted to maintain backward compatibility it could
be fired from `Model.__new__` through which is probably where it should
belong in the first place.
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:4>
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:5>
* status: assigned => closed
* resolution: => wontfix
Comment:
When we take into account performance doubts, `__new__()` tricky,
additional complexity, and that we can encourage bad practices to perform
queries in receivers of `pre_init` or `post_init` signals (hidden N+1
queries on every queryset iteration), I think we shouldn't fix this (see
also [https://code.djangoproject.com/ticket/18532#comment:5 Anssi's
comment] and [https://code.djangoproject.com/ticket/30083#comment:4
Simon's comment]). It's just not worth it. I'm going to add notes to the
signal documentation.
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:6>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"a2e1c17f193f5017e1f6fac7d860f1f9e34d7892" a2e1c17f]:
{{{
#!CommitTicketReference repository=""
revision="a2e1c17f193f5017e1f6fac7d860f1f9e34d7892"
Refs #30083 -- Clarified database state of instances in signals.pre_init
docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:7>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"fc1182af01c391ce33d7fcf51c756829c6a11d5b" fc1182af]:
{{{
#!CommitTicketReference repository=""
revision="fc1182af01c391ce33d7fcf51c756829c6a11d5b"
Refs #30083 -- Added a warning about performing queries in pre/post_init
receivers.
Thanks Carlton Gibson the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:8>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"506f800eadea2ab8c9d90f00278b4b03f3a9b771" 506f800e]:
{{{
#!CommitTicketReference repository=""
revision="506f800eadea2ab8c9d90f00278b4b03f3a9b771"
[2.2.x] Refs #30083 -- Added a warning about performing queries in
pre/post_init receivers.
Thanks Carlton Gibson the review.
Backport of fc1182af01c391ce33d7fcf51c756829c6a11d5b from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:10>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"fa3ae446d938455b47fc978cdfecb956b1238812" fa3ae44]:
{{{
#!CommitTicketReference repository=""
revision="fa3ae446d938455b47fc978cdfecb956b1238812"
[2.2.x] Refs #30083 -- Clarified database state of instances in
signals.pre_init docs.
Backport of a2e1c17f193f5017e1f6fac7d860f1f9e34d7892 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:9>
Comment (by Sebastiaan ten Pas):
Thanks for the PR Mariusz Felisiak and your initial PR Mariusz Felisiak. I
would prefer to have the code change, as I still consider it a bug, but I
can live with only the documentation change for now.
--
Ticket URL: <https://code.djangoproject.com/ticket/30083#comment:11>