Don't pass strings to Connection.execute()

Resolve the following RemovedIn20Warning warning:

  Passing a string to Connection.execute() is deprecated and will be
  removed in version 2.0.  Use the text() construct, or the
  Connection.exec_driver_sql() method to invoke a driver-level SQL
  string.

Change-Id: I1faa8c957649a04aa080518651045b432c6bd372
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

5 files changed, 66 insertions, 49 deletions

diff --git a/oslo_db/sqlalchemy/provision.py b/oslo_db/sqlalchemy/provision.py
index 199cba2..f891862 100644
--- a/oslo_db/sqlalchemy/provision.py
+++ b/oslo_db/sqlalchemy/provision.py
@@ -26,6 +26,7 @@ import string
 import sqlalchemy
 from sqlalchemy.engine import url as sa_url
 from sqlalchemy import schema
+from sqlalchemy import sql
 import testresources
 
 from oslo_db import exception
@@ -517,15 +518,16 @@ class MySQLBackendImpl(BackendImpl):
     def create_named_database(self, engine, ident, conditional=False):
         with engine.connect() as conn:
             if not conditional or not self.database_exists(conn, ident):
-                conn.execute("CREATE DATABASE %s" % ident)
+                conn.exec_driver_sql("CREATE DATABASE %s" % ident)
 
     def drop_named_database(self, engine, ident, conditional=False):
         with engine.connect() as conn:
             if not conditional or self.database_exists(conn, ident):
-                conn.execute("DROP DATABASE %s" % ident)
+                conn.exec_driver_sql("DROP DATABASE %s" % ident)
 
     def database_exists(self, engine, ident):
-        return bool(engine.scalar("SHOW DATABASES LIKE '%s'" % ident))
+        s = sql.text("SHOW DATABASES LIKE :ident")
+        return bool(engine.scalar(s, {'ident': ident}))
 
 
 @BackendImpl.impl.dispatch_for("sqlite")
@@ -582,29 +584,29 @@ class PostgresqlBackendImpl(BackendImpl):
         with engine.connect().execution_options(
                 isolation_level="AUTOCOMMIT") as conn:
             if not conditional or not self.database_exists(conn, ident):
-                conn.execute("CREATE DATABASE %s" % ident)
+                conn.exec_driver_sql("CREATE DATABASE %s" % ident)
 
     def drop_named_database(self, engine, ident, conditional=False):
         with engine.connect().execution_options(
                 isolation_level="AUTOCOMMIT") as conn:
             self._close_out_database_users(conn, ident)
             if conditional:
-                conn.execute("DROP DATABASE IF EXISTS %s" % ident)
+                conn.exec_driver_sql("DROP DATABASE IF EXISTS %s" % ident)
             else:
-                conn.execute("DROP DATABASE %s" % ident)
+                conn.exec_driver_sql("DROP DATABASE %s" % ident)
 
     def drop_additional_objects(self, conn):
         enums = [e['name'] for e in sqlalchemy.inspect(conn).get_enums()]
 
         for e in enums:
-            conn.execute("DROP TYPE %s" % e)
+            conn.exec_driver_sql("DROP TYPE %s" % e)
 
     def database_exists(self, engine, ident):
         return bool(
             engine.scalar(
                 sqlalchemy.text(
-                    "select datname from pg_database "
-                    "where datname=:name"), name=ident)
+                    "SELECT datname FROM pg_database "
+                    "WHERE datname=:name"), name=ident)
             )
 
     def _close_out_database_users(self, conn, ident):
@@ -624,11 +626,11 @@ class PostgresqlBackendImpl(BackendImpl):
         if conn.dialect.server_version_info >= (9, 2):
             conn.execute(
                 sqlalchemy.text(
-                    "select pg_terminate_backend(pid) "
-                    "from pg_stat_activity "
-                    "where usename=current_user and "
-                    "pid != pg_backend_pid() "
-                    "and datname=:dname"
+                    "SELECT pg_terminate_backend(pid) "
+                    "FROM pg_stat_activity "
+                    "WHERE usename=current_user AND "
+                    "pid != pg_backend_pid() AND "
+                    "datname=:dname"
                 ), dname=ident)
 
 
diff --git a/oslo_db/tests/fixtures.py b/oslo_db/tests/fixtures.py
index 00fdb98..5f1b299 100644
--- a/oslo_db/tests/fixtures.py
+++ b/oslo_db/tests/fixtures.py
@@ -104,11 +104,6 @@ class WarningsFixture(fixtures.Fixture):
 
         warnings.filterwarnings(
             'once',
-            message=r'Passing a string to Connection.execute\(\) .*',
-            category=sqla_exc.SADeprecationWarning)
-
-        warnings.filterwarnings(
-            'once',
             message=r'Using plain strings to indicate SQL statements .*',
             category=sqla_exc.SADeprecationWarning)
 
@@ -137,4 +132,15 @@ class WarningsFixture(fixtures.Fixture):
             message=r'The ``declarative_base\(\)`` function is now .*',
             category=sqla_exc.SADeprecationWarning)
 
+        # ...plus things that aren't our fault
+
+        # FIXME(stephenfin): These are caused by sqlalchemy-migrate, not us,
+        # and should be removed when we drop support for that library
+
+        warnings.filterwarnings(
+            'ignore',
+            message=r'Passing a string to Connection.execute\(\) .*',
+            module='migrate',
+            category=sqla_exc.SADeprecationWarning)
+
         self.addCleanup(warnings.resetwarnings)
diff --git a/oslo_db/tests/sqlalchemy/test_exc_filters.py b/oslo_db/tests/sqlalchemy/test_exc_filters.py
index d04e100..97f85e6 100644
--- a/oslo_db/tests/sqlalchemy/test_exc_filters.py
+++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py
@@ -24,6 +24,7 @@ from sqlalchemy import event
 import sqlalchemy.exc
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import mapper
+from sqlalchemy import sql
 
 from oslo_db import exception
 from oslo_db.sqlalchemy import engines
@@ -157,7 +158,7 @@ class TestsExceptionFilter(_SQLAExceptionMatcher, test_base.BaseTestCase):
         with self._fixture(dialect_name, raises, is_disconnect=is_disconnect):
             with self.engine.connect() as conn:
                 matched = self.assertRaises(
-                    expected, conn.execute, statement, params
+                    expected, conn.execute, sql.text(statement), params
                 )
                 return matched
 
@@ -487,7 +488,7 @@ class TestReferenceErrorSQLite(
         self.table_2.create(self.engine)
 
     def test_raise(self):
-        self.engine.execute("PRAGMA foreign_keys = ON;")
+        self.engine.execute(sql.text("PRAGMA foreign_keys = ON"))
 
         matched = self.assertRaises(
             exception.DBReferenceError,
@@ -509,7 +510,7 @@ class TestReferenceErrorSQLite(
         self.assertIsNone(matched.key_table)
 
     def test_raise_delete(self):
-        self.engine.execute("PRAGMA foreign_keys = ON;")
+        self.engine.execute(sql.text("PRAGMA foreign_keys = ON"))
 
         with self.engine.connect() as conn:
             conn.execute(self.table_1.insert({"id": 1234, "foo": 42}))
@@ -612,7 +613,7 @@ class TestReferenceErrorMySQL(
             conn.detach()  # will not be returned to the pool when closed
 
             # this is incompatible with some internals of the engine
-            conn.execute("SET SESSION sql_mode = 'ANSI';")
+            conn.execute(sql.text("SET SESSION sql_mode = 'ANSI';"))
 
             matched = self.assertRaises(
                 exception.DBReferenceError,
diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py
index b400249..f796368 100644
--- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py
+++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py
@@ -24,6 +24,7 @@ from unittest import mock
 import fixtures
 from oslo_config import cfg
 import sqlalchemy
+from sqlalchemy import sql
 from sqlalchemy import Column, MetaData, Table
 from sqlalchemy.engine import url
 from sqlalchemy import Integer, String
@@ -289,7 +290,7 @@ class MySQLDefaultModeTestCase(db_test_base._MySQLOpportunisticTestCase):
     def test_default_is_traditional(self):
         with self.engine.connect() as conn:
             sql_mode = conn.execute(
-                "SHOW VARIABLES LIKE 'sql_mode'"
+                sql.text("SHOW VARIABLES LIKE 'sql_mode'")
             ).first()[1]
 
         self.assertIn("TRADITIONAL", sql_mode)
@@ -479,14 +480,14 @@ class SQLiteConnectTest(test_base.BaseTestCase):
         engine = self._fixture(sqlite_fk=True)
         self.assertEqual(
             1,
-            engine.scalar("pragma foreign_keys")
+            engine.scalar(sql.text('pragma foreign_keys'))
         )
 
         engine = self._fixture(sqlite_fk=False)
 
         self.assertEqual(
             0,
-            engine.scalar("pragma foreign_keys")
+            engine.scalar(sql.text("pragma foreign_keys"))
         )
 
     def test_sqlite_synchronous_listener(self):
@@ -496,14 +497,14 @@ class SQLiteConnectTest(test_base.BaseTestCase):
         # http://www.sqlite.org/pragma.html#pragma_synchronous
         self.assertEqual(
             2,
-            engine.scalar("pragma synchronous")
+            engine.scalar(sql.text('pragma synchronous'))
         )
 
         engine = self._fixture(sqlite_synchronous=False)
 
         self.assertEqual(
             0,
-            engine.scalar("pragma synchronous")
+            engine.scalar(sql.text('pragma synchronous'))
         )
 
 
@@ -513,7 +514,9 @@ class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase):
         return session.create_engine(self.engine.url, mysql_sql_mode=sql_mode)
 
     def _assert_sql_mode(self, engine, sql_mode_present, sql_mode_non_present):
-        mode = engine.execute("SHOW VARIABLES LIKE 'sql_mode'").fetchone()[1]
+        mode = engine.execute(
+            sql.text("SHOW VARIABLES LIKE 'sql_mode'")
+        ).fetchone()[1]
         self.assertIn(
             sql_mode_present, mode
         )
@@ -538,7 +541,8 @@ class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase):
         # we get what is configured for the MySQL database, as opposed
         # to what our own session.create_engine() has set it to.
         expected = self.engine.execute(
-            "SELECT @@GLOBAL.sql_mode").scalar()
+            sql.text("SELECT @@GLOBAL.sql_mode")
+        ).scalar()
 
         engine = self._fixture(sql_mode=None)
         self._assert_sql_mode(engine, expected, None)
@@ -591,7 +595,8 @@ class MysqlConnectTest(db_test_base._MySQLOpportunisticTestCase):
         engine = self._fixture(sql_mode='TRADITIONAL')
 
         actual_mode = engine.execute(
-            "SHOW VARIABLES LIKE 'sql_mode'").fetchone()[1]
+            sql.text("SHOW VARIABLES LIKE 'sql_mode'")
+        ).fetchone()[1]
 
         self.assertIn('MySQL server mode set to %s' % actual_mode,
                       log.output)
@@ -832,7 +837,7 @@ class PatchStacktraceTest(db_test_base._DbTestCase):
             with mock.patch.object(engine.dialect, "do_execute") as mock_exec:
 
                 mock_exec.side_effect = orig_do_exec
-                conn.execute("select 1;")
+                conn.execute(sql.text("select 1"))
 
             call = mock_exec.mock_calls[0]
 
diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py
index 9f5f7f4..c63d52f 100644
--- a/oslo_db/tests/sqlalchemy/test_utils.py
+++ b/oslo_db/tests/sqlalchemy/test_utils.py
@@ -31,6 +31,7 @@ from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import mapper
 from sqlalchemy.orm import Session
 from sqlalchemy import PrimaryKeyConstraint
+from sqlalchemy import sql
 from sqlalchemy.sql.expression import cast
 from sqlalchemy.sql import select
 from sqlalchemy.types import UserDefinedType
@@ -933,14 +934,14 @@ class TestMigrationUtils(db_test_base._DbTestCase):
         # value here, 1.1 also coerces to "1/0" so use raw SQL to test the
         # constraint
         with self.engine.connect() as conn:
-            conn.execute(
+            conn.exec_driver_sql(
                 "INSERT INTO abc (deleted) VALUES (?)",
-                (10, )
+                (10, ),
             )
 
             self.assertEqual(
                 10,
-                conn.scalar("SELECT deleted FROM abc")
+                conn.scalar(sql.text("SELECT deleted FROM abc")),
             )
 
     def test_get_foreign_key_constraint_name(self):
@@ -1646,41 +1647,43 @@ class TestDialectFunctionDispatcher(test_base.BaseTestCase):
 class TestGetInnoDBTables(db_test_base._MySQLOpportunisticTestCase):
 
     def test_all_tables_use_innodb(self):
-        self.engine.execute("CREATE TABLE customers "
-                            "(a INT, b CHAR (20), INDEX (a)) ENGINE=InnoDB")
+        self.engine.execute(
+            sql.text(
+                "CREATE TABLE customers "
+                "(a INT, b CHAR (20), INDEX (a)) ENGINE=InnoDB"))
         self.assertEqual([], utils.get_non_innodb_tables(self.engine))
 
     def test_all_tables_use_innodb_false(self):
-        self.engine.execute("CREATE TABLE employee "
-                            "(i INT) ENGINE=MEMORY")
+        self.engine.execute(
+            sql.text("CREATE TABLE employee (i INT) ENGINE=MEMORY"))
         self.assertEqual(['employee'],
                          utils.get_non_innodb_tables(self.engine))
 
     def test_skip_tables_use_default_value(self):
-        self.engine.execute("CREATE TABLE migrate_version "
-                            "(i INT) ENGINE=MEMORY")
+        self.engine.execute(
+            sql.text("CREATE TABLE migrate_version (i INT) ENGINE=MEMORY"))
         self.assertEqual([],
                          utils.get_non_innodb_tables(self.engine))
 
     def test_skip_tables_use_passed_value(self):
-        self.engine.execute("CREATE TABLE some_table "
-                            "(i INT) ENGINE=MEMORY")
+        self.engine.execute(
+            sql.text("CREATE TABLE some_table (i INT) ENGINE=MEMORY"))
         self.assertEqual([],
                          utils.get_non_innodb_tables(
                              self.engine, skip_tables=('some_table',)))
 
     def test_skip_tables_use_empty_list(self):
-        self.engine.execute("CREATE TABLE some_table_3 "
-                            "(i INT) ENGINE=MEMORY")
+        self.engine.execute(
+            sql.text("CREATE TABLE some_table_3 (i INT) ENGINE=MEMORY"))
         self.assertEqual(['some_table_3'],
                          utils.get_non_innodb_tables(
                          self.engine, skip_tables=()))
 
     def test_skip_tables_use_several_values(self):
-        self.engine.execute("CREATE TABLE some_table_1 "
-                            "(i INT) ENGINE=MEMORY")
-        self.engine.execute("CREATE TABLE some_table_2 "
-                            "(i INT) ENGINE=MEMORY")
+        self.engine.execute(
+            sql.text("CREATE TABLE some_table_1 (i INT) ENGINE=MEMORY"))
+        self.engine.execute(
+            sql.text("CREATE TABLE some_table_2 (i INT) ENGINE=MEMORY"))
         self.assertEqual([],
                          utils.get_non_innodb_tables(
                              self.engine,