Skip to content

Commit 1d5981b

Browse files
committed
Resolving comments
1 parent e145104 commit 1d5981b

File tree

3 files changed

+391
-13
lines changed

3 files changed

+391
-13
lines changed

mssql_python/connection.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,12 @@ def __init__(
293293

294294
# Initialize encoding/decoding settings lock for thread safety
295295
# This lock protects both _encoding_settings and _decoding_settings dictionaries
296-
# to prevent race conditions when multiple threads are reading/writing encoding settings
297-
self._encoding_lock = threading.RLock() # RLock allows recursive locking
296+
# from concurrent modification. We use a simple Lock (not RLock) because:
297+
# - Write operations (setencoding/setdecoding) replace the entire dict atomically
298+
# - Read operations (getencoding/getdecoding) return a copy, so they're safe
299+
# - No recursive locking is needed in our usage pattern
300+
# This is more performant than RLock for the multiple-readers-single-writer pattern
301+
self._encoding_lock = threading.Lock()
298302

299303
# Initialize search escape character
300304
self._searchescape = None
@@ -559,6 +563,7 @@ def getencoding(self) -> Dict[str, Union[str, int]]:
559563
560564
Note:
561565
This method is thread-safe and can be called from multiple threads concurrently.
566+
Returns a copy of the settings to prevent external modification.
562567
"""
563568
if self._closed:
564569
raise InterfaceError(
@@ -725,6 +730,7 @@ def getdecoding(self, sqltype: int) -> Dict[str, Union[str, int]]:
725730
726731
Note:
727732
This method is thread-safe and can be called from multiple threads concurrently.
733+
Returns a copy of the settings to prevent external modification.
728734
"""
729735
if self._closed:
730736
raise InterfaceError(

mssql_python/pybind/ddbc_bindings.cpp

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -757,24 +757,28 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
757757
return rc;
758758
}
759759
SQL_NUMERIC_STRUCT* numericPtr = reinterpret_cast<SQL_NUMERIC_STRUCT*>(dataPtr);
760-
rc = SQLSetDescField_ptr(hDesc, 1, SQL_DESC_PRECISION,
761-
reinterpret_cast<SQLPOINTER>(static_cast<uintptr_t>(numericPtr->precision)), 0);
760+
rc = SQLSetDescField_ptr(
761+
hDesc, 1, SQL_DESC_PRECISION,
762+
reinterpret_cast<SQLPOINTER>(static_cast<uintptr_t>(numericPtr->precision)), 0);
762763
if (!SQL_SUCCEEDED(rc)) {
763764
LOG("BindParameters: SQLSetDescField(SQL_DESC_PRECISION) "
764765
"failed for param[%d] - SQLRETURN=%d",
765766
paramIndex, rc);
766767
return rc;
767768
}
768769

769-
rc = SQLSetDescField_ptr(hDesc, 1, SQL_DESC_SCALE, reinterpret_cast<SQLPOINTER>(static_cast<intptr_t>(numericPtr->scale)), 0);
770+
rc = SQLSetDescField_ptr(
771+
hDesc, 1, SQL_DESC_SCALE,
772+
reinterpret_cast<SQLPOINTER>(static_cast<intptr_t>(numericPtr->scale)), 0);
770773
if (!SQL_SUCCEEDED(rc)) {
771774
LOG("BindParameters: SQLSetDescField(SQL_DESC_SCALE) failed "
772775
"for param[%d] - SQLRETURN=%d",
773776
paramIndex, rc);
774777
return rc;
775778
}
776779

777-
rc = SQLSetDescField_ptr(hDesc, 1, SQL_DESC_DATA_PTR, reinterpret_cast<SQLPOINTER>(numericPtr), 0);
780+
rc = SQLSetDescField_ptr(hDesc, 1, SQL_DESC_DATA_PTR,
781+
reinterpret_cast<SQLPOINTER>(numericPtr), 0);
778782
if (!SQL_SUCCEEDED(rc)) {
779783
LOG("BindParameters: SQLSetDescField(SQL_DESC_DATA_PTR) failed "
780784
"for param[%d] - SQLRETURN=%d",
@@ -2833,8 +2837,9 @@ py::object FetchLobColumnData(SQLHSTMT hStmt, SQLUSMALLINT colIndex, SQLSMALLINT
28332837
}
28342838

28352839
// For SQL_C_CHAR data, decode using the specified encoding (like pyodbc does)
2840+
// Create py::bytes once to avoid double allocation
2841+
py::bytes raw_bytes(buffer.data(), buffer.size());
28362842
try {
2837-
py::bytes raw_bytes(buffer.data(), buffer.size());
28382843
py::object decoded = raw_bytes.attr("decode")(charEncoding, "strict");
28392844
LOG("FetchLobColumnData: Decoded narrow string with '%s' - %zu bytes -> %zu chars for "
28402845
"column %d",
@@ -2844,7 +2849,7 @@ py::object FetchLobColumnData(SQLHSTMT hStmt, SQLUSMALLINT colIndex, SQLSMALLINT
28442849
LOG_ERROR("FetchLobColumnData: Failed to decode with '%s' for column %d: %s",
28452850
charEncoding.c_str(), colIndex, e.what());
28462851
// Return raw bytes as fallback
2847-
return py::bytes(buffer.data(), buffer.size());
2852+
return raw_bytes;
28482853
}
28492854
}
28502855

@@ -2912,9 +2917,10 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
29122917
// SQLGetData will null-terminate the data
29132918
// Use Python's codec system to decode bytes with specified encoding
29142919
// (like pyodbc does)
2920+
// Create py::bytes once to avoid double allocation
2921+
py::bytes raw_bytes(reinterpret_cast<char*>(dataBuffer.data()),
2922+
static_cast<size_t>(dataLen));
29152923
try {
2916-
py::bytes raw_bytes(reinterpret_cast<char*>(dataBuffer.data()),
2917-
static_cast<size_t>(dataLen));
29182924
py::object decoded =
29192925
raw_bytes.attr("decode")(charEncoding, "strict");
29202926
row.append(decoded);
@@ -2926,8 +2932,6 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
29262932
"SQLGetData: Failed to decode CHAR column %d with '%s': %s",
29272933
i, charEncoding.c_str(), e.what());
29282934
// Return raw bytes as fallback
2929-
py::bytes raw_bytes(reinterpret_cast<char*>(dataBuffer.data()),
2930-
static_cast<size_t>(dataLen));
29312935
row.append(raw_bytes);
29322936
}
29332937
} else {

0 commit comments

Comments
 (0)