Warn on merge of already-pending object

A warning is now emitted for the case where a transient object is being
merged into the session with :meth:`.Session.merge` when that object is
already transient in the :class:`.Session`.   This warns for the case where
the object would normally be double-inserted.

Fixes: #4647
Change-Id: Ie5223a59a2856664bf283017e962caf8c4230536
(cherry picked from commit 23a1c60982dc4799c76f0cec276a3bae8a24395b)

diff --git a/doc/build/changelog/unreleased_13/4647.rst b/doc/build/changelog/unreleased_13/4647.rst
new file mode 100644 (file)
index 0000000..e78b801
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4647.rst
@@ -0,0 +1,9 @@
+.. change::
+   :tags: bug, orm
+   :tickets: 4647
+
+   A warning is now emitted for the case where a transient object is being
+   merged into the session with :meth:`.Session.merge` when that object is
+   already transient in the :class:`.Session`.   This warns for the case where
+   the object would normally be double-inserted.
+

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 0b1a3b1010227667123284ee822ea11012765c91..eccd60fe2ad3ce4f5fbf176a363568b1f1229b02 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -2110,6 +2110,13 @@ class Session(_SessionClassMethods):
         key = state.key
 
         if key is None:
+            if state in self._new:
+                util.warn(
+                    "Instance %s is already pending in this Session yet is "
+                    "being merged again; this is probably not what you want "
+                    "to do" % state_str(state)
+                )
+
             if not load:
                 raise sa_exc.InvalidRequestError(
                     "merge() with load=False option does not support "

diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py
index 995989cd9253cca514466dccdad660143562e7cb..48a519dd7c908a89a9afd17fc576baa9d05fa0f6 100644 (file)
--- a/test/orm/test_merge.py
+++ b/test/orm/test_merge.py
@@ -26,6 +26,7 @@ from sqlalchemy.orm.collections import attribute_mapped_collection
 from sqlalchemy.orm.interfaces import MapperOption
 from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import eq_
+from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import in_
 from sqlalchemy.testing import not_in_
@@ -84,6 +85,36 @@ class MergeTest(_fixtures.FixtureTest):
 
         self.assert_sql_count(testing.db, go, 0)
 
+    def test_warn_transient_already_pending_nopk(self):
+        User, users = self.classes.User, self.tables.users
+
+        mapper(User, users)
+        sess = create_session()
+        u = User(name="fred")
+
+        sess.add(u)
+
+        with expect_warnings(
+            "Instance <User.*> is already pending in this Session yet is "
+            "being merged again; this is probably not what you want to do"
+        ):
+            u2 = sess.merge(u)
+
+    def test_warn_transient_already_pending_pk(self):
+        User, users = self.classes.User, self.tables.users
+
+        mapper(User, users)
+        sess = create_session()
+        u = User(id=1, name="fred")
+
+        sess.add(u)
+
+        with expect_warnings(
+            "Instance <User.*> is already pending in this Session yet is "
+            "being merged again; this is probably not what you want to do"
+        ):
+            u2 = sess.merge(u)
+
     def test_transient_to_pending_collection(self):
         User, Address, addresses, users = (
             self.classes.User,