Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions mssql_python/cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down
9 changes: 8 additions & 1 deletion mssql_python/pybind/ddbc_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,12 @@ void SqlHandle::free() {
}
}

void SqlHandle::close_cursor() {
if (_handle && SQLFreeStmt_ptr && _type == SQL_HANDLE_STMT) {
SQLFreeStmt_ptr(_handle, SQL_CLOSE);
}
Comment on lines +1366 to +1368
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close_cursor() silently no-ops when SQLFreeStmt_ptr is not loaded and also ignores the SQLRETURN from SQLFreeStmt. This can mask failures and leave the cursor open, potentially preserving the original 'Invalid cursor state' behavior without surfacing an actionable error. Consider aligning behavior with other wrappers: (1) throw when SQLFreeStmt_ptr is not loaded, and (2) check the return code and raise via the existing error/diagnostics path when the call fails (treat SQL_SUCCESS_WITH_INFO appropriately).

Suggested change
if (_handle && SQLFreeStmt_ptr && _type == SQL_HANDLE_STMT) {
SQLFreeStmt_ptr(_handle, SQL_CLOSE);
}
if (_type != SQL_HANDLE_STMT || !_handle) {
return;
}
if (!SQLFreeStmt_ptr) {
ThrowStdException("SQLFreeStmt function not loaded");
}
SQLRETURN ret = SQLFreeStmt_ptr(_handle, SQL_CLOSE);
if (ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO) {
ThrowStdException("SQLFreeStmt(SQL_CLOSE) failed");
}

Copilot uses AI. Check for mistakes.
}

SQLRETURN SQLGetTypeInfo_Wrapper(SqlHandlePtr StatementHandle, SQLSMALLINT DataType) {
if (!SQLGetTypeInfo_ptr) {
ThrowStdException("SQLGetTypeInfo function not loaded");
Expand Down Expand Up @@ -5740,7 +5746,8 @@ PYBIND11_MODULE(ddbc_bindings, m) {
.def_readwrite("ddbcErrorMsg", &ErrorInfo::ddbcErrorMsg);

py::class_<SqlHandle, SqlHandlePtr>(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_<ConnectionHandle>(m, "Connection")
.def(py::init<const std::string&, bool, const py::dict&>(), py::arg("conn_str"),
Expand Down
1 change: 1 addition & 0 deletions mssql_python/pybind/ddbc_bindings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
81 changes: 81 additions & 0 deletions tests/test_004_cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]}"
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion message indexes row[0] even in the failure path; if fetchone() unexpectedly returns None, this will raise a TypeError and obscure the real failure. Prefer asserting the whole row (e.g., assert row == (i,)) or add a separate assert row is not None before indexing so test failures remain clear and diagnostic.

Suggested change
assert row[0] == i, f"Expected {i}, got {row[0]}"
assert row == (i,), f"Expected {(i,)}, got {row}"

Copilot uses AI. Check for mistakes.
_ = 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()
Loading