FIX: Timeout during cursor creation and not execute#348
Merged
Conversation
… jahnvi/final_linting
sumitmsft
reviewed
Nov 28, 2025
… jahnvi/timeout_291
…rosoft/mssql-python into jahnvi/timeout_291
github-actions
bot
added
pr-size: medium
sumitmsft
previously approved these changes
Dec 1, 2025
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors query timeout handling in the mssql-python driver by moving timeout configuration from the execute method to cursor initialization, following pyodbc's approach for better performance. The timeout is now set once when a cursor is created or reset, rather than on every query execution.
- Introduces
_set_timeout()method to configure query timeout during cursor initialization - Updates C++ bindings to properly handle integer statement attributes like
SQL_ATTR_QUERY_TIMEOUT - Adds comprehensive test coverage for timeout behavior across various scenarios
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| mssql_python/cursor.py | Refactors timeout handling by adding _set_timeout() method called during cursor initialization and removing redundant timeout logic from execute |
| mssql_python/pybind/ddbc_bindings.cpp | Updates DDBCSQLSetStmtAttr binding to handle integer attributes properly; minor comment additions and code reorganization for error handling |
| mssql_python/constants.py | Adds SQL_ATTR_QUERY_TIMEOUT constant for ODBC statement attribute configuration |
| tests/test_003_connection.py | Adds 6 new comprehensive test functions covering timeout behavior with long-running queries, multiple executions, cursor resets, compatibility, and edge cases |
| mssql_python/connection.py | Contains duplicate function/constant definitions and changes threading lock type; also includes minor documentation changes to encoding-related methods |
Comments suppressed due to low confidence (1)
mssql_python/connection.py:160
- Duplicate function and constant definitions detected. The
_validate_utf16_wchar_compatibilityfunction (lines 61-107 and 114-160) andUTF16_ENCODINGSconstant (lines 58, 111) are defined twice identically. This appears to be an accidental duplication that should be removed to avoid maintenance issues.
def _validate_utf16_wchar_compatibility(
encoding: str, wchar_type: int, context: str = "SQL_WCHAR"
) -> None:
"""
Validates UTF-16 encoding compatibility with SQL_WCHAR.
Centralizes the validation logic to eliminate duplication across setencoding/setdecoding.
Args:
encoding: The encoding string (already normalized to lowercase)
wchar_type: The SQL_WCHAR constant value to check against
context: Context string for error messages ('SQL_WCHAR', 'SQL_WCHAR ctype', etc.)
Raises:
ProgrammingError: If encoding is incompatible with SQL_WCHAR
"""
if encoding == "utf-16":
# UTF-16 with BOM is rejected due to byte order ambiguity
logger.warning("utf-16 with BOM rejected for %s", context)
raise ProgrammingError(
driver_error="UTF-16 with Byte Order Mark not supported for SQL_WCHAR",
ddbc_error=(
"Cannot use 'utf-16' encoding with SQL_WCHAR due to Byte Order Mark ambiguity. "
"Use 'utf-16le' or 'utf-16be' instead for explicit byte order."
),
)
elif encoding not in UTF16_ENCODINGS:
# Non-UTF-16 encodings are not supported with SQL_WCHAR
logger.warning(
"Non-UTF-16 encoding %s attempted with %s", sanitize_user_input(encoding), context
)
# Generate context-appropriate error messages
if "ctype" in context:
driver_error = f"SQL_WCHAR ctype only supports UTF-16 encodings"
ddbc_context = "SQL_WCHAR ctype"
else:
driver_error = f"SQL_WCHAR only supports UTF-16 encodings"
ddbc_context = "SQL_WCHAR"
raise ProgrammingError(
driver_error=driver_error,
ddbc_error=(
f"Cannot use encoding '{encoding}' with {ddbc_context}. "
f"SQL_WCHAR requires UTF-16 encodings (utf-16le, utf-16be)"
),
)
# Note: "utf-16" with BOM is NOT included as it's problematic for SQL_WCHAR
UTF16_ENCODINGS: frozenset[str] = frozenset(["utf-16le", "utf-16be"])
def _validate_utf16_wchar_compatibility(
encoding: str, wchar_type: int, context: str = "SQL_WCHAR"
) -> None:
"""
Validates UTF-16 encoding compatibility with SQL_WCHAR.
Centralizes the validation logic to eliminate duplication across setencoding/setdecoding.
Args:
encoding: The encoding string (already normalized to lowercase)
wchar_type: The SQL_WCHAR constant value to check against
context: Context string for error messages ('SQL_WCHAR', 'SQL_WCHAR ctype', etc.)
Raises:
ProgrammingError: If encoding is incompatible with SQL_WCHAR
"""
if encoding == "utf-16":
# UTF-16 with BOM is rejected due to byte order ambiguity
logger.warning("utf-16 with BOM rejected for %s", context)
raise ProgrammingError(
driver_error="UTF-16 with Byte Order Mark not supported for SQL_WCHAR",
ddbc_error=(
"Cannot use 'utf-16' encoding with SQL_WCHAR due to Byte Order Mark ambiguity. "
"Use 'utf-16le' or 'utf-16be' instead for explicit byte order."
),
)
elif encoding not in UTF16_ENCODINGS:
# Non-UTF-16 encodings are not supported with SQL_WCHAR
logger.warning(
"Non-UTF-16 encoding %s attempted with %s", sanitize_user_input(encoding), context
)
# Generate context-appropriate error messages
if "ctype" in context:
driver_error = f"SQL_WCHAR ctype only supports UTF-16 encodings"
ddbc_context = "SQL_WCHAR ctype"
else:
driver_error = f"SQL_WCHAR only supports UTF-16 encodings"
ddbc_context = "SQL_WCHAR"
raise ProgrammingError(
driver_error=driver_error,
ddbc_error=(
f"Cannot use encoding '{encoding}' with {ddbc_context}. "
f"SQL_WCHAR requires UTF-16 encodings (utf-16le, utf-16be)"
),
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/cursor.pyLines 705-713 705 timeout_value,
706 )
707 check_error(ddbc_sql_const.SQL_HANDLE_STMT.value, self.hstmt, ret)
708 logger.debug("Query timeout set to %d seconds", timeout_value)
! 709 except Exception as e: # pylint: disable=broad-exception-caught
710 logger.warning("Failed to set query timeout: %s", str(e))
711
712 def _reset_cursor(self) -> None:
713 """mssql_python/pybind/ddbc_bindings.cppLines 4400-4409 4400 ptr_value =
4401 reinterpret_cast<SQLPOINTER>(static_cast<SQLULEN>(value.cast<int64_t>()));
4402 } else {
4403 // For pointer attributes
! 4404 ptr_value = value.cast<SQLPOINTER>();
! 4405 }
4406 return SQLSetStmtAttr_ptr(stmt->get(), attr, ptr_value, 0);
4407 },
4408 "Set statement attributes");
4409 m.def("DDBCSQLGetTypeInfo", &SQLGetTypeInfo_Wrapper,📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.cpp: 66.3%
mssql_python.helpers.py: 67.5%
mssql_python.pybind.connection.connection.cpp: 73.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.connection.py: 83.7%
mssql_python.cursor.py: 84.3%
mssql_python.logging.py: 85.3%🔗 Quick Links
|
sumitmsft
approved these changes
Dec 10, 2025
gargsaumya
approved these changes
Dec 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
This pull request refactors how query timeouts are set for statement handles in the
mssql_python/cursor.pymodule. The main improvement is moving the logic for setting the query timeout from theexecutemethod to the cursor initialization process, ensuring the timeout is consistently applied whenever the statement handle is allocated or reset.Statement handle timeout management:
_set_timeoutmethod to set the query timeout attribute on the statement handle during cursor initialization, following best practices for performance. (mssql_python/cursor.py)executemethod, centralizing timeout management in the cursor lifecycle. (mssql_python/cursor.py)