diff --git a/Lib/sqlite3/test/transactions.py b/Lib/sqlite3/test/transactions.py index 80284902a1a6e9..05a09e66ccd830 100644 --- a/Lib/sqlite3/test/transactions.py +++ b/Lib/sqlite3/test/transactions.py @@ -174,6 +174,11 @@ def test_ddl_does_not_autostart_transaction(self): result = self.con.execute("select * from test").fetchall() self.assertEqual(result, []) + self.con.execute("alter table test rename to test2") + self.con.rollback() + result = self.con.execute("select * from test2").fetchall() + self.assertEqual(result, []) + def test_immediate_transactional_ddl(self): # You can achieve transactional DDL by issuing a BEGIN # statement manually. @@ -195,8 +200,81 @@ def test_transactional_ddl(self): def tearDown(self): self.con.close() + +class DMLStatementDetectionTestCase(unittest.TestCase): + """ + Test behavior of sqlite3_stmt_readonly() in determining if a statement is + DML or not. + """ + @unittest.skipIf(sqlite.sqlite_version_info < (3, 8, 3), 'needs sqlite 3.8.3 or newer') + def test_dml_detection_cte(self): + conn = sqlite.connect(':memory:') + conn.execute('CREATE TABLE kv ("key" TEXT, "val" INTEGER)') + self.assertFalse(conn.in_transaction) + conn.execute('INSERT INTO kv (key, val) VALUES (?, ?), (?, ?)', + ('k1', 1, 'k2', 2)) + self.assertTrue(conn.in_transaction) + conn.commit() + self.assertFalse(conn.in_transaction) + + rc = conn.execute('UPDATE kv SET val=val + ?', (10,)) + self.assertEqual(rc.rowcount, 2) + self.assertTrue(conn.in_transaction) + conn.commit() + self.assertFalse(conn.in_transaction) + + rc = conn.execute( + 'WITH c(k, v) AS (SELECT key, val + ? FROM kv) ' + 'UPDATE kv SET val=(SELECT v FROM c WHERE k=kv.key)', + (100,) + ) + self.assertEqual(rc.rowcount, 2) + self.assertTrue(conn.in_transaction) + + curs = conn.execute('SELECT key, val FROM kv ORDER BY key') + self.assertEqual(curs.fetchall(), [('k1', 111), ('k2', 112)]) + + def test_dml_detection_sql_comment(self): + conn = sqlite.connect(':memory:') + conn.execute('CREATE TABLE kv ("key" TEXT, "val" INTEGER)') + self.assertFalse(conn.in_transaction) + conn.execute('INSERT INTO kv (key, val) VALUES (?, ?), (?, ?)', + ('k1', 1, 'k2', 2)) + conn.commit() + self.assertFalse(conn.in_transaction) + + rc = conn.execute('-- a comment\nUPDATE kv SET val=val + ?', (10,)) + self.assertEqual(rc.rowcount, 2) + self.assertTrue(conn.in_transaction) + + curs = conn.execute('SELECT key, val FROM kv ORDER BY key') + self.assertEqual(curs.fetchall(), [('k1', 11), ('k2', 12)]) + conn.rollback() + self.assertFalse(conn.in_transaction) + # Fetch again after rollback. + curs = conn.execute('SELECT key, val FROM kv ORDER BY key') + self.assertEqual(curs.fetchall(), [('k1', 1), ('k2', 2)]) + + def test_dml_detection_begin_exclusive(self): + # sqlite3_stmt_readonly() reports BEGIN EXCLUSIVE as being a + # non-read-only statement. To retain compatibility with the + # transactional behavior, we add a special exclusion for these + # statements. + conn = sqlite.connect(':memory:') + conn.execute('BEGIN EXCLUSIVE') + self.assertTrue(conn.in_transaction) + conn.execute('ROLLBACK') + self.assertFalse(conn.in_transaction) + + def test_dml_detection_vacuum(self): + conn = sqlite.connect(':memory:') + conn.execute('vacuum') + self.assertFalse(conn.in_transaction) + + def suite(): tests = [ + DMLStatementDetectionTestCase, SpecialCommandTests, TransactionTests, TransactionalDDL, diff --git a/Misc/NEWS.d/next/Library/2019-05-09-09-54-00.bpo-36859.CuCkEd.rst b/Misc/NEWS.d/next/Library/2019-05-09-09-54-00.bpo-36859.CuCkEd.rst new file mode 100644 index 00000000000000..93d267b2da5c6a --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-05-09-09-54-00.bpo-36859.CuCkEd.rst @@ -0,0 +1,2 @@ +Use ``sqlite3_stmt_readonly()`` internally to determine if a SQL statement is +data-modifying. Patch by Charles Leifer. diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 89fe4bbec3ea43..6e0b504c2d3db5 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -48,6 +48,36 @@ typedef enum { TYPE_UNKNOWN } parameter_type; +static int +statement_is_dml(sqlite3_stmt *statement, const char *sql) +{ + int is_dml = !sqlite3_stmt_readonly(statement); + + if (is_dml) { + /* Retain backwards-compatibility, as sqlite3_stmt_readonly will return + * false for BEGIN [IMMEDIATE|EXCLUSIVE] or DDL statements. */ + for (const char *p = sql; *p != 0; p++) { + switch (*p) { + case ' ': + case '\r': + case '\n': + case '\t': + continue; + } + + is_dml = (PyOS_strnicmp(p, "begin", 5) && + PyOS_strnicmp(p, "create", 6) && + PyOS_strnicmp(p, "drop", 4) && + PyOS_strnicmp(p, "alter", 5) && + PyOS_strnicmp(p, "analyze", 7) && + PyOS_strnicmp(p, "reindex", 7) && + PyOS_strnicmp(p, "vacuum", 6)); + break; + } + } + return is_dml; +} + pysqlite_Statement * pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) { @@ -91,24 +121,6 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) goto error; } - /* Determine if the statement is a DML statement. - SELECT is the only exception. See #9924. */ - int is_dml = 0; - for (const char *p = sql_cstr; *p != 0; p++) { - switch (*p) { - case ' ': - case '\r': - case '\n': - case '\t': - continue; - } - - is_dml = (PyOS_strnicmp(p, "insert", 6) == 0) - || (PyOS_strnicmp(p, "update", 6) == 0) - || (PyOS_strnicmp(p, "delete", 6) == 0) - || (PyOS_strnicmp(p, "replace", 7) == 0); - break; - } pysqlite_state *state = pysqlite_get_state(NULL); pysqlite_Statement *self = PyObject_GC_New(pysqlite_Statement, @@ -119,7 +131,7 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) self->st = stmt; self->in_use = 0; - self->is_dml = is_dml; + self->is_dml = statement_is_dml(stmt, sql_cstr); self->in_weakreflist = NULL; PyObject_GC_Track(self);