From: Mike Bayer Date: Mon, 30 Sep 2019 14:54:50 +0000 (-0400) Subject: Deprecate plain string passed to session.query() X-Git-Tag: rel_1_3_9~7^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6acc785316bfe27326230d5010e5adee37b2f91d;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..758c3636f4 --- /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 08a085f86e..5998f57758 100644 --- 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 47653cc685..b5609deb06 100644 --- 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 76cb4f6db6..ec6b9a906a 100644 --- 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 04153c8b87..9ba5ed3b5b 100644 --- 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 505debd76f..8faefd3c95 100644 --- 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", )