What steps will reproduce the problem? 1. Create a connection to a database with autocommit off using the context manager, and then leave an SQL statement uncommitted. For example as follows:
import pyodbc
Note, auto-commit is set False on the connection
with pyodbc.connect('<...>', autocommit=False) as conn, \ conn.cursor() as cursor:
cursor.execute(r"CREATE TABLE ContextManagerCommitTest " \
r"(FirstName varchar(100) NULL, " \
r"MiddleName varchar(100) NULL, " \
r"LastName varchar(100) NULL)")
conn.commit()
cursor.execute(r"INSERT INTO BatchUpdaterTest " \
r"(FirstName, MiddleName, LastName) " \
r"VALUES " \
r"('FN1', 'MN1', 'LN1'), " \
r"('FN2', 'MN2', 'LN2'), " \
r"('FN3', 'MN3', 'LN3')")
# Note, this insert transaction is NOT committed
- After this code has run, examine the table. It should be empty but isn't.
What is the expected output? What do you see instead? The table is populated with the three records. This is absolutely NOT what I would expect when auto-commit is off. For example, suppose I am running multiple SQL updates against the database, and I want to commit all of them in one go or none of them. If an exception occurs half way through (for whatever reason), pyodbc will issue a commit regardless when the context manager exits. This is really bad because pyodbc is committing only half my updates and potentially leaving the database in an odd state. If autocommit is off for the connection, it is entirely the responsibility of the programmer to commit transactions. Pyodbc should never do a commit without being told to do so. Right now, if I want to use a connection with autocommit off, I must remember not to use the context manager because otherwise there is a significant risk of screwing up my data.
What version of the product are you using? On what operating system? Version 3.0.7, Python 3.4, on Windows 7 against a SQL Server 2008 R2 database.
Please provide any additional information below.
It appears that both connection.cpp and cursor.cpp do a commit in their "exit" functions. A quick fix may be to change the "cnxn->nAutoCommit" check from SQL_AUTOCOMMIT_OFF to SQL_AUTOCOMMIT_ON, which seems more logical regardless.
Comment #1
Posted on Apr 12, 2015 by Grumpy HorseOn further investigation, it appears that uncommitted transactions ARE rolled back if an exception occurs, but if no exception occurs and the context is exited normally then the transaction IS committed. So what I was saying in my previous message about the exception scenario wasn't correct.
Nevertheless, my original test is still relevant and this is still not good behavior. There are times when I want to hand a connection object to another module to do some updates, but maintain control over commits. If the other module is unknowingly committing transactions (by using a context manager), this is not good.
Comment #2
Posted on Apr 12, 2015 by Grumpy HorseI have created a git branch to give a suggestion for how this could be fixed: https://github.com/keitherskine/pyodbc/commit/6c0a9d18e496cee00ba8d412db3a1b72e8ee78de
The changes are as follows:
1) In connection.cpp, Connection_exit() commits only when auto-commit is on and it's a normal (non-error) exit, otherwise it rolls back the transaction. I believe this is the correct behavior. I also added a call to Connection_close() at the end. Without it, I'm not sure how the connection is being properly closed.
In cursor.cpp, I have removed all attempts to commit/rollback. When a cursor is closed, my understanding is that there is no requirement or implication that transactions will be committed or rolled back at the same time. I'm puzzled why this is happening in this function. Nevertheless, the cursor should still be closed, hence I've added a call to Cursor_close().
I've also added some tests to cover these changes.
If it helps to turn this into a pull request, let me know.
Status: New
Labels:
Type-Defect
Priority-Medium