From: Mike Bayer Date: Wed, 9 Dec 2020 16:39:45 +0000 (-0500) Subject: Emit deprecation warnings for plain text under Session X-Git-Tag: rel_1_4_0b2~99^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4beecfb90;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Emit deprecation warnings for plain text under Session Deprecation warnings are emitted under "SQLALCHEMY_WARN_20" mode when passing a plain string to :meth:`_orm.Session.execute`. It was also considered to have DDL string expressions to include this as well, however this leaves us with no backwards-compatible way of handling reflection of elemens, such as an Index() which reflects "postgresql_where='x > 5'", there's no place for a rule that will turn those into text() within the reflection process that would be separate from when the user passes postgresql_where to the Index. Not worth it right now. Fixes: #5754 Change-Id: I8673a79f0e87de0df576b655f39dad0351725ca8 --- diff --git a/doc/build/changelog/unreleased_14/5754.rst b/doc/build/changelog/unreleased_14/5754.rst new file mode 100644 index 0000000000..3e0e938012 --- /dev/null +++ b/doc/build/changelog/unreleased_14/5754.rst @@ -0,0 +1,7 @@ +.. change:: + :tags: bug, sql + :tickets: 5754 + + Deprecation warnings are emitted under "SQLALCHEMY_WARN_20" mode when + passing a plain string to :meth:`_orm.Session.execute`. + diff --git a/lib/sqlalchemy/sql/coercions.py b/lib/sqlalchemy/sql/coercions.py index 9b3acf5ad7..bdd807438c 100644 --- a/lib/sqlalchemy/sql/coercions.py +++ b/lib/sqlalchemy/sql/coercions.py @@ -682,6 +682,10 @@ class DDLExpressionImpl(_Deannotate, _CoerceLiterals, RoleImpl): _coerce_consts = True def _text_coercion(self, element, argname=None): + # see #5754 for why we can't easily deprecate this coercion. + # essentially expressions like postgresql_where would have to be + # text() as they come back from reflection and we don't want to + # have text() elements wired into the inspection dictionaries. return elements.TextClause(element) @@ -769,21 +773,6 @@ class StatementImpl(_NoTextCoercion, RoleImpl): class CoerceTextStatementImpl(_CoerceLiterals, RoleImpl): __slots__ = () - def _dont_literal_coercion(self, element, **kw): - if callable(element) and hasattr(element, "__code__"): - return lambdas.StatementLambdaElement( - element, - self._role_class, - additional_cache_criteria=kw.get( - "additional_cache_criteria", () - ), - tracked=kw["tra"], - ) - else: - return super(CoerceTextStatementImpl, self)._literal_coercion( - element, **kw - ) - def _implicit_coercions( self, original_element, resolved, argname=None, **kw ): @@ -795,8 +784,12 @@ class CoerceTextStatementImpl(_CoerceLiterals, RoleImpl): ) def _text_coercion(self, element, argname=None): - # TODO: this should emit deprecation warning, - # see deprecation warning in engine/base.py execute() + util.warn_deprecated_20( + "Using plain strings to indicate SQL statements without using " + "the text() construct is " + "deprecated and will be removed in version 2.0. Ensure plain " + "SQL statements are passed using the text() construct." + ) return elements.TextClause(element) diff --git a/test/dialect/postgresql/test_compiler.py b/test/dialect/postgresql/test_compiler.py index 6dc782e8ef..1763b210b2 100644 --- a/test/dialect/postgresql/test_compiler.py +++ b/test/dialect/postgresql/test_compiler.py @@ -458,7 +458,7 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL): idx3 = Index( "test_idx2", tbl.c.data, - postgresql_where="data > 'a' AND data < 'b''s'", + postgresql_where=text("data > 'a' AND data < 'b''s'"), ) self.assert_compile( schema.CreateIndex(idx3), diff --git a/test/orm/test_backref_mutations.py b/test/orm/test_backref_mutations.py index 8647ddb785..c873f46c70 100644 --- a/test/orm/test_backref_mutations.py +++ b/test/orm/test_backref_mutations.py @@ -10,6 +10,7 @@ UPDATE in the database. """ from sqlalchemy import testing +from sqlalchemy import text from sqlalchemy.orm import attributes from sqlalchemy.orm import backref from sqlalchemy.orm import mapper @@ -727,7 +728,7 @@ class M2MCollectionMoveTest(_fixtures.FixtureTest): eq_(i1.keywords, [k1, k2]) # prove it didn't flush - eq_(session.scalar("select count(*) from item_keywords"), 1) + eq_(session.scalar(text("select count(*) from item_keywords")), 1) # the pending collection was removed assert ( diff --git a/test/orm/test_deprecations.py b/test/orm/test_deprecations.py index 4bd921af45..d5cfca83a8 100644 --- a/test/orm/test_deprecations.py +++ b/test/orm/test_deprecations.py @@ -2003,6 +2003,33 @@ class SessionTest(fixtures.RemovesEvents, _LocalFixture): ): s1.transaction + def test_textual_execute(self, connection): + """test that Session.execute() converts to text()""" + + users = self.tables.users + + with Session(bind=connection) as sess: + sess.execute(users.insert(), dict(id=7, name="jack")) + + with testing.expect_deprecated_20( + "Using plain strings to indicate SQL statements " + "without using the text" + ): + # use :bindparam style + eq_( + sess.execute( + "select * from users where id=:id", {"id": 7} + ).fetchall(), + [(7, "jack")], + ) + + with testing.expect_deprecated_20( + "Using plain strings to indicate SQL statements " + "without using the text" + ): + # use :bindparam style + eq_(sess.scalar("select id from users where id=:id", {"id": 7}), 7) + def test_session_str(self): s1 = Session(testing.db) str(s1) diff --git a/test/orm/test_manytomany.py b/test/orm/test_manytomany.py index 226415e34b..8b51d7e203 100644 --- a/test/orm/test_manytomany.py +++ b/test/orm/test_manytomany.py @@ -387,7 +387,7 @@ class M2MTest(fixtures.MappedTest): p1.place_id p1.transitions - sess.execute("delete from place_input", mapper=Place) + sess.execute(place_input.delete(), mapper=Place) p1.place_id = 7 assert_raises_message( @@ -400,7 +400,7 @@ class M2MTest(fixtures.MappedTest): p1.place_id p1.transitions - sess.execute("delete from place_input", mapper=Place) + sess.execute(place_input.delete(), mapper=Place) p1.transitions.remove(t1) assert_raises_message( orm_exc.StaleDataError, diff --git a/test/orm/test_session.py b/test/orm/test_session.py index d2838e5bf8..759d78ba52 100644 --- a/test/orm/test_session.py +++ b/test/orm/test_session.py @@ -9,6 +9,7 @@ from sqlalchemy import select from sqlalchemy import Sequence from sqlalchemy import String from sqlalchemy import testing +from sqlalchemy import text from sqlalchemy.orm import attributes from sqlalchemy.orm import backref from sqlalchemy.orm import close_all_sessions @@ -57,24 +58,6 @@ class ExecutionTest(_fixtures.FixtureTest): finally: seq.drop(connection) - def test_textual_execute(self, connection): - """test that Session.execute() converts to text()""" - - users = self.tables.users - - with Session(bind=connection) as sess: - sess.execute(users.insert(), dict(id=7, name="jack")) - - # use :bindparam style - eq_( - sess.execute( - "select * from users where id=:id", {"id": 7} - ).fetchall(), - [(7, "jack")], - ) - - # use :bindparam style - eq_(sess.scalar("select id from users where id=:id", {"id": 7}), 7) def test_parameter_execute(self): users = self.tables.users @@ -555,44 +538,39 @@ class SessionStateTest(_fixtures.FixtureTest): User, users = self.classes.User, self.tables.users mapper(User, users) - try: - sess = create_session(autocommit=False, autoflush=True) + with create_session(autocommit=False, autoflush=True) as sess: u = User() u.name = "ed" sess.add(u) u2 = sess.query(User).filter_by(name="ed").one() - assert u2 is u - assert ( + is_(u2, u) + eq_( sess.execute( - "select count(1) from users", + text("select count(1) from users"), bind_arguments=dict(mapper=User), - ).scalar() - == 1 + ).scalar(), + 1, ) - assert ( + eq_( testing.db.connect() .exec_driver_sql("select count(1) from users") - .scalar() - == 0 + .scalar(), + 0, ) sess.commit() - assert ( + eq_( sess.execute( - "select count(1) from users", + text("select count(1) from users"), bind_arguments=dict(mapper=User), - ).scalar() - == 1 + ).scalar(), + 1, ) - assert ( + eq_( testing.db.connect() .exec_driver_sql("select count(1) from users") - .scalar() - == 1 + .scalar(), + 1, ) - sess.close() - except Exception: - sess.rollback() - raise @engines.close_open_connections def test_autoflush_2(self): @@ -1938,11 +1916,15 @@ class SessionInterface(fixtures.TestBase): raises_("connection", bind_arguments=dict(mapper=user_arg)) - raises_("execute", "SELECT 1", bind_arguments=dict(mapper=user_arg)) + raises_( + "execute", text("SELECT 1"), bind_arguments=dict(mapper=user_arg) + ) raises_("get_bind", mapper=user_arg) - raises_("scalar", "SELECT 1", bind_arguments=dict(mapper=user_arg)) + raises_( + "scalar", text("SELECT 1"), bind_arguments=dict(mapper=user_arg) + ) eq_( watchdog, diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py index 248f334cf6..6eda6fbb68 100644 --- a/test/orm/test_transaction.py +++ b/test/orm/test_transaction.py @@ -11,6 +11,7 @@ from sqlalchemy import select from sqlalchemy import String from sqlalchemy import Table from sqlalchemy import testing +from sqlalchemy import text from sqlalchemy.future import Engine from sqlalchemy.orm import attributes from sqlalchemy.orm import create_session @@ -602,7 +603,9 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): s1 = Session(eng) - assert_raises_message(Exception, "failure", s1.execute, "select 1") + assert_raises_message( + Exception, "failure", s1.execute, text("select 1") + ) conn, fairy = state[0] assert not fairy.is_valid @@ -630,7 +633,9 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): s1 = Session(eng) s1.begin_nested() - assert_raises_message(Exception, "failure", s1.execute, "select 1") + assert_raises_message( + Exception, "failure", s1.execute, text("select 1") + ) conn, fairy = state[0] assert fairy.is_valid @@ -710,7 +715,7 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): @event.listens_for(sess, "after_commit") def go(session): - session.execute("select 1") + session.execute(text("select 1")) assert_raises_message( sa_exc.InvalidRequestError, @@ -729,7 +734,7 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): "This session is in 'prepared' state; no further " "SQL can be emitted within this transaction.", sess.execute, - "select 1", + text("select 1"), ) def test_no_sql_during_rollback(self): @@ -739,7 +744,7 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): @event.listens_for(sess, "after_rollback") def go(session): - session.execute("select 1") + session.execute(text("select 1")) assert_raises_message( sa_exc.InvalidRequestError, @@ -911,7 +916,7 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest): conn = mock.Mock(engine=bind) bind.connect = mock.Mock(return_value=conn) sess = Session(bind=bind) - sess.execute("select 1") + sess.execute(text("select 1")) with expect_warnings( "Connection is already established for the " "given bind; execution_options ignored" diff --git a/test/sql/test_roles.py b/test/sql/test_roles.py index 8759bbb22f..0ef90e89e9 100644 --- a/test/sql/test_roles.py +++ b/test/sql/test_roles.py @@ -173,11 +173,14 @@ class RoleTest(fixtures.TestBase): ) def test_statement_text_coercion(self): - is_true( - expect( - roles.CoerceTextStatementRole, "select * from table" - ).compare(text("select * from table")) - ) + with testing.expect_deprecated_20( + "Using plain strings to indicate SQL statements" + ): + is_true( + expect( + roles.CoerceTextStatementRole, "select * from table" + ).compare(text("select * from table")) + ) def test_select_statement_no_text_coercion(self): assert_raises_message(