FIX: Setting autocommit as False by default & add rollback on connection close (#158)

Author: jahnvi480

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:

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.
 

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<SQLPOINTER>(static_cast<SQLULEN>(value)), 0);
     checkError(ret);
+    if(value == SQL_AUTOCOMMIT_ON) {
+        LOG("SQL Autocommit set to True");
+    } else {
+        LOG("SQL Autocommit set to False");
+    }
     _autocommit = enable;
 }
 

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