Provide an option to not load the old target of a scalar relation upon setting it

[Former user]

(original reporter: ged) Currently if you set a many-to-one relationship of an object to some value and that object already had a value for that relation, and the target object is not in the current session, an immediate fetch of that old record is triggered.

In my opinion, this shouldn't be the case, so either that behavior should be provided as an option, or scrapped entirely. I personally don't see any point that behavior, but I guess it is there for a reason. Maybe it is needed to provide before_update hooks to the backref if any (I don't know if before_update are guaranteed to happen even for one-to-many modifications). And even if that's the case, I fail to see any point in that behavior when there is no backref for the many-to-one relationship (which is the case in the attached test case)

It can also produce confusing effects in combination with the default autoflush session: it sends an update to the database earlier than expected if some objects in the session were modified before that attribute is modified (but not the opposite).

[jek]

foo.rel = None needs to emit a delete event for the previous value of rel and process Python-side cascades, no?

[Mike Bayer]

Replying to jek:

foo.rel = None needs to emit a delete event for the previous value of rel and process Python-side cascades, no?

it does. So, what the attached patch does is checks to see if the events we are handling will actually need the previous value present when the event is emitted - if not, a flag is sent through to the ScalarObjectAttributeImpl. The flag indicates a passive get() of the previous instance, and if the instance is not loaded, the event will receive PASSIVE_NORESULT.

We need the previous value if we are doing delete-orphan cascade, since that is handled partially in an on-detach event handler. We might need it also if there are any foreign key attributes on the remote side of the relation, although the dependency processors might do an active load at flush time in that case. We don't need it if we are doing delete cascade, since delete cascade results in a load during dependency processing depending on the value of "passive_deletes".

We also may or may not decide we need it if any user-defined AttributeExtensions are present. What may work there would be a class-level option on AttributeExtension, such as needs_removed_item=True or something like that.

I'm trying to bring 0.5 into deep stability so I'm not sure if i want to push this patch to 0.6.

[Former user]

(original author: ged) That line doesn't seem right: if hasattr(prop, 'cascade') and prop.cascade.delete_orphan:

Shouldn't the delete_orphan check be on the backref?

Also, the name "simple_many_to_one" seems awkward.

Since the patch changes the default behavior if there is no delete-orphan cascade, I understand you don't want it in the 0.5 branch. That's fine with me.

[Mike Bayer]

Replying to ged:

That line doesn't seem right: if hasattr(prop, 'cascade') and prop.cascade.delete_orphan:

Shouldn't the delete_orphan check be on the backref?

delete-orphan cascade doesn't require a backref to be present (backrefs should never be required for anything). the hasparent() capability of the attribute package handles the question of "orphan" status.

Also, the name "simple_many_to_one" seems awkward.

it seems perfectly descriptive to me - many to one, and only the "simple" ones with a foreign key on the left and primary key on the right (i.e. no weird join conditions). it's also a name that is local to the private strategies module so the name here is not very critical.

[Mike Bayer]

#1483 addressed part of this, and in 306c901946da9b06e79171c167a454cf0550b11d you can see an exhaustive set of tests illustrating everything you need to know about this issue. In fact the "dont load at all" approach on the many-to-one side does have side effects, which are illustrated in the tests. However they aren't horrible effects and are similar to a lot of other "dont load" related behaviors, so we should carefully consider if we want to roll this out in 0.6.

[Mike Bayer]

a more involved solution that maintains the expected behavior minus the load is in 7b019f3d49cb2e4340e4c2029e9841af0153b2e6.

[Mike Bayer]

Removing milestone: 0.6.0 (automated comment)