bpo-28518: Start a transaction implicitly before a DML statement (#245)

Patch by Aviv Palivoda.

diff --git a/Lib/sqlite3/test/transactions.py b/Lib/sqlite3/test/transactions.py
index 45f1b04c69648f9d118b35f17c2038bc4051df0b..b8a13de55bc7209baadf2786ec10375e9e6e2b3f 100644 (file)
--- a/Lib/sqlite3/test/transactions.py
+++ b/Lib/sqlite3/test/transactions.py
@@ -179,6 +179,15 @@ class TransactionalDDL(unittest.TestCase):
         result = self.con.execute("select * from test").fetchall()
         self.assertEqual(result, [])
 
+    def CheckImmediateTransactionalDDL(self):
+        # You can achieve transactional DDL by issuing a BEGIN
+        # statement manually.
+        self.con.execute("begin immediate")
+        self.con.execute("create table test(i)")
+        self.con.rollback()
+        with self.assertRaises(sqlite.OperationalError):
+            self.con.execute("select * from test")
+
     def CheckTransactionalDDL(self):
         # You can achieve transactional DDL by issuing a BEGIN
         # statement manually.

diff --git a/Misc/NEWS b/Misc/NEWS
index 4413c511157fcd18a177c2e6152cbf24c0e0e5ec..8949ab4c75e7f6257c610cd0cb898a17140b78cf 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -249,6 +249,9 @@ Extension Modules
 Library
 -------
 
+- bpo-28518: Start a transaction implicitly before a DML statement.
+  Patch by Aviv Palivoda.
+
 - Issue #16285: urrlib.parse.quote is now based on RFC 3986 and hence includes
   '~' in the set of characters that is not quoted by default. Patch by
   Christian Theune and Ratnadeep Debnath.

diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c
index 39f7a6508c2aef1a013ba1270945d2ab079abb9b..8341fb8480172c946bfdc62c0c52dcac402d221f 100644 (file)
--- a/Modules/_sqlite/cursor.c
+++ b/Modules/_sqlite/cursor.c
@@ -511,10 +511,9 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
     pysqlite_statement_reset(self->statement);
     pysqlite_statement_mark_dirty(self->statement);
 
-    /* For backwards compatibility reasons, do not start a transaction if a
-       DDL statement is encountered.  If anybody wants transactional DDL,
-       they can issue a BEGIN statement manually. */
-    if (self->connection->begin_statement && !sqlite3_stmt_readonly(self->statement->st) && !self->statement->is_ddl) {
+    /* We start a transaction implicitly before a DML statement.
+       SELECT is the only exception. See #9924. */
+    if (self->connection->begin_statement && self->statement->is_dml) {
         if (sqlite3_get_autocommit(self->connection->db)) {
             result = _pysqlite_connection_begin(self->connection);
             if (!result) {
@@ -609,7 +608,7 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
             }
         }
 
-        if (!sqlite3_stmt_readonly(self->statement->st)) {
+        if (self->statement->is_dml) {
             self->rowcount += (long)sqlite3_changes(self->connection->db);
         } else {
             self->rowcount= -1L;

diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c
index 8b45a0332044ea0087ffa7dcd9fb42e9636a5571..27b2654b103419cc18c2a14eb94e8fa1fc752dd8 100644 (file)
--- a/Modules/_sqlite/statement.c
+++ b/Modules/_sqlite/statement.c
@@ -73,8 +73,9 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con
     Py_INCREF(sql);
     self->sql = sql;
 
-    /* determine if the statement is a DDL statement */
-    self->is_ddl = 0;
+    /* 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 ' ':
@@ -84,9 +85,10 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con
                 continue;
         }
 
-        self->is_ddl = (PyOS_strnicmp(p, "create ", 7) == 0)
-                    || (PyOS_strnicmp(p, "drop ", 5) == 0)
-                    || (PyOS_strnicmp(p, "reindex ", 8) == 0);
+        self->is_dml = (PyOS_strnicmp(p, "insert ", 7) == 0)
+                    || (PyOS_strnicmp(p, "update ", 7) == 0)
+                    || (PyOS_strnicmp(p, "delete ", 7) == 0)
+                    || (PyOS_strnicmp(p, "replace ", 8) == 0);
         break;
     }
 

diff --git a/Modules/_sqlite/statement.h b/Modules/_sqlite/statement.h
index 6eef16857f7b8596b8e15bd008ce06687b3377a1..8db10f6649ce80684fea005dd9b2bdb4f397386c 100644 (file)
--- a/Modules/_sqlite/statement.h
+++ b/Modules/_sqlite/statement.h
@@ -38,7 +38,7 @@ typedef struct
     sqlite3_stmt* st;
     PyObject* sql;
     int in_use;
-    int is_ddl;
+    int is_dml;
     PyObject* in_weakreflist; /* List of weak references */
 } pysqlite_Statement;