Closes #59.

ghaering · ghaering · commit 94eae5002967 · 2015-08-19T00:02:50.000+02:00
Closes #21. Fixes #24. Closes #80. Completety got rid of statement parsing. We now use SQLite functions to determine if a statement modifies the database or not. If a statement modifies the database, then we implicitly start a transaction. This also means that DDL statements like CREATE TABLE now start transactions. This is not 100% backwards compatible, but the fix is easy: add commit() calls to your code after DDL statements. On older pysqlite releases and when using the sqlite3 module from the standard library, adding the commit() doesn't hurt.
diff --git a/lib/test/dbapi.py b/lib/test/dbapi.py
@@ -88,6 +88,7 @@ def setUp(self):
         self.cx = sqlite.connect(":memory:")
         cu = self.cx.cursor()
         cu.execute("create table test(id integer primary key, name text)")
+        self.cx.commit()
         cu.execute("insert into test(name) values (?)", ("foo",))
 
     def tearDown(self):
@@ -145,6 +146,7 @@ def setUp(self):
         self.cx = sqlite.connect(":memory:")
         self.cu = self.cx.cursor()
         self.cu.execute("create table test(id integer primary key, name text, income number)")
+        self.cx.commit()
         self.cu.execute("insert into test(name) values (?)", ("foo",))
 
     def tearDown(self):
@@ -476,6 +478,7 @@ def setUp(self):
         self.con = sqlite.connect(":memory:")
         self.cur = self.con.cursor()
         self.cur.execute("create table test(id integer primary key, name text, bin binary, ratio number, ts timestamp)")
+        self.con.commit()
 
     def tearDown(self):
         self.cur.close()
@@ -706,6 +709,7 @@ def CheckConnectionExecute(self):
     def CheckConnectionExecutemany(self):
         con = sqlite.connect(":memory:")
         con.execute("create table test(foo)")
+        con.commit()
         con.executemany("insert into test(foo) values (?)", [(3,), (4,)])
         result = con.execute("select foo from test order by foo").fetchall()
         self.assertEqual(result[0][0], 3, "Basic test of Connection.executemany")
@@ -878,6 +882,7 @@ def setUp(self):
         global did_rollback
         self.con = sqlite.connect(":memory:", factory=MyConnection)
         self.con.execute("create table test(c unique)")
+        self.con.commit()
         did_rollback = False
 
     def tearDown(self):
diff --git a/lib/test/factory.py b/lib/test/factory.py
@@ -204,6 +204,7 @@ class TextFactoryTestsWithEmbeddedZeroBytes(unittest.TestCase):
     def setUp(self):
         self.con = sqlite.connect(":memory:")
         self.con.execute("create table test (value text)")
+        self.con.commit()
         self.con.execute("insert into test (value) values (?)", ("a\x00b",))
 
     def CheckString(self):
diff --git a/lib/test/hooks.py b/lib/test/hooks.py
@@ -136,6 +136,7 @@ def progress():
         con.execute("""
             create table foo(a, b)
             """)
+        con.commit()
         self.assertTrue(progress_calls)
 
 
diff --git a/lib/test/transactions.py b/lib/test/transactions.py
@@ -53,23 +53,17 @@ def tearDown(self):
         except OSError:
             pass
 
-    def CheckDMLdoesAutoCommitBefore(self):
-        self.cur1.execute("create table test(i)")
-        self.cur1.execute("insert into test(i) values (5)")
-        self.cur1.execute("create table test2(j)")
-        self.cur2.execute("select i from test")
-        res = self.cur2.fetchall()
-        self.assertEqual(len(res), 1)
-
     def CheckInsertStartsTransaction(self):
         self.cur1.execute("create table test(i)")
+        self.con1.commit()
         self.cur1.execute("insert into test(i) values (5)")
         self.cur2.execute("select i from test")
         res = self.cur2.fetchall()
         self.assertEqual(len(res), 0)
 
     def CheckUpdateStartsTransaction(self):
         self.cur1.execute("create table test(i)")
+        self.con1.commit()
         self.cur1.execute("insert into test(i) values (5)")
         self.con1.commit()
         self.cur1.execute("update test set i=6")
@@ -79,6 +73,7 @@ def CheckUpdateStartsTransaction(self):
 
     def CheckDeleteStartsTransaction(self):
         self.cur1.execute("create table test(i)")
+        self.con1.commit()
         self.cur1.execute("insert into test(i) values (5)")
         self.con1.commit()
         self.cur1.execute("delete from test")
@@ -88,6 +83,7 @@ def CheckDeleteStartsTransaction(self):
 
     def CheckReplaceStartsTransaction(self):
         self.cur1.execute("create table test(i)")
+        self.con1.commit()
         self.cur1.execute("insert into test(i) values (5)")
         self.con1.commit()
         self.cur1.execute("replace into test(i) values (6)")
@@ -98,6 +94,7 @@ def CheckReplaceStartsTransaction(self):
 
     def CheckToggleAutoCommit(self):
         self.cur1.execute("create table test(i)")
+        self.con1.commit()
         self.cur1.execute("insert into test(i) values (5)")
         self.con1.isolation_level = None
         self.assertEqual(self.con1.isolation_level, None)
@@ -114,6 +111,7 @@ def CheckToggleAutoCommit(self):
 
     def CheckRaiseTimeout(self):
         self.cur1.execute("create table test(i)")
+        self.con1.commit()
         self.cur1.execute("insert into test(i) values (5)")
         try:
             self.cur2.execute("insert into test(i) values (5)")
@@ -129,6 +127,7 @@ def CheckLocking(self):
         to roll back con2 before you could commit con1.
         """
         self.cur1.execute("create table test(i)")
+        self.con1.commit()
         self.cur1.execute("insert into test(i) values (5)")
         try:
             self.cur2.execute("insert into test(i) values (5)")
@@ -148,6 +147,7 @@ def CheckRollbackCursorConsistency(self):
         con = sqlite.connect(":memory:")
         cur = con.cursor()
         cur.execute("create table test(x)")
+        con.commit()
         cur.execute("insert into test(x) values (5)")
         cur.execute("select 1 union select 2 union select 3")
 
@@ -165,13 +165,9 @@ def setUp(self):
         self.con = sqlite.connect(":memory:")
         self.cur = self.con.cursor()
 
-    def CheckVacuum(self):
-        self.cur.execute("create table test(i)")
-        self.cur.execute("insert into test(i) values (5)")
-        self.cur.execute("vacuum")
-
     def CheckDropTable(self):
         self.cur.execute("create table test(i)")
+        self.con.commit()
         self.cur.execute("insert into test(i) values (5)")
         self.cur.execute("drop table test")
 
diff --git a/lib/test/types.py b/lib/test/types.py
@@ -35,6 +35,7 @@ def setUp(self):
         self.con = sqlite.connect(":memory:")
         self.cur = self.con.cursor()
         self.cur.execute("create table test(i integer, s varchar, f number, b blob)")
+        self.con.commit()
 
     def tearDown(self):
         self.cur.close()
@@ -141,6 +142,7 @@ def setUp(self):
         self.con = sqlite.connect(":memory:", detect_types=sqlite.PARSE_DECLTYPES)
         self.cur = self.con.cursor()
         self.cur.execute("create table test(i int, s str, f float, b bool, u unicode, foo foo, bin blob, n1 number, n2 number(5))")
+        self.con.commit()
 
         # override float, make them always return the same number
         sqlite.converters["FLOAT"] = lambda x: 47.2
@@ -279,6 +281,7 @@ def setUp(self):
         self.con = sqlite.connect(":memory:", detect_types=sqlite.PARSE_COLNAMES)
         self.cur = self.con.cursor()
         self.cur.execute("create table test(x foo)")
+        self.con.commit()
 
         sqlite.converters["FOO"] = lambda x: "[%s]" % x
         sqlite.converters["BAR"] = lambda x: "<%s>" % x
@@ -379,6 +382,7 @@ def setUp(self):
         self.con = sqlite.connect(":memory:", detect_types=sqlite.PARSE_DECLTYPES)
         self.cur = self.con.cursor()
         self.cur.execute("create table test(d date, ts timestamp)")
+        self.con.commit()
 
     def tearDown(self):
         self.cur.close()
diff --git a/lib/test/userfunctions.py b/lib/test/userfunctions.py
@@ -269,6 +269,7 @@ def setUp(self):
                 b blob
                 )
             """)
+        self.con.commit()
         cur.execute("insert into test(t, i, f, n, b) values (?, ?, ?, ?, ?)",
             ("foo", 5, 3.14, None, buffer("blob"),))
 
diff --git a/src/cursor.c b/src/cursor.c
@@ -37,44 +37,6 @@ PyObject* pysqlite_cursor_iternext(pysqlite_Cursor* self);
 
 static char* errmsg_fetch_across_rollback = "Cursor needed to be reset because of commit/rollback and can no longer be fetched from.";
 
-static pysqlite_StatementKind detect_statement_type(char* statement)
-{
-    char buf[20];
-    char* src;
-    char* dst;
-
-    src = statement;
-    /* skip over whitepace */
-    while (*src == '\r' || *src == '\n' || *src == ' ' || *src == '\t') {
-        src++;
-    }
-
-    if (*src == 0)
-        return STATEMENT_INVALID;
-
-    dst = buf;
-    *dst = 0;
-    while (Py_ISALPHA(*src) && dst - buf < sizeof(buf) - 2) {
-        *dst++ = Py_TOLOWER(*src++);
-    }
-
-    *dst = 0;
-
-    if (!strcmp(buf, "select")) {
-        return STATEMENT_SELECT;
-    } else if (!strcmp(buf, "insert")) {
-        return STATEMENT_INSERT;
-    } else if (!strcmp(buf, "update")) {
-        return STATEMENT_UPDATE;
-    } else if (!strcmp(buf, "delete")) {
-        return STATEMENT_DELETE;
-    } else if (!strcmp(buf, "replace")) {
-        return STATEMENT_REPLACE;
-    } else {
-        return STATEMENT_OTHER;
-    }
-}
-
 static int pysqlite_cursor_init(pysqlite_Cursor* self, PyObject* args, PyObject* kwargs)
 {
     pysqlite_Connection* connection;
@@ -454,7 +416,6 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
     PyObject* func_args;
     PyObject* result;
     int numcols;
-    int statement_type;
     PyObject* descriptor;
     PyObject* second_argument = NULL;
     int allow_8bit_chars;
@@ -550,7 +511,7 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
     Py_DECREF(self->description);
     Py_INCREF(Py_None);
     self->description = Py_None;
-    self->rowcount = -1L;
+    self->rowcount = 0L;
 
     func_args = PyTuple_New(1);
     if (!func_args) {
@@ -589,39 +550,13 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
     pysqlite_statement_reset(self->statement);
     pysqlite_statement_mark_dirty(self->statement);
 
-    statement_type = detect_statement_type(operation_cstr);
-    if (self->connection->begin_statement) {
-        switch (statement_type) {
-            case STATEMENT_UPDATE:
-            case STATEMENT_DELETE:
-            case STATEMENT_INSERT:
-            case STATEMENT_REPLACE:
-                if (sqlite3_get_autocommit(self->connection->db)) {
-                    result = _pysqlite_connection_begin(self->connection);
-                    if (!result) {
-                        goto error;
-                    }
-                    Py_DECREF(result);
-                }
-                break;
-            case STATEMENT_OTHER:
-                /* it's a DDL statement or something similar
-                   - we better COMMIT first so it works for all cases */
-                if (!sqlite3_get_autocommit(self->connection->db)) {
-                    result = pysqlite_connection_commit(self->connection, NULL);
-                    if (!result) {
-                        goto error;
-                    }
-                    Py_DECREF(result);
-                }
-                break;
-            case STATEMENT_SELECT:
-                if (multiple) {
-                    PyErr_SetString(pysqlite_ProgrammingError,
-                                "You cannot execute SELECT statements in executemany().");
-                    goto error;
-                }
-                break;
+    if (self->connection->begin_statement && !sqlite3_stmt_readonly(self->statement->st)) {
+        if (sqlite3_get_autocommit(self->connection->db)) {
+            result = _pysqlite_connection_begin(self->connection);
+            if (!result) {
+                goto error;
+            }
+            Py_DECREF(result);
         }
     }
 
@@ -658,7 +593,7 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
             goto error;
         }
 
-        if (rc == SQLITE_ROW || (rc == SQLITE_DONE && statement_type == STATEMENT_SELECT)) {
+        if (rc == SQLITE_ROW || rc == SQLITE_DONE) {
             if (self->description == Py_None) {
                 Py_BEGIN_ALLOW_THREADS
                 numcols = sqlite3_column_count(self->statement->st);
@@ -686,6 +621,21 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
             }
         }
 
+        if (!sqlite3_stmt_readonly(self->statement->st)) {
+            self->rowcount += (long)sqlite3_changes(self->connection->db);
+        } else {
+            self->rowcount= -1;
+        }
+
+        Py_DECREF(self->lastrowid);
+        if (!multiple) {
+            sqlite_int64 lastrowid;
+            Py_BEGIN_ALLOW_THREADS
+            lastrowid = sqlite3_last_insert_rowid(self->connection->db);
+            Py_END_ALLOW_THREADS
+            self->lastrowid = _pysqlite_long_from_int64(lastrowid);
+        }
+
         if (rc == SQLITE_ROW) {
             if (multiple) {
                 PyErr_SetString(pysqlite_ProgrammingError, "executemany() can only execute DML statements.");
@@ -698,29 +648,6 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
             Py_CLEAR(self->statement);
         }
 
-        switch (statement_type) {
-            case STATEMENT_UPDATE:
-            case STATEMENT_DELETE:
-            case STATEMENT_INSERT:
-            case STATEMENT_REPLACE:
-                if (self->rowcount == -1L) {
-                    self->rowcount = 0L;
-                }
-                self->rowcount += (long)sqlite3_changes(self->connection->db);
-        }
-
-        Py_DECREF(self->lastrowid);
-        if (!multiple && statement_type == STATEMENT_INSERT) {
-            sqlite_int64 lastrowid;
-            Py_BEGIN_ALLOW_THREADS
-            lastrowid = sqlite3_last_insert_rowid(self->connection->db);
-            Py_END_ALLOW_THREADS
-            self->lastrowid = _pysqlite_long_from_int64(lastrowid);
-        } else {
-            Py_INCREF(Py_None);
-            self->lastrowid = Py_None;
-        }
-
         if (multiple) {
             rc = pysqlite_statement_reset(self->statement);
         }
diff --git a/src/cursor.h b/src/cursor.h
@@ -50,12 +50,6 @@ typedef struct
     PyObject* in_weakreflist; /* List of weak references */
 } pysqlite_Cursor;
 
-typedef enum {
-    STATEMENT_INVALID, STATEMENT_INSERT, STATEMENT_DELETE,
-    STATEMENT_UPDATE, STATEMENT_REPLACE, STATEMENT_SELECT,
-    STATEMENT_OTHER
-} pysqlite_StatementKind;
-
 extern PyTypeObject pysqlite_CursorType;
 
 PyObject* pysqlite_cursor_execute(pysqlite_Cursor* self, PyObject* args);

Original file line number	Diff line number	Diff line change
`@@ -269,6 +269,7 @@ def setUp(self):`
`269`	`269`	`b blob`
`270`	`270`	`)`
`271`	`271`	`""")`
	`272`	`+ self.con.commit()`
`272`	`273`	`cur.execute("insert into test(t, i, f, n, b) values (?, ?, ?, ?, ?)",`
`273`	`274`	`("foo", 5, 3.14, None, buffer("blob"),))`
`274`	`275`