From 6ab925243f112ee5786beb52d15291672020687f Mon Sep 17 00:00:00 2001 From: Jahnvi Thakkar Date: Tue, 14 Apr 2026 14:43:54 +0530 Subject: [PATCH] FIX: cursor.execute() with reset_cursor=False raises Invalid cursor state #506 Add SqlHandle::close_cursor() which calls SQLFreeStmt(SQL_CLOSE) to close the ODBC cursor without destroying the prepared statement handle. When reset_cursor=False, execute() now calls close_cursor() instead of skipping cleanup entirely, allowing the prepared plan to be reused. Added 5 tests covering reset_cursor=False scenarios. --- mssql_python/cursor.py | 10 ++++ mssql_python/pybind/ddbc_bindings.cpp | 9 ++- mssql_python/pybind/ddbc_bindings.h | 1 + tests/test_004_cursor.py | 81 +++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 1 deletion(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index ba5065d56..3b4e60887 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -1351,6 +1351,16 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state if reset_cursor: logger.debug("execute: Resetting cursor state") self._reset_cursor() + else: + # Close just the ODBC cursor (not the statement handle) so the + # prepared plan can be reused. SQLFreeStmt(SQL_CLOSE) releases + # the cursor associated with hstmt without destroying the + # prepared statement, which is the standard ODBC pattern for + # re-executing a prepared query. + if self.hstmt: + logger.debug("execute: Closing cursor for re-execution (reset_cursor=False)") + self.hstmt.close_cursor() + self._clear_rownumber() # Clear any previous messages self.messages = [] diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index c7537cbe8..0e82a9865 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -1362,6 +1362,12 @@ void SqlHandle::free() { } } +void SqlHandle::close_cursor() { + if (_handle && SQLFreeStmt_ptr && _type == SQL_HANDLE_STMT) { + SQLFreeStmt_ptr(_handle, SQL_CLOSE); + } +} + SQLRETURN SQLGetTypeInfo_Wrapper(SqlHandlePtr StatementHandle, SQLSMALLINT DataType) { if (!SQLGetTypeInfo_ptr) { ThrowStdException("SQLGetTypeInfo function not loaded"); @@ -5740,7 +5746,8 @@ PYBIND11_MODULE(ddbc_bindings, m) { .def_readwrite("ddbcErrorMsg", &ErrorInfo::ddbcErrorMsg); py::class_(m, "SqlHandle") - .def("free", &SqlHandle::free, "Free the handle"); + .def("free", &SqlHandle::free, "Free the handle") + .def("close_cursor", &SqlHandle::close_cursor, "Close the cursor on the statement handle without freeing the prepared statement"); py::class_(m, "Connection") .def(py::init(), py::arg("conn_str"), diff --git a/mssql_python/pybind/ddbc_bindings.h b/mssql_python/pybind/ddbc_bindings.h index 8c6cfed97..958758a08 100644 --- a/mssql_python/pybind/ddbc_bindings.h +++ b/mssql_python/pybind/ddbc_bindings.h @@ -378,6 +378,7 @@ class SqlHandle { SQLHANDLE get() const; SQLSMALLINT type() const; void free(); + void close_cursor(); // Mark this handle as implicitly freed (freed by parent handle) // This prevents double-free attempts when the ODBC driver automatically diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index 6ac157389..d83225b76 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -15961,3 +15961,84 @@ def reader(reader_id): finally: stop_event.set() mssql_python.native_uuid = original + + +def test_execute_reset_cursor_false_reuses_prepared_plan(db_connection): + """Test that reset_cursor=False reuses the prepared statement handle + and successfully re-executes after consuming the previous result set.""" + cursor = db_connection.cursor() + try: + cursor.execute("SELECT 1 AS val WHERE 1 = ?", (1,)) + row = cursor.fetchone() + assert row[0] == 1 + _ = cursor.fetchall() # consume remaining + + # Re-execute with reset_cursor=False — this was raising + # ProgrammingError: Invalid cursor state before the fix. + cursor.execute("SELECT 1 AS val WHERE 1 = ?", (2,), reset_cursor=False) + row = cursor.fetchone() + assert row is None # No match for WHERE 1 = 2 + finally: + cursor.close() + + +def test_execute_reset_cursor_false_returns_new_results(db_connection): + """Test that reset_cursor=False correctly returns results from the + second execution with different parameter values.""" + cursor = db_connection.cursor() + try: + cursor.execute("SELECT ? AS val", (42,)) + row = cursor.fetchone() + assert row[0] == 42 + _ = cursor.fetchall() + + cursor.execute("SELECT ? AS val", (99,), reset_cursor=False) + row = cursor.fetchone() + assert row[0] == 99 + finally: + cursor.close() + + +def test_execute_reset_cursor_false_multiple_iterations(db_connection): + """Test that reset_cursor=False works across several consecutive + re-executions on the same cursor.""" + cursor = db_connection.cursor() + try: + for i in range(5): + kwargs = {"reset_cursor": False} if i > 0 else {} + cursor.execute("SELECT ? AS iter", (i,), **kwargs) + row = cursor.fetchone() + assert row[0] == i, f"Expected {i}, got {row[0]}" + _ = cursor.fetchall() + finally: + cursor.close() + + +def test_execute_reset_cursor_false_no_params(db_connection): + """Test that reset_cursor=False works for queries without parameters.""" + cursor = db_connection.cursor() + try: + cursor.execute("SELECT 1 AS a") + _ = cursor.fetchall() + + cursor.execute("SELECT 2 AS a", reset_cursor=False) + row = cursor.fetchone() + assert row[0] == 2 + finally: + cursor.close() + + +def test_execute_reset_cursor_false_after_fetchone_only(db_connection): + """Test reset_cursor=False when only fetchone() was called (result set + not fully consumed via fetchall).""" + cursor = db_connection.cursor() + try: + cursor.execute("SELECT ? AS val", (1,)) + row = cursor.fetchone() + assert row[0] == 1 + # Do NOT call fetchall — go straight to re-execute + cursor.execute("SELECT ? AS val", (2,), reset_cursor=False) + row = cursor.fetchone() + assert row[0] == 2 + finally: + cursor.close()