From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 7 Dec 2009 22:39:33 +0000 (+0000)
Subject: - Fixed a needless select which would occur when merging
X-Git-Tag: rel_0_6beta1~140
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2fbdb67cbec56a85e3fe000db94eee14101a47fa;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- 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 f5404b0e8a..36ac4f2eb8 100644
--- 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 c0b5666b08..d3d02ef3f0 100644
--- 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 5433515caa..e76e588d80 100644
--- 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={