From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 7 Oct 2022 15:25:08 +0000 (-0400)
Subject: dont mutate bind_arguments incoming dictionary
X-Git-Tag: rel_1_4_42~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=41df10db65ef5dfd5d04644e5f447908dad33cb8;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

dont mutate bind_arguments incoming dictionary

The :paramref:`_orm.Session.execute.bind_arguments` dictionary is no longer
mutated when passed to :meth:`_orm.Session.execute` and similar; instead,
it's copied to an internal dictionary for state changes. Among other
things, this fixes and issue where the "clause" passed to the
:meth:`_orm.Session.get_bind` method would be incorrectly referring to the
:class:`_sql.Select` construct used for the "fetch" synchronization
strategy, when the actual query being emitted was a :class:`_dml.Delete` or
:class:`_dml.Update`. This would interfere with recipes for "routing
sessions".

Fixes: #8614
Change-Id: I8d237449485c9bbf41db2b29a34b6136aa43b7bc
(cherry picked from commit 3efc9e1df378be8046d4b1f1b624968a62eb100f)
---

diff --git a/doc/build/changelog/unreleased_14/8614.rst b/doc/build/changelog/unreleased_14/8614.rst
new file mode 100644
index 0000000000..b975dbc1f9
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/8614.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 8614
+
+    The :paramref:`_orm.Session.execute.bind_arguments` dictionary is no longer
+    mutated when passed to :meth:`_orm.Session.execute` and similar; instead,
+    it's copied to an internal dictionary for state changes. Among other
+    things, this fixes and issue where the "clause" passed to the
+    :meth:`_orm.Session.get_bind` method would be incorrectly referring to the
+    :class:`_sql.Select` construct used for the "fetch" synchronization
+    strategy, when the actual query being emitted was a :class:`_dml.Delete` or
+    :class:`_dml.Update`. This would interfere with recipes for "routing
+    sessions".
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 4b05381db2..79b723184d 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1639,6 +1639,8 @@ class Session(_SessionClassMethods):
                 bind_arguments.update(kw)
         elif not bind_arguments:
             bind_arguments = {}
+        else:
+            bind_arguments = dict(bind_arguments)
 
         if (
             statement._propagate_attrs.get("compile_state_plugin", None)
diff --git a/test/orm/test_bind.py b/test/orm/test_bind.py
index 6326e0c4dc..802de99696 100644
--- a/test/orm/test_bind.py
+++ b/test/orm/test_bind.py
@@ -290,6 +290,28 @@ class BindIntegrationTest(_fixtures.FixtureTest):
 
         sess.close()
 
+    @testing.combinations(True, False)
+    def test_dont_mutate_binds(self, empty_dict):
+        users, User = (
+            self.tables.users,
+            self.classes.User,
+        )
+
+        mp = self.mapper_registry.map_imperatively(User, users)
+
+        sess = fixture_session()
+
+        if empty_dict:
+            bind_arguments = {}
+        else:
+            bind_arguments = {"mapper": mp}
+        sess.execute(select(1), bind_arguments=bind_arguments)
+
+        if empty_dict:
+            eq_(bind_arguments, {})
+        else:
+            eq_(bind_arguments, {"mapper": mp})
+
     @testing.combinations(
         (
             lambda session, Address: session.query(Address).statement,
diff --git a/test/orm/test_update_delete.py b/test/orm/test_update_delete.py
index 4eabe2f6c4..6be271e460 100644
--- a/test/orm/test_update_delete.py
+++ b/test/orm/test_update_delete.py
@@ -22,6 +22,9 @@ from sqlalchemy.orm import Session
 from sqlalchemy.orm import sessionmaker
 from sqlalchemy.orm import synonym
 from sqlalchemy.orm import with_loader_criteria
+from sqlalchemy.sql.dml import Delete
+from sqlalchemy.sql.dml import Update
+from sqlalchemy.sql.selectable import Select
 from sqlalchemy.testing import assert_raises
 from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import eq_
@@ -1460,6 +1463,42 @@ class UpdateDeleteTest(fixtures.MappedTest):
         ]
         eq_(["name", "age_int"], cols)
 
+    @testing.combinations(("update",), ("delete",), argnames="stmt_type")
+    @testing.combinations(
+        ("evaluate",), ("fetch",), (None,), argnames="sync_type"
+    )
+    def test_routing_session(self, stmt_type, sync_type, connection):
+        User = self.classes.User
+
+        if stmt_type == "update":
+            stmt = update(User).values(age=123)
+            expected = [Update]
+        elif stmt_type == "delete":
+            stmt = delete(User)
+            expected = [Delete]
+        else:
+            assert False
+
+        received = []
+
+        class RoutingSession(Session):
+            def get_bind(self, **kw):
+                received.append(type(kw["clause"]))
+                return super(RoutingSession, self).get_bind(**kw)
+
+        stmt = stmt.execution_options(synchronize_session=sync_type)
+
+        if sync_type == "fetch":
+            expected.insert(0, Select)
+
+            if not connection.dialect.full_returning:
+                expected.insert(0, Select)
+
+        with RoutingSession(bind=connection) as sess:
+            sess.execute(stmt)
+
+        eq_(received, expected)
+
 
 class UpdateDeleteIgnoresLoadersTest(fixtures.MappedTest):
     @classmethod