From 11e982936fb015be47e0e9fc5665e5dbefa44257 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Wed, 8 May 2019 23:18:29 -0500 Subject: [PATCH 01/12] Use sqlite3_stmt_readonly API to determine is statement is DML. --- Lib/sqlite3/test/regression.py | 54 ++++++++++++++++++++++++- Modules/_sqlite/statement.c | 72 ++++++++++++++++++++++++---------- 2 files changed, 105 insertions(+), 21 deletions(-) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index c8e0b27564ad10..894cb5ecc47df3 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -411,9 +411,61 @@ def log(self, *args): +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(): tests = [ - RegressionTests + DMLStatementDetectionTestCase, + RegressionTests, ] return unittest.TestSuite( [unittest.TestLoader().loadTestsFromTestCase(t) for t in tests] diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 57026270e1eeb5..aea31eeda49978 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; @@ -74,25 +124,6 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con self->in_weakreflist = NULL; self->sql = Py_NewRef(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, @@ -102,6 +133,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 29a7e2056aeb6cca710b8c88be495dd979884c07 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Wed, 8 May 2019 23:45:56 -0500 Subject: [PATCH 02/12] 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 894cb5ecc47df3..f6f709fe5a7ac9 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -445,6 +445,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 f20d41b7ccbc8828794f719fc7bea0a4fe6facdf Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Thu, 9 May 2019 09:58:09 -0500 Subject: [PATCH 03/12] 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 868b97a55215c4c1968d4509b8c53b77a2982dd2 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Thu, 9 May 2019 11:03:57 -0500 Subject: [PATCH 04/12] 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 f6f709fe5a7ac9..b9d77c8d0282b7 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -413,9 +413,8 @@ def log(self, *args): 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') @@ -463,6 +462,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(): tests = [ 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 aea31eeda49978..27fb11ff3040a0 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 a13fb7136ea145f19bcaa9dd3d6a6f4f1302d9d6 Mon Sep 17 00:00:00 2001 From: Berker Peksag Date: Thu, 9 May 2019 21:58:56 +0300 Subject: [PATCH 05/12] 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 6e1cf0669400420c3b7db393e3534e689b4654c1 Mon Sep 17 00:00:00 2001 From: Berker Peksag Date: Thu, 9 May 2019 23:19:57 +0300 Subject: [PATCH 06/12] 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 b9d77c8d0282b7..a6dd93dfc2ace8 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -416,51 +416,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 @@ -468,9 +472,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 27fb11ff3040a0..789bb4c9f3e931 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 013e43b4ffd680a099316bd638f138cb04be56a0 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Thu, 9 May 2019 15:55:32 -0500 Subject: [PATCH 07/12] 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 a6dd93dfc2ace8..82562f738cbbc5 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -477,6 +477,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(): tests = [ diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 789bb4c9f3e931..a14713c991afd9 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 6e6f2a02eda06efa5e2a77925bb1f79b2ebf4e7a Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Fri, 10 May 2019 08:57:06 -0500 Subject: [PATCH 08/12] Move DML detection tests into transactions test module. Also add additional assertion covering ALTER statements. --- Lib/sqlite3/test/regression.py | 75 +----------------------------- Lib/sqlite3/test/transactions.py | 78 ++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 74 deletions(-) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index 82562f738cbbc5..c8e0b27564ad10 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -411,82 +411,9 @@ def log(self, *args): -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(): tests = [ - DMLStatementDetectionTestCase, - RegressionTests, + RegressionTests ] return unittest.TestSuite( [unittest.TestLoader().loadTestsFromTestCase(t) for t in tests] 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, From 09cac6c3953060af25df002cc1168603c5b65280 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Sat, 11 May 2019 18:00:23 -0500 Subject: [PATCH 09/12] 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 a14713c991afd9..14dee1bd60e7ae 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 From d1efa55e0b8630715219161b627f6f928f635da5 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 9 Feb 2021 14:44:46 +0100 Subject: [PATCH 10/12] Sync with GH-24106 --- .../2019-05-09-09-54-00.bpo-36859.CuCkEd.rst | 2 +- Modules/_sqlite/statement.c | 25 ------------------- 2 files changed, 1 insertion(+), 26 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 587d516a953010..93d267b2da5c6a 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,2 +1,2 @@ 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. +data-modifying. Patch by Charles Leifer. diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 14dee1bd60e7ae..29eacb6bff379c 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -28,10 +28,6 @@ #include "prepare_protocol.h" #include "util.h" -#if SQLITE_VERSION_NUMBER >= 3007004 -#define HAVE_SQLITE3_STMT_READONLY -#endif - /* prototypes */ static int pysqlite_check_remaining_sql(const char* tail); @@ -57,7 +53,6 @@ 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(statement); if (is_dml) { /* Retain backwards-compatibility, as sqlite3_stmt_readonly will return @@ -81,26 +76,6 @@ static int pysqlite_statement_is_dml(sqlite3_stmt *statement, const char *sql) break; } } -#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. */ - 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; } From dce7ada0f8b5917de854b4397f9f5a42b76a4a1d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sat, 5 Jun 2021 21:02:17 +0200 Subject: [PATCH 11/12] PEP 7 --- Modules/_sqlite/statement.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 91cbe64d3c3831..78ed0eb16c9beb 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -48,16 +48,15 @@ typedef enum { TYPE_UNKNOWN } parameter_type; -static int pysqlite_statement_is_dml(sqlite3_stmt *statement, const char *sql) +static int +pysqlite_statement_is_dml(sqlite3_stmt *statement, const char *sql) { - const char* p; - int is_dml = 0; + int is_dml = !sqlite3_stmt_readonly(statement); - 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 (p = sql; *p != 0; p++) { + for (const char *p = sql; *p != 0; p++) { switch (*p) { case ' ': case '\r': From 6eb6ec98402efc2a3b7441c4a32d5756224c824a Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 8 Jun 2021 22:36:50 +0200 Subject: [PATCH 12/12] Improve namespace --- Modules/_sqlite/statement.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 97d5f0b324815a..99465ed9998345 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -49,7 +49,7 @@ typedef enum { } parameter_type; static int -pysqlite_statement_is_dml(sqlite3_stmt *statement, const char *sql) +statement_is_dml(sqlite3_stmt *statement, const char *sql) { int is_dml = !sqlite3_stmt_readonly(statement); @@ -129,7 +129,7 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) self->st = stmt; self->in_use = 0; - self->is_dml = pysqlite_statement_is_dml(stmt, sql_cstr); + self->is_dml = statement_is_dml(stmt, sql_cstr); self->in_weakreflist = NULL; PyObject_GC_Track(self);