- merge() may actually work now, though we've heard that before...

- merge() uses the priamry key attributes on the object if _instance_key not present.  so merging works for instances that dont have an instnace_key, will still issue UPDATE for existing rows.
- improved collection behavior for merge() - will remove elements from a destination collection that are not in the source.
- fixed naive set-mutation issue in Select._get_display_froms
- simplified fixtures.Base a bit

4 files changed, 26 insertions, 18 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index a511c9bbb..f57298d7c 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -569,7 +569,7 @@ class CollectionAttributeImpl(AttributeImpl):
             self.fire_remove_event(state, value, initiator)
         else:
             collection.remove_with_event(value, initiator)
-
+    
     def set(self, state, value, initiator):
         """Set a value on the given object.
 
diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 970e49ea4..d050f40d7 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -416,7 +416,6 @@ class PropertyLoader(StrategizedProperty):
             return
             
         if not "merge" in self.cascade:
-            # TODO: lazy callable should merge to the new instance
             dest._state.expire_attributes([self.key])
             return
 
@@ -425,15 +424,18 @@ class PropertyLoader(StrategizedProperty):
             return
         
         if self.uselist:
-            dest_list = attributes.init_collection(dest, self.key)
+            dest_list = []
             for current in instances:
                 _recursive[(current, self)] = True
                 obj = session.merge(current, entity_name=self.mapper.entity_name, dont_load=dont_load, _recursive=_recursive)
                 if obj is not None:
-                    if dont_load:
-                        dest_list.append_without_event(obj)
-                    else:
-                        dest_list.append_with_event(obj)
+                    dest_list.append(obj)
+            if dont_load:
+                coll = attributes.init_collection(dest, self.key)
+                for c in dest_list:
+                    coll.append_without_event(c) 
+            else:
+                getattr(dest.__class__, self.key).impl._set_iterable(dest._state, dest_list)
         else:
             current = instances[0]
             if current is not None:
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 391bc925b..b7a4aa911 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -955,8 +955,10 @@ class Session(object):
         if key is None:
             if dont_load:
                 raise exceptions.InvalidRequestError("merge() with dont_load=True option does not support objects transient (i.e. unpersisted) objects.  flush() all changes on mapped instances before merging with dont_load=True.")
-            merged = attributes.new_instance(mapper.class_)
-        else:
+            key = mapper.identity_key_from_instance(instance)
+
+        merged = None
+        if key:
             if key in self.identity_map:
                 merged = self.identity_map[key]
             elif dont_load:
@@ -969,15 +971,19 @@ class Session(object):
                 self._update_impl(merged, entity_name=mapper.entity_name)
             else:
                 merged = self.get(mapper.class_, key[1])
-                if merged is None:
-                    raise exceptions.AssertionError("Instance %s has an instance key but is not persisted" % mapperutil.instance_str(instance))
+        
+        if merged is None:
+            merged = attributes.new_instance(mapper.class_)
+            self.save(merged, entity_name=mapper.entity_name)
+            
         _recursive[instance] = merged
+        
         for prop in mapper.iterate_properties:
             prop.merge(self, instance, merged, dont_load, _recursive)
-        if key is None:
-            self.save(merged, entity_name=mapper.entity_name)
-        elif dont_load:
-            merged._state.commit_all()
+            
+        if dont_load:
+            merged._state.commit_all()  # remove any history
+
         return merged
 
     def identity_key(cls, *args, **kwargs):
diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index 2cd10720a..758f75ebe 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -3096,9 +3096,9 @@ class Select(_SelectBaseMixin, FromClause):
 
         if self._froms:
             froms.update(self._froms)
-
-        for f in froms:
-            froms.difference_update(f._hide_froms)
+        
+        toremove = itertools.chain(*[f._hide_froms for f in froms])
+        froms.difference_update(toremove)
 
         if len(froms) > 1 or self.__correlate:
             if self.__correlate: