- attached load_on_fks.patch
ken wants FK attributes to load on pending with an option for transient too
the attached patch begins to address this. many more tests will be needed.
Highlights include:
-
the mechanism by which we determine if we should use "committed" attributes or not changes. This part of the patch I like regardless. Persistent objects do a better job of loading related objects with this behavior and the approach is simple - only use the "committed" state if we're in a flush.
-
attribute events send through a flag that still disables loading on pending
-
to get a transient to load, just set its session_id. this is fairly clean and sneaky enough not to confuse new users.
-
backrefs need to be more careful about things
-
I've worked with this all day, and I still don't think load-on-pending is a great idea most of the time, it issues more autoflushes, more useless SELECT statements, more complex logic needed to guess if the SELECT is wasteful - and the load on transient thing is entirely special case stuff. That aspect of the functionality is pushed into a relationship() flag
load_on_pending
.
Comments (9)
-
reporter -
reporter - changed milestone to 0.6.5
The majority of the patch is a bugfix for lazyloads of m2o attributes. The more controversial "load on pending" behavior is disabled by default. No existing tests needed to be changed, so the most recent version of the patch is in fe250af8eb7294f08f491b3c1af9cf86a769f78c.
-
Account Deleted I really appreciate you spending your time on this, Mike. Thank you.
I thought about setting the session_id on transients, but my concern is that would "turn them into" pending or persistent objects in some way. Is that the case in any way?
At the extreme risk of pushing my luck, have you thought about auto-expiration of related objects/collections when the join criteria of the main object changes (generally, this will be fks, but i think a more generic approach is any join field that exists on the main object's table)?
My plan is to implement at least a simple version of this (maybe using
AttributeExtension
), and I'm not really asking you to do this, but you mentioned "That feature is something I am open to exploring." so I wanted to know your thoughts.Thanks very much again.
-
reporter Replying to guest:
I thought about setting the session_id on transients, but my concern is that would "turn them into" pending or persistent objects in some way. Is that the case in any way?
there isn't with that approach, since the object isn't part of any of the session's collections. Note that I am not making
_attach
public at this time, I don't foresee changes there but I'm not comfortable yet saying that the pattern is part of the final API.At the extreme risk of pushing my luck, have you thought about auto-expiration of related objects/collections when the join criteria of the main object changes (generally, this will be fks, but i think a more generic approach is any join field that exists on the main object's table)?
I think the room still exists for that behavior and it would fit more nicely now than it did before. It would be a little easier to address after 0.7 once the new event model is in place and I've made another pass through the attributes package, but its just a matter of putting event listeners on the ColumnProperty objects that are in the left side of the RelationshipProperty object's local_remote pairs collection.
My plan is to implement at least a simple version of this (maybe using
AttributeExtension
), and I'm not really asking you to do this, but you mentioned "That feature is something I am open to exploring." so I wanted to know your thoughts.yup you got it
-
Account Deleted Replying to zzzeek:
I thought about setting the session_id on transients, but my concern is that would "turn them into" pending or persistent objects in some way. Is that the case in any way?
there isn't with that approach, since the object isn't part of any of the session's collections. Note that I am not making
_attach
public at this time, I don't foresee changes there but I'm not comfortable yet saying that the pattern is part of the final API.For transient loading, the idea is "set session_id on the state." This seems to work if I set it just during the call to attribute.impl.get() and then unset it after the return. However, if I set session_id immediately at the objects creation during init(), then I'm getting strange problems like the session trying to INSERT after a merge() when the object is persistent in the database and other similar issues.
Does this surprise you? Were you working off the assumption the session_id would be set only on transient objects that were never going to become pending, or would you think an approach like the following ''should'' work?
def __init__(self): attributes.instance_state(self).session_id = DBSession().hash_key
I ask because I think I prefer the above code in our turbogears single scoped session at a time environment where I always want transient loading available globally. If you are under the impression that ''should'' work, then I want to get it there. However, if that approach is immediately obviously flawed (when it is added to the (same) session later) or merged(), then I'll back off and set the attribute.impl.callable_ to s function that sets the session_id if it is None, invokes attribute.impl.get() and then unsets session_id.
-
reporter if merge() is flaky when you hand it a transient with session_id set, that doesn't totally surprise me. I don't know offhand exactly what's going on there but merge() is definitely very interested to know if the incoming object is already associated with that session. It's probably assuming the object you're handing it is the object, and since it has no state.key it will turn it into an INSERT, sure.
-
reporter - changed status to resolved
-
Account Deleted At the extreme risk of pushing my luck, have you thought about auto-expiration of related objects/collections when the join criteria of the main object changes (generally, this will be fks, but i think a more generic approach is any join field that exists on the main object's table)?
I think the room still exists for that behavior and it would fit more nicely now than it did before. It would be a little easier to address after 0.7 once the new event model is in place and I've made another pass through the attributes package, but its just a matter of putting event listeners on the ColumnProperty objects that are in the left side of the RelationshipProperty object's local_remote pairs collection.
Is there a ticket for this? If so, would you please add kb at retailarchitects ... I want to follow it. Thanks.
-
reporter - removed milestone
Removing milestone: 0.6.5 (automated comment)
- Log in to comment
the pending/transient load is a flag. I still don't like this as a default. Did add checks for "NULL" keys to help it work better