expunge newly minted instance if merge() fails?

[Mike Bayer]

If a session.merge() raises an error after the new instance is created and saved, its now stuck in the session and you can't really get to it. a patch to resolve this is below, but we should come up with tests and think this through, including cascade scenarios. Errors can be raised by AttributeExtensions that reject parameter values, for example.

Index: lib/sqlalchemy/orm/session.py
===================================================================
--- lib/sqlalchemy/orm/session.py   (revision 5603)
+++ lib/sqlalchemy/orm/session.py   (working copy)
@@ -1209,16 +1209,19 @@

         _recursive[instance](instance) = merged

-        for prop in mapper.iterate_properties:
-            prop.merge(self, instance, merged, dont_load, _recursive)
+        try:
+            for prop in mapper.iterate_properties:
+                prop.merge(self, instance, merged, dont_load, _recursive)

-        if dont_load:
-            attributes.instance_state(merged).commit_all()  # remove any history
+            if dont_load:
+                attributes.instance_state(merged).commit_all()  # remove any history

-        if new_instance:
-            merged_state._run_on_load(merged)
-        return merged
-
+            if new_instance:
+                merged_state._run_on_load(merged)
+            return merged
+        except:
+            if new_instance:
+                self.expunge(merged)
     @classmethod
     def identity_key(cls, *args, **kwargs):
         return mapperutil.identity_key(*args, **kwargs)

[Mike Bayer]

the behavior on cascade is if A->(B1->C1, B2->C2), and C2 fails, C2, B2 and A would be evicted, and B1 and C1 would not, if they were already in. Trying to end-run and guess if we should find everything and evict, etc. is not really worth it, because you just call rollback() and that resets everything to clean.