diff --git a/mssql_python/connection.py b/mssql_python/connection.py index d1ed6e78..12760df4 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -147,7 +147,7 @@ def autocommit(self, value: bool) -> None: self.setautocommit(value) log('info', "Autocommit mode set to %s.", value) - def setautocommit(self, value: bool = True) -> None: + def setautocommit(self, value: bool = False) -> None: """ Set the autocommit mode of the connection. Args: @@ -259,6 +259,13 @@ def close(self) -> None: # Close the connection even if cursor cleanup had issues try: if self._conn: + if not self.autocommit: + # If autocommit is disabled, rollback any uncommitted changes + # This is important to ensure no partial transactions remain + # For autocommit True, this is not necessary as each statement is committed immediately + self._conn.rollback() + # TODO: Check potential race conditions in case of multithreaded scenarios + # Close the connection self._conn.close() self._conn = None except Exception as e: diff --git a/mssql_python/db_connection.py b/mssql_python/db_connection.py index 9c688ac6..5e98056e 100644 --- a/mssql_python/db_connection.py +++ b/mssql_python/db_connection.py @@ -5,7 +5,7 @@ """ from mssql_python.connection import Connection -def connect(connection_str: str = "", autocommit: bool = True, attrs_before: dict = None, **kwargs) -> Connection: +def connect(connection_str: str = "", autocommit: bool = False, attrs_before: dict = None, **kwargs) -> Connection: """ Constructor for creating a connection to the database. diff --git a/mssql_python/pybind/connection/connection.cpp b/mssql_python/pybind/connection/connection.cpp index a5c5f37f..9782efd2 100644 --- a/mssql_python/pybind/connection/connection.cpp +++ b/mssql_python/pybind/connection/connection.cpp @@ -132,9 +132,14 @@ void Connection::setAutocommit(bool enable) { ThrowStdException("Connection handle not allocated"); } SQLINTEGER value = enable ? SQL_AUTOCOMMIT_ON : SQL_AUTOCOMMIT_OFF; - LOG("Set SQL Connection Attribute"); + LOG("Setting SQL Connection Attribute"); SQLRETURN ret = SQLSetConnectAttr_ptr(_dbcHandle->get(), SQL_ATTR_AUTOCOMMIT, reinterpret_cast(static_cast(value)), 0); checkError(ret); + if(value == SQL_AUTOCOMMIT_ON) { + LOG("SQL Autocommit set to True"); + } else { + LOG("SQL Autocommit set to False"); + } _autocommit = enable; } diff --git a/tests/test_003_connection.py b/tests/test_003_connection.py index bef23815..422445c8 100644 --- a/tests/test_003_connection.py +++ b/tests/test_003_connection.py @@ -7,7 +7,15 @@ - test_commit: Make a transaction and commit. - test_rollback: Make a transaction and rollback. - test_invalid_connection_string: Check if initializing with an invalid connection string raises an exception. -Note: The cursor function is not yet implemented, so related tests are commented out. +- test_connection_pooling_speed: Test connection pooling speed. +- test_connection_pooling_basic: Test basic connection pooling functionality. +- test_autocommit_default: Check if autocommit is False by default. +- test_autocommit_setter: Test setting autocommit mode and its effect on transactions. +- test_set_autocommit: Test the setautocommit method. +- test_construct_connection_string: Check if the connection string is constructed correctly with kwargs. +- test_connection_string_with_attrs_before: Check if the connection string is constructed correctly with attrs_before. +- test_connection_string_with_odbc_param: Check if the connection string is constructed correctly with ODBC parameters. +- test_rollback_on_close: Test that rollback occurs on connection close if autocommit is False. """ from mssql_python.exceptions import InterfaceError @@ -73,7 +81,7 @@ def test_connection_string_with_odbc_param(db_connection): assert "Driver={ODBC Driver 18 for SQL Server};;APP=MSSQL-Python;Server=localhost;Uid=me;Pwd=mypwd;Database=mydb;Encrypt=yes;TrustServerCertificate=yes;" == conn_str, "Connection string is incorrect" def test_autocommit_default(db_connection): - assert db_connection.autocommit is True, "Autocommit should be True by default" + assert db_connection.autocommit is False, "Autocommit should be False by default" def test_autocommit_setter(db_connection): db_connection.autocommit = True @@ -140,6 +148,41 @@ def test_commit(db_connection): cursor.execute("DROP TABLE #pytest_test_commit;") db_connection.commit() +def test_rollback_on_close(conn_str, db_connection): + # Test that rollback occurs on connection close if autocommit is False + # Using a permanent table to ensure rollback is tested correctly + cursor = db_connection.cursor() + drop_table_if_exists(cursor, "pytest_test_rollback_on_close") + try: + # Create a permanent table for testing + cursor.execute("CREATE TABLE pytest_test_rollback_on_close (id INT PRIMARY KEY, value VARCHAR(50));") + db_connection.commit() + + # This simulates a scenario where the connection is closed without committing + # and checks if the rollback occurs + temp_conn = connect(conn_str) + temp_cursor = temp_conn.cursor() + temp_cursor.execute("INSERT INTO pytest_test_rollback_on_close (id, value) VALUES (1, 'test');") + + # Verify data is visible within the same transaction + temp_cursor.execute("SELECT * FROM pytest_test_rollback_on_close WHERE id = 1;") + result = temp_cursor.fetchone() + assert result is not None, "Rollback on close failed: No data found before close" + assert result[1] == 'test', "Rollback on close failed: Incorrect data before close" + + # Close the temporary connection without committing + temp_conn.close() + + # Now check if the data is rolled back + cursor.execute("SELECT * FROM pytest_test_rollback_on_close WHERE id = 1;") + result = cursor.fetchone() + assert result is None, "Rollback on close failed: Data found after rollback" + except Exception as e: + pytest.fail(f"Rollback on close failed: {e}") + finally: + drop_table_if_exists(cursor, "pytest_test_rollback_on_close") + db_connection.commit() + def test_rollback(db_connection): # Make a transaction and rollback cursor = db_connection.cursor()