- Session.merge() is performance optimized, using half the

call counts for "load=False" mode compared to 0.5 and
significantly fewer SQL queries in the case of collections
for "load=True" mode.

2 files changed, 60 insertions, 38 deletions

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 1bb850488..e63c2b867 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -105,13 +105,17 @@ class ColumnProperty(StrategizedProperty):
     def setattr(self, state, value, column):
         state.get_impl(self.key).set(state, state.dict, value, None)
 
-    def merge(self, session, source, dest, load, _recursive):
-        value = attributes.instance_state(source).value_as_iterable(
-            self.key, passive=True)
-        if value:
-            setattr(dest, self.key, value[0])
+    def merge(self, session, source_state, source_dict, dest_state, dest_dict, load, _recursive):
+        if self.key in source_dict:
+            value = source_dict[self.key]
+        
+            if not load:
+                dest_dict[self.key] = value
+            else:
+                impl = dest_state.get_impl(self.key)
+                impl.set(dest_state, dest_dict, value, None)
         else:
-            attributes.instance_state(dest).expire_attributes([self.key])
+            dest_state.expire_attributes([self.key])
 
     def get_col_value(self, column, value):
         return value
@@ -301,7 +305,7 @@ class SynonymProperty(MapperProperty):
             proxy_property=self.descriptor
             )
 
-    def merge(self, session, source, dest, load, _recursive):
+    def merge(self, session, source_state, source_dict, dest_state, dest_dict, load, _recursive):
         pass
         
 log.class_logger(SynonymProperty)
@@ -334,7 +338,7 @@ class ComparableProperty(MapperProperty):
     def create_row_processor(self, selectcontext, path, mapper, row, adapter):
         return (None, None)
 
-    def merge(self, session, source, dest, load, _recursive):
+    def merge(self, session, source_state, source_dict, dest_state, dest_dict, load, _recursive):
         pass
 
 
@@ -624,50 +628,61 @@ class RelationProperty(StrategizedProperty):
     def __str__(self):
         return str(self.parent.class_.__name__) + "." + self.key
 
-    def merge(self, session, source, dest, load, _recursive):
+    def merge(self, session, source_state, source_dict, dest_state, dest_dict, load, _recursive):
         if load:
             # TODO: no test coverage for recursive check
             for r in self._reverse_property:
-                if (source, r) in _recursive:
+                if (source_state, r) in _recursive:
                     return
 
-        source_state = attributes.instance_state(source)
-        dest_state, dest_dict = attributes.instance_state(dest), attributes.instance_dict(dest)
-
         if not "merge" in self.cascade:
             dest_state.expire_attributes([self.key])
             return
 
-        instances = source_state.value_as_iterable(self.key, passive=True)
-
-        if not instances:
+        if self.key not in source_dict:
             return
 
         if self.uselist:
+            instances = source_state.get_impl(self.key).\
+                            get(source_state, source_dict)
+            
+            if load:
+                # for a full merge, pre-load the destination collection,
+                # so that individual _merge of each item pulls from identity
+                # map for those already present.  
+                # also assumes CollectionAttrbiuteImpl behavior of loading
+                # "old" list in any case
+                dest_state.get_impl(self.key).get(dest_state, dest_dict)
+                
             dest_list = []
             for current in instances:
-                _recursive[(current, self)] = True
-                obj = session._merge(current, load=load, _recursive=_recursive)
+                current_state = attributes.instance_state(current)
+                current_dict = attributes.instance_dict(current)
+                _recursive[(current_state, self)] = True
+                obj = session._merge(current_state, current_dict, load=load, _recursive=_recursive)
                 if obj is not None:
                     dest_list.append(obj)
+                    
             if not load:
                 coll = attributes.init_state_collection(dest_state, dest_dict, self.key)
                 for c in dest_list:
                     coll.append_without_event(c)
             else:
-                getattr(dest.__class__, self.key).impl._set_iterable(dest_state, dest_dict, dest_list)
+                dest_state.get_impl(self.key)._set_iterable(dest_state, dest_dict, dest_list)
         else:
-            current = instances[0]
+            current = source_dict[self.key]
             if current is not None:
-                _recursive[(current, self)] = True
-                obj = session._merge(current, load=load, _recursive=_recursive)
+                current_state = attributes.instance_state(current)
+                current_dict = attributes.instance_dict(current)
+                _recursive[(current_state, self)] = True
+                obj = session._merge(current_state, current_dict, load=load, _recursive=_recursive)
             else:
                 obj = None
-            
+
             if not load:
-                dest_state.dict[self.key] = obj
+                dest_dict[self.key] = obj
             else:
-                setattr(dest, self.key, obj)
+                dest_state.get_impl(self.key).set(dest_state, dest_dict, obj, None)
 
     def cascade_iterator(self, type_, state, visited_instances, halt_on=None):
         if not type_ in self.cascade:
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index f171622d4..ee4286c67 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1103,24 +1103,25 @@ class Session(object):
             load = not kw['dont_load']
             util.warn_deprecated("dont_load=True has been renamed to load=False.")
         
-        # TODO: this should be an IdentityDict for instances, but will
-        # need a separate dict for PropertyLoader tuples
         _recursive = {}
         self._autoflush()
+        _object_mapper(instance) # verify mapped
         autoflush = self.autoflush
         try:
             self.autoflush = False
-            return self._merge(instance, load=load, _recursive=_recursive)
+            return self._merge(
+                            attributes.instance_state(instance), 
+                            attributes.instance_dict(instance), 
+                            load=load, _recursive=_recursive)
         finally:
             self.autoflush = autoflush
         
-    def _merge(self, instance, load=True, _recursive=None):
-        mapper = _object_mapper(instance)
-        if instance in _recursive:
-            return _recursive[instance]
+    def _merge(self, state, state_dict, load=True, _recursive=None):
+        mapper = _state_mapper(state)
+        if state in _recursive:
+            return _recursive[state]
 
         new_instance = False
-        state = attributes.instance_state(instance)
         key = state.key
         
         if key is None:
@@ -1134,6 +1135,7 @@ class Session(object):
 
         if key in self.identity_map:
             merged = self.identity_map[key]
+            
         elif not load:
             if state.modified:
                 raise sa_exc.InvalidRequestError(
@@ -1154,16 +1156,21 @@ class Session(object):
         if merged is None:
             merged = mapper.class_manager.new_instance()
             merged_state = attributes.instance_state(merged)
+            merged_dict = attributes.instance_dict(merged)
             new_instance = True
-            self.add(merged)
-
-        _recursive[instance] = merged
+            self._save_or_update_state(merged_state)
+        else:
+            merged_state = attributes.instance_state(merged)
+            merged_dict = attributes.instance_dict(merged)
+            
+        _recursive[state] = merged
 
         for prop in mapper.iterate_properties:
-            prop.merge(self, instance, merged, load, _recursive)
+            prop.merge(self, state, state_dict, merged_state, merged_dict, load, _recursive)
 
         if not load:
-            attributes.instance_state(merged).commit_all(attributes.instance_dict(merged), self.identity_map)  # remove any history
+            # remove any history
+            merged_state.commit_all(merged_dict, self.identity_map)  
 
         if new_instance:
             merged_state._run_on_load(merged)