- Fixed bug in :meth:`.Session.merge` where an object with a composite

primary key that has values for some but not all of the PK fields
would emit a SELECT statement leaking the internal NEVER_SET symbol
into the query, rather than detecting that this object does not have
a searchable primary key and no SELECT should be emitted.
fixes #3647

(cherry picked from commit 366f97b5617af0d15cfaf594ec5ef0408c70e873)

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index d607110aae601d539e6083dcc7ecce0bb6e2328f..ac250a7aa7a77f9f8aea9abc81aeb5dec7b0f676 100644 (file)
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -19,6 +19,16 @@
     :version: 1.0.12
     :released:
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3647
+
+        Fixed bug in :meth:`.Session.merge` where an object with a composite
+        primary key that has values for some but not all of the PK fields
+        would emit a SELECT statement leaking the internal NEVER_SET symbol
+        into the query, rather than detecting that this object does not have
+        a searchable primary key and no SELECT should be emitted.
+
     .. change::
         :tags: bug, postgresql
         :tickets: 3644

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 2c1bdedee0175afa129b3e5f13d1509d290176dc..000441fb95ef8530374420314e15ebabbcb934d2 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1726,6 +1726,9 @@ class Session(_SessionClassMethods):
                     "all changes on mapped instances before merging with "
                     "load=False.")
             key = mapper._identity_key_from_state(state)
+            key_is_persistent = attributes.NEVER_SET not in key[1]
+        else:
+            key_is_persistent = True
 
         if key in self.identity_map:
             merged = self.identity_map[key]
@@ -1742,9 +1745,10 @@ class Session(_SessionClassMethods):
             self._update_impl(merged_state)
             new_instance = True
 
-        elif not _none_set.intersection(key[1]) or \
+        elif key_is_persistent and (
+            not _none_set.intersection(key[1]) or
             (mapper.allow_partial_pks and
-             not _none_set.issuperset(key[1])):
+             not _none_set.issuperset(key[1]))):
             merged = self.query(mapper.class_).get(key[1])
         else:
             merged = None

diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py
index a52274896b882734492b401a1d0ab690e8583754..14b1b05131ad1c5607469d1cf5345fef03bc4c6d 100644 (file)
--- a/test/orm/test_merge.py
+++ b/test/orm/test_merge.py
@@ -6,7 +6,7 @@ from sqlalchemy import testing
 from sqlalchemy.util import OrderedSet
 from sqlalchemy.orm import mapper, relationship, create_session, \
     PropComparator, synonym, comparable_property, sessionmaker, \
-    attributes, Session, backref, configure_mappers
+    attributes, Session, backref, configure_mappers, foreign
 from sqlalchemy.orm.collections import attribute_mapped_collection
 from sqlalchemy.orm.interfaces import MapperOption
 from sqlalchemy.testing import eq_, ne_
@@ -451,6 +451,55 @@ class MergeTest(_fixtures.FixtureTest):
         eq_(u2.addresses[1].email_address, 'afafds')
         eq_(load.called, 21)
 
+    def test_dont_send_neverset_to_get(self):
+        # test issue #3647
+        CompositePk, composite_pk_table = (
+            self.classes.CompositePk, self.tables.composite_pk_table
+        )
+        mapper(CompositePk, composite_pk_table)
+        cp1 = CompositePk(j=1, k=1)
+
+        sess = Session()
+
+        rec = []
+
+        def go():
+            rec.append(sess.merge(cp1))
+        self.assert_sql_count(testing.db, go, 0)
+        rec[0].i = 5
+        sess.commit()
+        eq_(rec[0].i, 5)
+
+    def test_dont_send_neverset_to_get_w_relationship(self):
+        # test issue #3647
+        CompositePk, composite_pk_table = (
+            self.classes.CompositePk, self.tables.composite_pk_table
+        )
+        User, users = (
+            self.classes.User, self.tables.users
+        )
+        mapper(User, users, properties={
+            'elements': relationship(
+                CompositePk,
+                primaryjoin=users.c.id == foreign(composite_pk_table.c.i))
+        })
+        mapper(CompositePk, composite_pk_table)
+
+        u1 = User(id=5, name='some user')
+        cp1 = CompositePk(j=1, k=1)
+        u1.elements.append(cp1)
+        sess = Session()
+
+        rec = []
+
+        def go():
+            rec.append(sess.merge(u1))
+        self.assert_sql_count(testing.db, go, 1)
+        u2 = rec[0]
+        sess.commit()
+        eq_(u2.elements[0].i, 5)
+        eq_(u2.id, 5)
+
     def test_no_relationship_cascade(self):
         """test that merge doesn't interfere with a relationship()
            target that specifically doesn't include 'merge' cascade.