From 699d77958239d299812af57aa3ba9e7fd7a5ac88 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 7 Dec 2009 22:51:19 +0000
Subject: [PATCH] - merge -r6534 of trunk, for [ticket:1618] - backported 0.6's
 approach to "null pks allowed" in mapper._instance_processor

---
 CHANGES                       |  4 ++++
 lib/sqlalchemy/orm/mapper.py  |  6 ++----
 lib/sqlalchemy/orm/session.py | 40 +++++++++++++++++------------------
 test/orm/test_merge.py        | 10 +++++++++
 4 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/CHANGES b/CHANGES
index d0401f6329..0189b06567 100644
--- a/CHANGES
+++ b/CHANGES
@@ -129,6 +129,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/mapper.py b/lib/sqlalchemy/orm/mapper.py
index fd6d3a4a71..07cc7aaeb3 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -41,6 +41,7 @@ __all__ = (
 _mapper_registry = weakref.WeakKeyDictionary()
 _new_mappers = False
 _already_compiling = False
+_none_set = frozenset([None])
 
 # a list of MapperExtensions that will be installed in all mappers by default
 global_extensions = []
@@ -1684,10 +1685,7 @@ class Mapper(object):
                     self._log_debug("_instance(): identity key %s not in session" % (identitykey,))
 
                 if self.allow_null_pks:
-                    for x in identitykey[1]:
-                        if x is not None:
-                            break
-                    else:
+                    if _none_set.issuperset(identitykey[1]):
                         return None
                 else:
                     if None in identitykey[1]:
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 914da874d5..3dea2f6a3d 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
 
@@ -1173,7 +1173,7 @@ class Session(object):
         new_instance = False
         state = attributes.instance_state(instance)
         key = state.key
-
+        
         if key is None:
             if dont_load:
                 raise sa_exc.InvalidRequestError(
@@ -1183,24 +1183,24 @@ class Session(object):
                     "dont_load=True.")
             key = mapper._identity_key_from_state(state)
 
-        merged = None
-        if key:
-            if key in self.identity_map:
-                merged = self.identity_map[key]
-            elif dont_load:
-                if state.modified:
-                    raise sa_exc.InvalidRequestError(
-                        "merge() with dont_load=True option does not support "
-                        "objects marked as 'dirty'.  flush() all changes on "
-                        "mapped instances before merging with dont_load=True.")
-                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 dont_load:
+            if state.modified:
+                raise sa_exc.InvalidRequestError(
+                    "merge() with dont_load=True option does not support "
+                    "objects marked as 'dirty'.  flush() all changes on "
+                    "mapped instances before merging with dont_load=True.")
+            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 f4e3872b06..d0f667237c 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={
-- 
2.47.3