On 14 touko, 18:07, Tim Chase <[email protected]> wrote: > On 2013-05-14 10:43, Alex Ogier wrote: > > > What happens in the following case? > > > a = A(dict_field={"hello": "world"}) > > d = a.dict_field > > a.save() > > a.refresh() > > d["hello"] = "planet" # does this mutate a.dict_field? does the > > answer change if somebody changed the database in between saving > > and refreshing? > > I'd expect the same as the stdlib's pickle module: when you load an > object from a pickled stream (what I'd consider analogous to > the .refresh() topic at hand), you create a new object. So "d" would > still reference the original dict_field, while the a.dict_field would > contain a new dict object that doesn't contain "hello" as a key. > > It is easy to explain, consistent with the stdlib, and unlikely to > harbor strange edge conditions once you understand what it's doing. > It also allows for doing things like > > d = a.dict_field > a.save() > # semi-lengthy-process > a.refresh() > if a.dict_field != d: > somebody_changed_something_while_we_were_occupied("!!!") > > Prudent? Maybe, maybe not. Believable? sure. But if "d" > auto-updates upon refresh, there's no easy way to make this test > (short of doing a deep-copy of "d" to preserve it). > > I think DB-trumps-local is pretty sensible in both cases you describe > (above, and the OneToOne), that if you reload, you get what the DB > tells you and you're responsible for refreshing any internal > references you have reaching into the object. As our preschooler has > learned, "you get what you get, and you don't throw a fit." :-)
IMO we should always create new instances for mutable field values. Else we are in for huge amount of complexity. An exception is cached related instances. For those an useful optimization is to not throw them away if the related object's local id hasn't changed. The user can refresh() the related instances if need be, but there is no way to keep them if they are automatically thrown out... And if the local id has changed, then the local cached value + the remote cached value for reverse o2o field should be cleared. Next access will fetch a fresh object from DB, but refresh() itself wont do that. As for how to implement the feature on technical level: I think we will need to go through get() of new instance and then either direct assignment to __dict__ for each fetched field, or use setattr() for each fetched field. Getting a new instance is a good idea so that pre/ post_init signals are called (some fields rely on those). Direct __dict__ assignment is what deferred field loading does currently so that might be better idea than setattr(). And setattr() was already called, but for different instance. We should likely try to rely less on __init__ in general, and have a different path for model instance creation when loading from DB. Direct assignment to the object's __dict__ would make DB loading work more like unpickle. See https://code.djangoproject.com/ticket/19501 for details. - Anssi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers?hl=en. For more options, visit https://groups.google.com/groups/opt_out.
