Deprecate plain string passed to session.query()

Passing a plain string expression to :meth:`.Session.query` is deprecated,
as all string coercions were removed in :ticket:`4481` and this one should
have been included.   The :func:`.literal_column` function may be used to
produce a textual column expression.

Note this changeset is in the 1.3 branch only.  The 1.4 (currently master)
branch will already have removed this behavior.

Fixes: #4873
Change-Id: I20067a6707c7e2f028c67b1fbf0e8e4d9d136233

diff --git a/doc/build/changelog/unreleased_13/4873.rst b/doc/build/changelog/unreleased_13/4873.rst
new file mode 100644 (file)
index 0000000..758c363
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4873.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4873
+
+    Passing a plain string expression to :meth:`.Session.query` is deprecated,
+    as all string coercions were removed in :ticket:`4481` and this one should
+    have been included.   The :func:`.literal_column` function may be used to
+    produce a textual column expression.

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 08a085f86ed0f7e5f647499dcba083923b10b2be..5998f57758dd45f3700411b27d22829d59b820c9 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -3543,7 +3543,7 @@ class Query(object):
         # we get just "SELECT 1" without any entities.
         return sql.exists(
             self.enable_eagerloads(False)
-            .add_columns("1")
+            .add_columns(sql.literal_column("1"))
             .with_labels()
             .statement.with_only_columns([1])
         )
@@ -4477,6 +4477,12 @@ class _ColumnEntity(_QueryEntity):
         check_column = False
 
         if isinstance(column, util.string_types):
+            util.warn_deprecated(
+                "Plain string expression passed to Query() should be "
+                "explicitly declared using literal_column(); "
+                "automatic coercion of this value will be removed in "
+                "SQLAlchemy 1.4"
+            )
             column = sql.literal_column(column)
             self._label_name = column.name
             search_entities = False

diff --git a/test/orm/test_deprecations.py b/test/orm/test_deprecations.py
index 47653cc685d547cdce39dd29de9e10036a2d1588..b5609deb06903de207232b3d462055547fec511f 100644 (file)
--- a/test/orm/test_deprecations.py
+++ b/test/orm/test_deprecations.py
@@ -48,6 +48,7 @@ from sqlalchemy.testing.util import gc_collect
 from sqlalchemy.util.compat import pypy
 from . import _fixtures
 from .test_options import PathTest as OptionsPathTest
+from .test_query import QueryTest
 from .test_transaction import _LocalFixture
 
 
@@ -214,6 +215,73 @@ class DeprecationWarningsTest(fixtures.DeclarativeMappedTest):
             s.is_modified(f1, passive=True)
 
 
+class DeprecationQueryTest(QueryTest, AssertsCompiledSQL):
+    __dialect__ = "default"
+
+    def test_textual_query_column(self):
+        s = Session()
+
+        with assertions.expect_deprecated(
+            r"Plain string expression passed to Query\(\) should be "
+            "explicitly "
+        ):
+            self.assert_compile(s.query("1"), "SELECT 1")
+
+    def test_as_column(self):
+        User = self.classes.User
+
+        s = Session()
+        with assertions.expect_deprecated(
+            r"Plain string expression passed to Query\(\) should be "
+            "explicitly "
+        ):
+            eq_(
+                s.query(User.id, "name").order_by(User.id).all(),
+                [(7, "jack"), (8, "ed"), (9, "fred"), (10, "chuck")],
+            )
+
+    def test_raw_columns(self):
+        addresses, users, User = (
+            self.tables.addresses,
+            self.tables.users,
+            self.classes.User,
+        )
+
+        sess = create_session()
+        (user7, user8, user9, user10) = sess.query(User).all()
+        expected = [
+            (user7, 1, "Name:jack"),
+            (user8, 3, "Name:ed"),
+            (user9, 1, "Name:fred"),
+            (user10, 0, "Name:chuck"),
+        ]
+
+        # test with a straight statement
+        s = select(
+            [
+                users,
+                func.count(addresses.c.id).label("count"),
+                ("Name:" + users.c.name).label("concat"),
+            ],
+            from_obj=[users.outerjoin(addresses)],
+            group_by=[c for c in users.c],
+            order_by=[users.c.id],
+        )
+        q = create_session().query(User)
+
+        with assertions.expect_deprecated(
+            r"Plain string expression passed to Query\(\) should be "
+            "explicitly "
+        ):
+            result = (
+                q.add_column("count")
+                .add_column("concat")
+                .from_statement(s)
+                .all()
+            )
+        assert result == expected
+
+
 class DeprecatedAccountingFlagsTest(_LocalFixture):
     def test_rollback_no_accounting(self):
         User, users = self.classes.User, self.tables.users

diff --git a/test/orm/test_froms.py b/test/orm/test_froms.py
index 76cb4f6db6e24251e81dd3922c88aa1910312685..ec6b9a906ac3078b69f73567ade14497141705e9 100644 (file)
--- a/test/orm/test_froms.py
+++ b/test/orm/test_froms.py
@@ -2270,7 +2270,7 @@ class MixedEntitiesTest(QueryTest, AssertsCompiledSQL):
             .order_by(User.id)
         )
         q = sess.query(User)
-        result = q.add_column("count").from_statement(s).all()
+        result = q.add_column(literal_column("count")).from_statement(s).all()
         assert result == expected
 
     def test_raw_columns(self):
@@ -2315,7 +2315,10 @@ class MixedEntitiesTest(QueryTest, AssertsCompiledSQL):
         )
         q = create_session().query(User)
         result = (
-            q.add_column("count").add_column("concat").from_statement(s).all()
+            q.add_column(literal_column("count"))
+            .add_column(literal_column("concat"))
+            .from_statement(s)
+            .all()
         )
         assert result == expected
 

diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 04153c8b874c1d08a75ca169e615b4f883c14efd..9ba5ed3b5b75ff65dea221ae80a344420b01e7bc 100644 (file)
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -4160,12 +4160,14 @@ class TextTest(QueryTest, AssertsCompiledSQL):
         User = self.classes.User
 
         s = create_session()
+
+        # text() will work in 1.4
         assert_raises(
             sa_exc.InvalidRequestError, s.query, User.id, text("users.name")
         )
 
         eq_(
-            s.query(User.id, "name").order_by(User.id).all(),
+            s.query(User.id, literal_column("name")).order_by(User.id).all(),
             [(7, "jack"), (8, "ed"), (9, "fred"), (10, "chuck")],
         )
 

diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py
index 505debd76ffd61ba7dd57757c0ffa6c58320b057..8faefd3c9594b25ec6d1a4111e7e89f36b503212 100644 (file)
--- a/test/orm/test_subquery_relations.py
+++ b/test/orm/test_subquery_relations.py
@@ -3,6 +3,7 @@ from sqlalchemy import bindparam
 from sqlalchemy import ForeignKey
 from sqlalchemy import inspect
 from sqlalchemy import Integer
+from sqlalchemy import literal_column
 from sqlalchemy import select
 from sqlalchemy import String
 from sqlalchemy import testing
@@ -792,7 +793,7 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
         sess = create_session()
 
         self.assert_compile(
-            sess.query(User, "1"),
+            sess.query(User, literal_column("1")),
             "SELECT users.id AS users_id, users.name AS users_name, "
             "1 FROM users",
         )