Consider merge key with (None, ) as non-persistent

Fixed bug in :meth:`.Session.merge` where objects in a collection that had
the primary key attribute set to ``None`` for a key that is  typically
autoincrementing would be considered to be a database-persisted key for
part of the internal deduplication process, causing only one object to
actually be inserted in the database.

Change-Id: I0a6e00043be0b2979cda33740e1be3b430ecf8c7
Fixes: #4056

diff --git a/doc/build/changelog/unreleased_11/4056.rst b/doc/build/changelog/unreleased_11/4056.rst
new file mode 100644 (file)
index 0000000..b8e02a0
--- /dev/null
+++ b/doc/build/changelog/unreleased_11/4056.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4056
+    :versions: 1.2.0b3
+
+    Fixed bug in :meth:`.Session.merge` where objects in a collection that had
+    the primary key attribute set to ``None`` for a key that is  typically
+    autoincrementing would be considered to be a database-persisted key for
+    part of the internal deduplication process, causing only one object to
+    actually be inserted in the database.

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 7ec8bfb2d1de4cad15812164ae76246b96c64b23..ebf1f61f1d0512d5375531473ea03f86bdbbf4d7 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1867,7 +1867,10 @@ 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]
+            key_is_persistent = attributes.NEVER_SET not in key[1] and (
+                not _none_set.intersection(key[1]) or
+                (mapper.allow_partial_pks and not _none_set.issuperset(key[1]))
+            )
         else:
             key_is_persistent = True
 
@@ -1888,10 +1891,7 @@ class Session(_SessionClassMethods):
             self._update_impl(merged_state)
             new_instance = True
 
-        elif key_is_persistent and (
-            not _none_set.intersection(key[1]) or
-            (mapper.allow_partial_pks and
-             not _none_set.issuperset(key[1]))):
+        elif key_is_persistent:
             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 f751d43a816ec601bc5fa84c7b9472b3a4d90154..8c139023882c3e2e1e8b377b96f7bc8d95b7e674 100644 (file)
--- a/test/orm/test_merge.py
+++ b/test/orm/test_merge.py
@@ -98,6 +98,42 @@ class MergeTest(_fixtures.FixtureTest):
                 Address(id=2, email_address='fred2'),
             ])))
 
+    def test_transient_to_pending_collection_pk_none(self):
+        User, Address, addresses, users = (self.classes.User,
+                                           self.classes.Address,
+                                           self.tables.addresses,
+                                           self.tables.users)
+
+        mapper(User, users, properties={
+            'addresses': relationship(Address, backref='user',
+                                      collection_class=OrderedSet)})
+        mapper(Address, addresses)
+        load = self.load_tracker(User)
+        self.load_tracker(Address, load)
+
+        u = User(id=None, name='fred', addresses=OrderedSet([
+            Address(id=None, email_address='fred1'),
+            Address(id=None, email_address='fred2'),
+        ]))
+        eq_(load.called, 0)
+
+        sess = create_session()
+        sess.merge(u)
+        eq_(load.called, 3)
+
+        merged_users = [e for e in sess if isinstance(e, User)]
+        eq_(len(merged_users), 1)
+        assert merged_users[0] is not u
+
+        sess.flush()
+        sess.expunge_all()
+
+        eq_(sess.query(User).one(),
+            User(name='fred', addresses=OrderedSet([
+                Address(email_address='fred1'),
+                Address(email_address='fred2'),
+            ])))
+
     def test_transient_to_persistent(self):
         User, users = self.classes.User, self.tables.users