- Fixed a needless select which would occur when merging

transient objects that contained a null primary key
identifier.  [ticket:1618]

diff --git a/CHANGES b/CHANGES
index f5404b0e8a28c6961396793a6de48cfeb11e9897..36ac4f2eb88e9d914f785ed757cb8e687ee1cc8b 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -854,6 +854,10 @@ CHANGES
       would populate the related object's "subclass" table with
       data from the "subclass" table of the parent.
       [ticket:1485]
+    
+    - Fixed a needless select which would occur when merging
+      transient objects that contained a null primary key
+      identifier.  [ticket:1618]
       
     - relations() now have greater ability to be "overridden",
       meaning a subclass that explicitly specifies a relation()

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index c0b5666b08b32afc85569acf907be310406592b9..d3d02ef3f0c6f3a2d9f25f40e6b476b337a2fd53 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -19,7 +19,7 @@ from sqlalchemy.orm.util import class_mapper as _class_mapper
 from sqlalchemy.orm.util import (
     _class_to_mapper, _state_has_identity, _state_mapper,
     )
-from sqlalchemy.orm.mapper import Mapper
+from sqlalchemy.orm.mapper import Mapper, _none_set
 from sqlalchemy.orm.unitofwork import UOWTransaction
 from sqlalchemy.orm import identity
 
@@ -1124,7 +1124,7 @@ class Session(object):
         new_instance = False
         state = attributes.instance_state(instance)
         key = state.key
-
+        
         if key is None:
             if not load:
                 raise sa_exc.InvalidRequestError(
@@ -1134,24 +1134,25 @@ class Session(object):
                     "load=False.")
             key = mapper._identity_key_from_state(state)
 
-        merged = None
-        if key:
-            if key in self.identity_map:
-                merged = self.identity_map[key]
-            elif not load:
-                if state.modified:
-                    raise sa_exc.InvalidRequestError(
-                        "merge() with load=False option does not support "
-                        "objects marked as 'dirty'.  flush() all changes on "
-                        "mapped instances before merging with load=False.")
-                merged = mapper.class_manager.new_instance()
-                merged_state = attributes.instance_state(merged)
-                merged_state.key = key
-                self._update_impl(merged_state)
-                new_instance = True
-            else:
-                merged = self.query(mapper.class_).get(key[1])
+        if key in self.identity_map:
+            merged = self.identity_map[key]
+        elif not load:
+            if state.modified:
+                raise sa_exc.InvalidRequestError(
+                    "merge() with load=False option does not support "
+                    "objects marked as 'dirty'.  flush() all changes on "
+                    "mapped instances before merging with load=False.")
+            merged = mapper.class_manager.new_instance()
+            merged_state = attributes.instance_state(merged)
+            merged_state.key = key
+            self._update_impl(merged_state)
+            new_instance = True
 
+        elif not _none_set.issuperset(key[1]):
+            merged = self.query(mapper.class_).get(key[1])
+        else:
+            merged = None
+            
         if merged is None:
             merged = mapper.class_manager.new_instance()
             merged_state = attributes.instance_state(merged)

diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py
index 5433515caa5cf6a6d09dccd9722fda30469df325..e76e588d804a7a7cbeeca5dcf71de57d88856131 100644 (file)
--- a/test/orm/test_merge.py
+++ b/test/orm/test_merge.py
@@ -41,6 +41,16 @@ class MergeTest(_fixtures.FixtureTest):
         sess.expunge_all()
         eq_(sess.query(User).first(), User(id=7, name='fred'))
 
+    @testing.resolve_artifact_names
+    def test_transient_to_pending_no_pk(self):
+        """test that a transient object with no PK attribute doesn't trigger a needless load."""
+        mapper(User, users)
+        sess = create_session()
+        u = User(name='fred')
+        def go():
+            sess.merge(u)
+        self.assert_sql_count(testing.db, go, 0)
+        
     @testing.resolve_artifact_names
     def test_transient_to_pending_collection(self):
         mapper(User, users, properties={