From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sun, 28 Apr 2019 16:40:31 +0000 (-0400)
Subject: Warn on merge of already-pending object
X-Git-Tag: rel_1_4_0b1~894^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=23a1c60982dc4799c76f0cec276a3bae8a24395b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

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
---

diff --git a/doc/build/changelog/unreleased_13/4647.rst b/doc/build/changelog/unreleased_13/4647.rst
new file mode 100644
index 0000000000..e78b801457
--- /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 0b1a3b1010..eccd60fe2a 100644
--- 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 995989cd92..48a519dd7c 100644
--- 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,