From: Federico Caselli <cfederico87@gmail.com>
Date: Fri, 1 Oct 2021 20:16:14 +0000 (+0200)
Subject: Warn when trying to execute a query object with a session.
X-Git-Tag: rel_1_4_26~39^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3f2209776183193552cbc9516710deb6598fd6cd;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Warn when trying to execute a query object with a session.

Passing a :class:`.Query` object to :meth:`_orm.Session.execute` is not
the intended use of this object, and will now raise a deprecation warning.

Fixes: #6284
Change-Id: I30a406d5a72335f9405f10f0a2030a32ccda41b9
---

diff --git a/doc/build/changelog/unreleased_14/6284.rst b/doc/build/changelog/unreleased_14/6284.rst
new file mode 100644
index 0000000000..ffb506ce0b
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6284.rst
@@ -0,0 +1,6 @@
+.. change::
+    :tags: orm
+    :tickets: 6284
+
+    Passing a :class:`.Query` object to :meth:`_orm.Session.execute` is not
+    the intended use of this object, and will now raise a deprecation warning.
diff --git a/lib/sqlalchemy/sql/coercions.py b/lib/sqlalchemy/sql/coercions.py
index dce3293524..f888bad4ca 100644
--- a/lib/sqlalchemy/sql/coercions.py
+++ b/lib/sqlalchemy/sql/coercions.py
@@ -191,7 +191,6 @@ def expect(
                 resolved = element
     else:
         resolved = element
-
     if (
         apply_propagate_attrs is not None
         and not apply_propagate_attrs._propagate_attrs
@@ -843,6 +842,28 @@ class ReturnsRowsImpl(RoleImpl):
 class StatementImpl(_CoerceLiterals, RoleImpl):
     __slots__ = ()
 
+    def _post_coercion(self, resolved, original_element, argname=None, **kw):
+        if resolved is not original_element and not isinstance(
+            original_element, util.string_types
+        ):
+            # use same method as Connection uses; this will later raise
+            # ObjectNotExecutableError
+            try:
+                original_element._execute_on_connection
+            except AttributeError:
+                util.warn_deprecated(
+                    "Object %r should not be used directly in a SQL statement "
+                    "context, such as passing to methods such as "
+                    "session.execute().  This usage will be disallowed in a "
+                    "future release.  "
+                    "Please use Core select() / update() / delete() etc. "
+                    "with Session.execute() and other statement execution "
+                    "methods." % original_element,
+                    "1.4",
+                )
+
+        return resolved
+
     def _implicit_coercions(
         self, original_element, resolved, argname=None, **kw
     ):
diff --git a/test/ext/test_baked.py b/test/ext/test_baked.py
index 8e96dd3adb..ace76cff57 100644
--- a/test/ext/test_baked.py
+++ b/test/ext/test_baked.py
@@ -1080,14 +1080,14 @@ class CustomIntegrationTest(testing.AssertsCompiledSQL, BakedTest):
         q = sess.query(User).filter(User.id == 7).set_cache_key("user7")
 
         eq_(
-            sess.execute(q).all(),
+            sess.execute(q.statement).all(),
             [(User(id=7, addresses=[Address(id=1)]),)],
         )
 
         eq_(list(q.cache), ["user7"])
 
         eq_(
-            sess.execute(q).all(),
+            sess.execute(q.statement).all(),
             [(User(id=7, addresses=[Address(id=1)]),)],
         )
 
diff --git a/test/orm/test_bind.py b/test/orm/test_bind.py
index 3df5a27177..4f6f4c4fbb 100644
--- a/test/orm/test_bind.py
+++ b/test/orm/test_bind.py
@@ -292,7 +292,7 @@ class BindIntegrationTest(_fixtures.FixtureTest):
 
     @testing.combinations(
         (
-            lambda session, Address: session.query(Address),
+            lambda session, Address: session.query(Address).statement,
             lambda Address: {"mapper": inspect(Address), "clause": mock.ANY},
             "e2",
         ),
diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 80fb913ffc..def2fd6791 100644
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -137,6 +137,36 @@ class MiscTest(QueryTest):
             ).compare(q1.selectable)
         )
 
+    @testing.combinations(("session",), ("connection",), argnames="executor")
+    @testing.combinations(
+        ("execute",), ("scalars",), ("scalar",), argnames="method"
+    )
+    def test_no_query_in_execute(self, executor, method, connection):
+        # even though this test is testing deprecations, these deprecations
+        # become errors when removed so we dont want to remove this test,
+        # just update it
+
+        if executor == "session":
+            exec_obj = Session(connection)
+        else:
+            exec_obj = connection
+
+        meth = getattr(exec_obj, method)
+
+        q = Session().query(literal_column("1"))
+
+        if executor == "session":
+            with testing.expect_deprecated(
+                r"Object .*Query.* should not be used directly in a "
+                r"SQL statement context"
+            ):
+                meth(q)
+        else:
+            with testing.expect_raises_message(
+                sa_exc.ObjectNotExecutableError, "Not an executable object"
+            ):
+                meth(q)
+
 
 class OnlyReturnTuplesTest(QueryTest):
     def test_single_entity_false(self):
@@ -297,7 +327,10 @@ class RowTupleTest(QueryTest, AssertsCompiledSQL):
 
         q = testing.resolve_lambda(test_case, **locals())
 
-        row = s.execute(q.order_by(User.id)).first()
+        if isinstance(q, Query):
+            row = q.first()
+        else:
+            row = s.execute(q.order_by(User.id)).first()
         assert "jack" in row
 
     @testing.combinations(