From c51aab0baa236e15dd01aba9633200106b37f1ad Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Wed, 8 May 2019 23:18:29 -0500 Subject: [PATCH 1/9] Use sqlite3_stmt_readonly API to determine is statement is DML. --- Lib/sqlite3/test/regression.py | 52 ++++++++++++++++++++++++ Modules/_sqlite/statement.c | 72 ++++++++++++++++++++++++---------- 2 files changed, 104 insertions(+), 20 deletions(-) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index 865bd88f74f10a..fe130dd76f9809 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -444,11 +444,63 @@ class UnhashableType(type): self.con.execute('SELECT %s()' % aggr_name) +class DMLStatementDetectionTestCase(unittest.TestCase): + """ + https://bugs.python.org/issue36859 + + Use sqlite3_stmt_readonly to determine if the 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)') + 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() + + def suite(): regression_suite = unittest.makeSuite(RegressionTests, "Check") return unittest.TestSuite(( regression_suite, unittest.makeSuite(UnhashableCallbacksTestCase), + unittest.makeSuite(DMLStatementDetectionTestCase), )) def test(): diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 575ac69d9030ef..9dbb8d328515ca 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -28,6 +28,10 @@ #include "prepare_protocol.h" #include "util.h" +#if SQLITE_VERSION_NUMBER >= 3007011 +#define HAVE_SQLITE3_STMT_READONLY +#endif + /* prototypes */ static int pysqlite_check_remaining_sql(const char* tail); @@ -48,13 +52,59 @@ typedef enum { TYPE_UNKNOWN } parameter_type; +int pysqlite_statement_is_dml(sqlite3_stmt *st, const char *sql) +{ + const char* p; + int is_dml = 0; + +#ifdef HAVE_SQLITE3_STMT_READONLY + is_dml = ! sqlite3_stmt_readonly(st); + if (is_dml) { + /* Retain backwards-compatibility, as sqlite3_stmt_readonly will return + * false for BEGIN [IMMEDIATE|EXCLUSIVE] or DDL statements. + */ + for (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)); + break; + } + } +#else + /* Original implementation. */ + for (p = sql; *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; + } +#endif + return is_dml; +} + int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* connection, PyObject* sql) { const char* tail; int rc; const char* sql_cstr; Py_ssize_t sql_cstr_len; - const char* p; self->st = NULL; self->in_use = 0; @@ -73,25 +123,6 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con Py_INCREF(sql); self->sql = sql; - /* Determine if the statement is a DML statement. - SELECT is the only exception. See #9924. */ - self->is_dml = 0; - for (p = sql_cstr; *p != 0; p++) { - switch (*p) { - case ' ': - case '\r': - case '\n': - case '\t': - continue; - } - - self->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; - } - Py_BEGIN_ALLOW_THREADS rc = sqlite3_prepare_v2(connection->db, sql_cstr, @@ -101,6 +132,7 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con Py_END_ALLOW_THREADS self->db = connection->db; + self->is_dml = pysqlite_statement_is_dml(self->st, sql_cstr); if (rc == SQLITE_OK && pysqlite_check_remaining_sql(tail)) { (void)sqlite3_finalize(self->st); From aa42615e137da81f22160e167b20813192e37993 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Wed, 8 May 2019 23:45:56 -0500 Subject: [PATCH 2/9] Skip dml detection + sql comment for old sqlite. --- Lib/sqlite3/test/regression.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index fe130dd76f9809..213a03b9002628 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -478,6 +478,8 @@ def test_dml_detection_cte(self): curs = conn.execute('select key, val from kv order by key') self.assertEqual(curs.fetchall(), [('k1', 111), ('k2', 112)]) + @unittest.skipIf(sqlite.sqlite_version_info < (3, 7, 11), + 'needs sqlite 3.7.11 or newer') def test_dml_detection_sql_comment(self): conn = sqlite.connect(':memory:') conn.execute('create table kv ("key" text, "val" integer)') From 56dc71bf71640787f13be190039a0c656fc61092 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Thu, 9 May 2019 09:58:09 -0500 Subject: [PATCH 3/9] Add news item. --- .../NEWS.d/next/Library/2019-05-09-09-54-00.bpo-36859.CuCkEd.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2019-05-09-09-54-00.bpo-36859.CuCkEd.rst 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..3a9126c722f69c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-05-09-09-54-00.bpo-36859.CuCkEd.rst @@ -0,0 +1 @@ +Use `sqlite3_stmt_readonly` internally to determine if a SQL statement is data-modifying. From d54ce97f34d2a6c59a8997d602cc8f1e15e8def1 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Thu, 9 May 2019 11:03:57 -0500 Subject: [PATCH 4/9] Address review comments, additional regression test. --- Lib/sqlite3/test/regression.py | 16 +++++++++++++--- .../2019-05-09-09-54-00.bpo-36859.CuCkEd.rst | 4 +++- Modules/_sqlite/statement.c | 7 +++++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index 213a03b9002628..ef230e8303742a 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -446,9 +446,8 @@ class UnhashableType(type): class DMLStatementDetectionTestCase(unittest.TestCase): """ - https://bugs.python.org/issue36859 - - Use sqlite3_stmt_readonly to determine if the statement is DML or not. + 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') @@ -496,6 +495,17 @@ def test_dml_detection_sql_comment(self): self.assertEqual(curs.fetchall(), [('k1', 11), ('k2', 12)]) conn.rollback() + 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 suite(): regression_suite = unittest.makeSuite(RegressionTests, "Check") 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 index 3a9126c722f69c..4a320dadb3b5ab 100644 --- 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 @@ -1 +1,3 @@ -Use `sqlite3_stmt_readonly` internally to determine if a SQL statement is data-modifying. +Use `sqlite3_stmt_readonly()` internally to determine if a SQL statement is +data-modifying. Requires sqlite3 3.7.11 or newer. Patch contributed by Charles +Leifer. diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 9dbb8d328515ca..5118f0e9792614 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -58,7 +58,7 @@ int pysqlite_statement_is_dml(sqlite3_stmt *st, const char *sql) int is_dml = 0; #ifdef HAVE_SQLITE3_STMT_READONLY - is_dml = ! sqlite3_stmt_readonly(st); + is_dml = !sqlite3_stmt_readonly(st); if (is_dml) { /* Retain backwards-compatibility, as sqlite3_stmt_readonly will return * false for BEGIN [IMMEDIATE|EXCLUSIVE] or DDL statements. @@ -79,7 +79,10 @@ int pysqlite_statement_is_dml(sqlite3_stmt *st, const char *sql) } } #else - /* Original implementation. */ + /* Determine if the statement is a DML statement. SELECT is the only + * exception. This is a fallback for older versions of SQLite which do not + * support the sqlite3_stmt_readonly() API. + */ for (p = sql; *p != 0; p++) { switch (*p) { case ' ': From afb7b6cb3e09e0a46cdc97a75f11653373e2a3da Mon Sep 17 00:00:00 2001 From: Berker Peksag Date: Thu, 9 May 2019 21:58:56 +0300 Subject: [PATCH 5/9] Fix rest markup --- .../next/Library/2019-05-09-09-54-00.bpo-36859.CuCkEd.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 index 4a320dadb3b5ab..587d516a953010 100644 --- 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 @@ -1,3 +1,2 @@ -Use `sqlite3_stmt_readonly()` internally to determine if a SQL statement is -data-modifying. Requires sqlite3 3.7.11 or newer. Patch contributed by Charles -Leifer. +Use ``sqlite3_stmt_readonly()`` internally to determine if a SQL statement is +data-modifying. Requires sqlite3 3.7.11 or newer. Patch by Charles Leifer. From cc49a97449ad7fb6475e5af2da5681690e5b619f Mon Sep 17 00:00:00 2001 From: Berker Peksag Date: Thu, 9 May 2019 23:19:57 +0300 Subject: [PATCH 6/9] cosmetic fixes --- Lib/sqlite3/test/regression.py | 42 +++++++++++++++++++--------------- Modules/_sqlite/statement.c | 10 ++++---- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index ef230e8303742a..af09f0f49c5ce9 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -449,51 +449,55 @@ 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') + @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)') + conn.execute('CREATE TABLE kv ("key" TEXT, "val" INTEGER)') self.assertFalse(conn.in_transaction) - conn.execute('insert into kv (key, val) values (?, ?), (?, ?)', + 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,)) + 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,)) + 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') + curs = conn.execute('SELECT key, val FROM kv ORDER BY key') self.assertEqual(curs.fetchall(), [('k1', 111), ('k2', 112)]) - @unittest.skipIf(sqlite.sqlite_version_info < (3, 7, 11), - 'needs sqlite 3.7.11 or newer') + @unittest.skipIf(sqlite.sqlite_version_info < (3, 7, 11), 'needs sqlite 3.7.11 or newer') def test_dml_detection_sql_comment(self): conn = sqlite.connect(':memory:') - conn.execute('create table kv ("key" text, "val" integer)') - conn.execute('insert into kv (key, val) values (?, ?), (?, ?)', + 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,)) + + 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') + 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 @@ -501,9 +505,9 @@ def test_dml_detection_begin_exclusive(self): # transactional behavior, we add a special exclusion for these # statements. conn = sqlite.connect(':memory:') - conn.execute('begin exclusive') + conn.execute('BEGIN EXCLUSIVE') self.assertTrue(conn.in_transaction) - conn.execute('rollback') + conn.execute('ROLLBACK') self.assertFalse(conn.in_transaction) diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 5118f0e9792614..f7b962724881fb 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -52,17 +52,16 @@ typedef enum { TYPE_UNKNOWN } parameter_type; -int pysqlite_statement_is_dml(sqlite3_stmt *st, const char *sql) +static int pysqlite_statement_is_dml(sqlite3_stmt *statement, const char *sql) { const char* p; int is_dml = 0; #ifdef HAVE_SQLITE3_STMT_READONLY - is_dml = !sqlite3_stmt_readonly(st); + 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. - */ + * false for BEGIN [IMMEDIATE|EXCLUSIVE] or DDL statements. */ for (p = sql; *p != 0; p++) { switch (*p) { case ' ': @@ -81,8 +80,7 @@ int pysqlite_statement_is_dml(sqlite3_stmt *st, const char *sql) #else /* Determine if the statement is a DML statement. SELECT is the only * exception. This is a fallback for older versions of SQLite which do not - * support the sqlite3_stmt_readonly() API. - */ + * support the sqlite3_stmt_readonly() API. */ for (p = sql; *p != 0; p++) { switch (*p) { case ' ': From 8b1cceda1f92723f749da4e79632f163859416e5 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Thu, 9 May 2019 15:55:32 -0500 Subject: [PATCH 7/9] Ensure all non-DML statements that write are covered. --- Lib/sqlite3/test/regression.py | 5 +++++ Modules/_sqlite/statement.c | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index af09f0f49c5ce9..d6f39016323863 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -510,6 +510,11 @@ def test_dml_detection_begin_exclusive(self): 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(): regression_suite = unittest.makeSuite(RegressionTests, "Check") diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index f7b962724881fb..597aba0a93683a 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -73,7 +73,11 @@ static int pysqlite_statement_is_dml(sqlite3_stmt *statement, const char *sql) is_dml = (PyOS_strnicmp(p, "begin", 5) && PyOS_strnicmp(p, "create", 6) && - PyOS_strnicmp(p, "drop", 4)); + 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; } } From 3ab2ba86f55426374f2781654d34317882025240 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Fri, 10 May 2019 08:57:06 -0500 Subject: [PATCH 8/9] Move DML detection tests into transactions test module. Also add additional assertion covering ALTER statements. --- Lib/sqlite3/test/regression.py | 73 ---------------------------- Lib/sqlite3/test/transactions.py | 81 +++++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 74 deletions(-) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index d6f39016323863..865bd88f74f10a 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -444,84 +444,11 @@ class UnhashableType(type): self.con.execute('SELECT %s()' % aggr_name) -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)]) - - @unittest.skipIf(sqlite.sqlite_version_info < (3, 7, 11), 'needs sqlite 3.7.11 or newer') - 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(): regression_suite = unittest.makeSuite(RegressionTests, "Check") return unittest.TestSuite(( regression_suite, unittest.makeSuite(UnhashableCallbacksTestCase), - unittest.makeSuite(DMLStatementDetectionTestCase), )) def test(): diff --git a/Lib/sqlite3/test/transactions.py b/Lib/sqlite3/test/transactions.py index b8a13de55bc720..61f6331e312c91 100644 --- a/Lib/sqlite3/test/transactions.py +++ b/Lib/sqlite3/test/transactions.py @@ -179,6 +179,11 @@ def CheckDdlDoesNotAutostartTransaction(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 CheckImmediateTransactionalDDL(self): # You can achieve transactional DDL by issuing a BEGIN # statement manually. @@ -200,11 +205,85 @@ def CheckTransactionalDDL(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)]) + + @unittest.skipIf(sqlite.sqlite_version_info < (3, 7, 11), 'needs sqlite 3.7.11 or newer') + 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(): default_suite = unittest.makeSuite(TransactionTests, "Check") special_command_suite = unittest.makeSuite(SpecialCommandTests, "Check") ddl_suite = unittest.makeSuite(TransactionalDDL, "Check") - return unittest.TestSuite((default_suite, special_command_suite, ddl_suite)) + dml_suite = unittest.makeSuite(DMLStatementDetectionTestCase) + return unittest.TestSuite((default_suite, special_command_suite, ddl_suite, dml_suite)) def test(): runner = unittest.TextTestRunner() From 6667caa2903b7c531d1846f9db6e19a20432b913 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Sat, 11 May 2019 18:00:23 -0500 Subject: [PATCH 9/9] Fix version-check for sqlite3_stmt_readonly availability. Feature added in sqlite 3.7.4, not .11. --- Modules/_sqlite/statement.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 597aba0a93683a..155eac596b8e5a 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -28,7 +28,7 @@ #include "prepare_protocol.h" #include "util.h" -#if SQLITE_VERSION_NUMBER >= 3007011 +#if SQLITE_VERSION_NUMBER >= 3007004 #define HAVE_SQLITE3_STMT_READONLY #endif