Skip to content

Commit

Permalink
For backwards compatibility reasons, a DDL statement should not
Browse files Browse the repository at this point in the history
implicitly start a transaction. This code achieves that.
  • Loading branch information
ghaering committed Aug 18, 2015
1 parent 94eae50 commit 796b3af
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 17 deletions.
5 changes: 0 additions & 5 deletions lib/test/dbapi.py
Expand Up @@ -88,7 +88,6 @@ 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):
Expand Down Expand Up @@ -146,7 +145,6 @@ 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):
Expand Down Expand Up @@ -478,7 +476,6 @@ 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()
Expand Down Expand Up @@ -709,7 +706,6 @@ 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")
Expand Down Expand Up @@ -882,7 +878,6 @@ 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):
Expand Down
1 change: 0 additions & 1 deletion lib/test/factory.py
Expand Up @@ -204,7 +204,6 @@ 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):
Expand Down
1 change: 0 additions & 1 deletion lib/test/hooks.py
Expand Up @@ -136,7 +136,6 @@ def progress():
con.execute("""
create table foo(a, b)
""")
con.commit()
self.assertTrue(progress_calls)


Expand Down
1 change: 0 additions & 1 deletion lib/test/regression.py
Expand Up @@ -87,7 +87,6 @@ def CheckStatementFinalizationOnCloseDb(self):
def CheckOnConflictRollback(self):
con = sqlite.connect(":memory:")
con.execute("create table foo(x, unique(x) on conflict rollback)")
con.commit()
con.execute("insert into foo(x) values (1)")
try:
con.execute("insert into foo(x) values (1)")
Expand Down
31 changes: 28 additions & 3 deletions lib/test/transactions.py
Expand Up @@ -147,7 +147,6 @@ 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")

Expand All @@ -167,7 +166,6 @@ def setUp(self):

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")

Expand All @@ -180,10 +178,37 @@ def tearDown(self):
self.cur.close()
self.con.close()

class TransactionalDDL(unittest.TestCase):
def setUp(self):
self.con = sqlite.connect(":memory:")

def CheckDdlDoesNotAutostartTransaction(self):
"""
For backwards compatibility reasons, DDL statements should not
implicitly start a transaction.
"""
self.con.execute("create table test(i)")
self.con.rollback()
self.con.execute("select * from test")

def CheckTransactionalDDL(self):
"""
You can achieve transactional DDL by issuing a BEGIN statement manually.
"""
self.con.execute("begin")
self.con.execute("create table test(i)")
self.con.rollback()
with self.assertRaises(sqlite.OperationalError):
self.con.execute("select * from test")

def tearDown(self):
self.con.close()

def suite():
default_suite = unittest.makeSuite(TransactionTests, "Check")
special_command_suite = unittest.makeSuite(SpecialCommandTests, "Check")
return unittest.TestSuite((default_suite, special_command_suite))
ddl_suite = unittest.makeSuite(TransactionalDDL, "Check")
return unittest.TestSuite((default_suite, special_command_suite, ddl_suite))

def test():
runner = unittest.TextTestRunner()
Expand Down
4 changes: 0 additions & 4 deletions lib/test/types.py
Expand Up @@ -35,7 +35,6 @@ 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()
Expand Down Expand Up @@ -142,7 +141,6 @@ 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
Expand Down Expand Up @@ -281,7 +279,6 @@ 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
Expand Down Expand Up @@ -382,7 +379,6 @@ 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()
Expand Down
1 change: 0 additions & 1 deletion lib/test/userfunctions.py
Expand Up @@ -269,7 +269,6 @@ 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"),))

Expand Down
4 changes: 3 additions & 1 deletion src/cursor.c
Expand Up @@ -550,7 +550,9 @@ PyObject* _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject*
pysqlite_statement_reset(self->statement);
pysqlite_statement_mark_dirty(self->statement);

if (self->connection->begin_statement && !sqlite3_stmt_readonly(self->statement->st)) {
/* For backwards compatibility, 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) {
if (sqlite3_get_autocommit(self->connection->db)) {
result = _pysqlite_connection_begin(self->connection);
if (!result) {
Expand Down
18 changes: 18 additions & 0 deletions src/statement.c
Expand Up @@ -56,6 +56,7 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con
int rc;
PyObject* sql_str;
char* sql_cstr;
char* p;

self->st = NULL;
self->in_use = 0;
Expand All @@ -79,6 +80,23 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con

sql_cstr = PyString_AsString(sql_str);

/* determine if the statement is a DDL statement */
self->is_ddl = 0;
for (p = sql_cstr; *p != 0; p++) {
switch (*p) {
case ' ':
case '\r':
case '\n':
case '\t':
continue;
}

self->is_ddl = (strncasecmp(p, "create", 6) == 0)
|| (strncasecmp(p, "drop", 4) == 0)
|| (strncasecmp(p, "reindex", 7) == 0);
break;
}

Py_BEGIN_ALLOW_THREADS
rc = sqlite3_prepare_v2(connection->db,
sql_cstr,
Expand Down
1 change: 1 addition & 0 deletions src/statement.h
Expand Up @@ -38,6 +38,7 @@ typedef struct
sqlite3_stmt* st;
PyObject* sql;
int in_use;
int is_ddl;
PyObject* in_weakreflist; /* List of weak references */
} pysqlite_Statement;

Expand Down

0 comments on commit 796b3af

Please sign in to comment.