Skip to content

Commit 28e3b9f

Browse files
authored
FIX: cursor.execute() with reset_cursor=False raises Invalid cursor state (#521)
### Work Item / Issue Reference <!-- IMPORTANT: Please follow the PR template guidelines below. For mssql-python maintainers: Insert your ADO Work Item ID below For external contributors: Insert Github Issue number below Only one reference is required - either GitHub issue OR ADO Work Item. --> <!-- mssql-python maintainers: ADO Work Item --> > [AB#44009](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/44009) <!-- External contributors: GitHub Issue --> > GitHub Issue: #506 ------------------------------------------------------------------- ### Summary This pull request improves the handling of cursor state when executing prepared statements with `reset_cursor=False`, ensuring that the prepared plan is correctly reused and the cursor state is properly managed between executions. It introduces a new `close_cursor` method at the ODBC statement handle level, updates the Python bindings, and adds comprehensive tests to verify the new behavior. **Enhancements to cursor state management:** - Added logic in `cursor.py` to call `hstmt.close_cursor()` (which issues an ODBC `SQLFreeStmt(SQL_CLOSE)`) when `reset_cursor=False`, allowing prepared statements to be reused safely without resetting the entire cursor state. This resolves issues with invalid cursor state on re-execution. **ODBC handle and Python bindings updates:** - Implemented the `close_cursor` method in the `SqlHandle` C++ class, which closes only the cursor on the statement handle without freeing the prepared statement. - Exposed the new `close_cursor` method to Python via the `ddbc_bindings` Pybind11 module, allowing it to be called from Python code. **Testing improvements:** - Added multiple test cases in `test_004_cursor.py` to verify that `reset_cursor=False` correctly reuses prepared plans, returns new results, works across multiple iterations, handles queries without parameters, and functions correctly when the previous result set was not fully consumed.
1 parent ed33bc5 commit 28e3b9f

4 files changed

Lines changed: 111 additions & 1 deletion

File tree

mssql_python/cursor.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,6 +1365,16 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state
13651365
if reset_cursor:
13661366
logger.debug("execute: Resetting cursor state")
13671367
self._reset_cursor()
1368+
else:
1369+
# Close just the ODBC cursor (not the statement handle) so the
1370+
# prepared plan can be reused. SQLFreeStmt(SQL_CLOSE) releases
1371+
# the cursor associated with hstmt without destroying the
1372+
# prepared statement, which is the standard ODBC pattern for
1373+
# re-executing a prepared query.
1374+
if self.hstmt:
1375+
logger.debug("execute: Closing cursor for re-execution (reset_cursor=False)")
1376+
self.hstmt._close_cursor()
1377+
self._clear_rownumber()
13681378

13691379
# Clear any previous messages
13701380
self.messages = []

mssql_python/pybind/ddbc_bindings.cpp

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1362,6 +1362,22 @@ void SqlHandle::free() {
13621362
}
13631363
}
13641364

1365+
void SqlHandle::close_cursor() {
1366+
if (_type != SQL_HANDLE_STMT || !_handle) {
1367+
return;
1368+
}
1369+
if (_implicitly_freed) {
1370+
return;
1371+
}
1372+
if (!SQLFreeStmt_ptr) {
1373+
ThrowStdException("SQLFreeStmt function not loaded");
1374+
}
1375+
SQLRETURN ret = SQLFreeStmt_ptr(_handle, SQL_CLOSE);
1376+
if (ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO) {
1377+
ThrowStdException("SQLFreeStmt(SQL_CLOSE) failed");
1378+
}
1379+
}
1380+
13651381
SQLRETURN SQLGetTypeInfo_Wrapper(SqlHandlePtr StatementHandle, SQLSMALLINT DataType) {
13661382
if (!SQLGetTypeInfo_ptr) {
13671383
ThrowStdException("SQLGetTypeInfo function not loaded");
@@ -5740,7 +5756,8 @@ PYBIND11_MODULE(ddbc_bindings, m) {
57405756
.def_readwrite("ddbcErrorMsg", &ErrorInfo::ddbcErrorMsg);
57415757

57425758
py::class_<SqlHandle, SqlHandlePtr>(m, "SqlHandle")
5743-
.def("free", &SqlHandle::free, "Free the handle");
5759+
.def("free", &SqlHandle::free, "Free the handle")
5760+
.def("_close_cursor", &SqlHandle::close_cursor, "Internal: close the cursor without freeing the prepared statement");
57445761

57455762
py::class_<ConnectionHandle>(m, "Connection")
57465763
.def(py::init<const std::string&, bool, const py::dict&>(), py::arg("conn_str"),

mssql_python/pybind/ddbc_bindings.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,7 @@ class SqlHandle {
378378
SQLHANDLE get() const;
379379
SQLSMALLINT type() const;
380380
void free();
381+
void close_cursor();
381382

382383
// Mark this handle as implicitly freed (freed by parent handle)
383384
// This prevents double-free attempts when the ODBC driver automatically

tests/test_004_cursor.py

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16253,3 +16253,85 @@ def reader(reader_id):
1625316253
finally:
1625416254
stop_event.set()
1625516255
mssql_python.native_uuid = original
16256+
16257+
16258+
def test_execute_reset_cursor_false_reuses_prepared_plan(db_connection):
16259+
"""Test that reset_cursor=False reuses the prepared statement handle
16260+
and successfully re-executes after consuming the previous result set."""
16261+
cursor = db_connection.cursor()
16262+
try:
16263+
cursor.execute("SELECT 1 AS val WHERE 1 = ?", (1,))
16264+
row = cursor.fetchone()
16265+
assert row[0] == 1
16266+
_ = cursor.fetchall() # consume remaining
16267+
16268+
# Re-execute with reset_cursor=False — this was raising
16269+
# ProgrammingError: Invalid cursor state before the fix.
16270+
cursor.execute("SELECT 1 AS val WHERE 1 = ?", (2,), reset_cursor=False)
16271+
row = cursor.fetchone()
16272+
assert row is None # No match for WHERE 1 = 2
16273+
finally:
16274+
cursor.close()
16275+
16276+
16277+
def test_execute_reset_cursor_false_returns_new_results(db_connection):
16278+
"""Test that reset_cursor=False correctly returns results from the
16279+
second execution with different parameter values."""
16280+
cursor = db_connection.cursor()
16281+
try:
16282+
cursor.execute("SELECT ? AS val", (42,))
16283+
row = cursor.fetchone()
16284+
assert row[0] == 42
16285+
_ = cursor.fetchall()
16286+
16287+
cursor.execute("SELECT ? AS val", (99,), reset_cursor=False)
16288+
row = cursor.fetchone()
16289+
assert row[0] == 99
16290+
finally:
16291+
cursor.close()
16292+
16293+
16294+
def test_execute_reset_cursor_false_multiple_iterations(db_connection):
16295+
"""Test that reset_cursor=False works across several consecutive
16296+
re-executions on the same cursor."""
16297+
cursor = db_connection.cursor()
16298+
try:
16299+
for i in range(5):
16300+
kwargs = {"reset_cursor": False} if i > 0 else {}
16301+
cursor.execute("SELECT ? AS iter", (i,), **kwargs)
16302+
row = cursor.fetchone()
16303+
assert row is not None, f"Expected row with value {i}, got None"
16304+
assert row[0] == i, f"Expected {i}, got {row[0]}"
16305+
_ = cursor.fetchall()
16306+
finally:
16307+
cursor.close()
16308+
16309+
16310+
def test_execute_reset_cursor_false_no_params(db_connection):
16311+
"""Test that reset_cursor=False works for queries without parameters."""
16312+
cursor = db_connection.cursor()
16313+
try:
16314+
cursor.execute("SELECT 1 AS a")
16315+
_ = cursor.fetchall()
16316+
16317+
cursor.execute("SELECT 2 AS a", reset_cursor=False)
16318+
row = cursor.fetchone()
16319+
assert row[0] == 2
16320+
finally:
16321+
cursor.close()
16322+
16323+
16324+
def test_execute_reset_cursor_false_after_fetchone_only(db_connection):
16325+
"""Test reset_cursor=False when only fetchone() was called (result set
16326+
not fully consumed via fetchall)."""
16327+
cursor = db_connection.cursor()
16328+
try:
16329+
cursor.execute("SELECT ? AS val", (1,))
16330+
row = cursor.fetchone()
16331+
assert row[0] == 1
16332+
# Do NOT call fetchall — go straight to re-execute
16333+
cursor.execute("SELECT ? AS val", (2,), reset_cursor=False)
16334+
row = cursor.fetchone()
16335+
assert row[0] == 2
16336+
finally:
16337+
cursor.close()

0 commit comments

Comments
 (0)