Skip to content

Commit

Permalink
Rollback if exception raised in with clause.
Browse files Browse the repository at this point in the history
The previous code would only commit at the end of a with clause if an exception was raised, but
it did not perform a rollback like the documentation stated.  While this is harmless if you are
allocating new connections in a scope so the automatic close would roll it back, it would be a
disaster if you were using a manual connection pool and didn't roll back yourself.
  • Loading branch information
mkleehammer committed Feb 11, 2017
1 parent 4ad3989 commit 23db3d0
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 13 deletions.
10 changes: 7 additions & 3 deletions src/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1281,15 +1281,19 @@ static PyObject* Connection_exit(PyObject* self, PyObject* args)
// If an error has occurred, `args` will be a tuple of 3 values. Otherwise it will be a tuple of 3 `None`s.
I(PyTuple_Check(args));

if (cnxn->nAutoCommit == SQL_AUTOCOMMIT_OFF && PyTuple_GetItem(args, 0) == Py_None)
if (cnxn->nAutoCommit == SQL_AUTOCOMMIT_OFF)
{
SQLSMALLINT CompletionType = (PyTuple_GetItem(args, 0) == Py_None) ? SQL_COMMIT : SQL_ROLLBACK;
SQLRETURN ret;
Py_BEGIN_ALLOW_THREADS
ret = SQLEndTran(SQL_HANDLE_DBC, cnxn->hdbc, SQL_COMMIT);
ret = SQLEndTran(SQL_HANDLE_DBC, cnxn->hdbc, CompletionType);
Py_END_ALLOW_THREADS

if (!SQL_SUCCEEDED(ret))
return RaiseErrorFromHandle("SQLEndTran(SQL_COMMIT)", cnxn->hdbc, SQL_NULL_HANDLE);
{
const char* szFunc = (CompletionType == SQL_COMMIT) ? "SQLEndTran(SQL_COMMIT)" : "SQLEndTran(SQL_ROLLBACK)";
return RaiseErrorFromHandle(szFunc, cnxn->hdbc, SQL_NULL_HANDLE);
}
}

Py_RETURN_NONE;
Expand Down
33 changes: 23 additions & 10 deletions tests3/sqlservertests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1236,24 +1236,37 @@ def test_row_gtlt(self):
rows.sort() # uses <

def test_context_manager_success(self):

"Ensure `with` commits if an exception is not raised"
self.cursor.execute("create table t1(n int)")
self.cnxn.commit()

try:
with pyodbc.connect(self.connection_string) as cnxn:
cursor = cnxn.cursor()
cursor.execute("insert into t1 values (1)")
except Exception:
pass

cnxn = None
cursor = None
with self.cnxn:
self.cursor.execute("insert into t1 values (1)")

rows = self.cursor.execute("select n from t1").fetchall()
self.assertEquals(len(rows), 1)
self.assertEquals(rows[0][0], 1)

def test_context_manager_failure(self):
"Ensure `with` rolls back if an exception is raised"
# We'll insert a row and commit it. Then we'll insert another row followed by an
# exception.

self.cursor.execute("create table t1(n int)")
self.cursor.execute("insert into t1 values (1)")
self.cnxn.commit()

def _fail():
with self.cnxn:
self.cursor.execute("insert into t1 values (2)")
self.cursor.execute("delete from bogus")

self.assertRaises(pyodbc.Error, _fail)

self.cursor.execute("select max(n) from t1")
val = self.cursor.fetchval()
self.assertEqual(val, 1)


def test_untyped_none(self):
# From issue 129
Expand Down

0 comments on commit 23db3d0

Please sign in to comment.