Skip to content

Commit d2104d1

Browse files
committed
Resolving conflicts'
2 parents b85445d + eb95d2e commit d2104d1

File tree

10 files changed

+2252
-283
lines changed

10 files changed

+2252
-283
lines changed

eng/pipelines/pr-validation-pipeline.yml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,12 +1395,14 @@ jobs:
13951395
13961396
- script: |
13971397
# Create a Docker container for testing on x86_64
1398+
# TODO(AB#40901): Temporary pin to 3.22 due to msodbcsql ARM64 package arch mismatch
1399+
# Revert to alpine:latest once ODBC team releases fixed ARM64 package
13981400
docker run -d --name test-container-alpine \
13991401
--platform linux/amd64 \
14001402
-v $(Build.SourcesDirectory):/workspace \
14011403
-w /workspace \
14021404
--network bridge \
1403-
alpine:latest \
1405+
alpine:3.22 \
14041406
tail -f /dev/null
14051407
displayName: 'Create Alpine x86_64 container'
14061408
@@ -1637,12 +1639,14 @@ jobs:
16371639
16381640
- script: |
16391641
# Create a Docker container for testing on ARM64
1642+
# TODO(AB#40901): Temporary pin to 3.22 due to msodbcsql ARM64 package arch mismatch
1643+
# Revert to alpine:latest once ODBC team releases fixed ARM64 package
16401644
docker run -d --name test-container-alpine-arm64 \
16411645
--platform linux/arm64 \
16421646
-v $(Build.SourcesDirectory):/workspace \
16431647
-w /workspace \
16441648
--network bridge \
1645-
alpine:latest \
1649+
alpine:3.22 \
16461650
tail -f /dev/null
16471651
displayName: 'Create Alpine ARM64 container'
16481652

mssql_python/connection.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,59 @@
5858
UTF16_ENCODINGS: frozenset[str] = frozenset(["utf-16le", "utf-16be"])
5959

6060

61+
def _validate_utf16_wchar_compatibility(
62+
encoding: str, wchar_type: int, context: str = "SQL_WCHAR"
63+
) -> None:
64+
"""
65+
Validates UTF-16 encoding compatibility with SQL_WCHAR.
66+
67+
Centralizes the validation logic to eliminate duplication across setencoding/setdecoding.
68+
69+
Args:
70+
encoding: The encoding string (already normalized to lowercase)
71+
wchar_type: The SQL_WCHAR constant value to check against
72+
context: Context string for error messages ('SQL_WCHAR', 'SQL_WCHAR ctype', etc.)
73+
74+
Raises:
75+
ProgrammingError: If encoding is incompatible with SQL_WCHAR
76+
"""
77+
if encoding == "utf-16":
78+
# UTF-16 with BOM is rejected due to byte order ambiguity
79+
logger.warning("utf-16 with BOM rejected for %s", context)
80+
raise ProgrammingError(
81+
driver_error="UTF-16 with Byte Order Mark not supported for SQL_WCHAR",
82+
ddbc_error=(
83+
"Cannot use 'utf-16' encoding with SQL_WCHAR due to Byte Order Mark ambiguity. "
84+
"Use 'utf-16le' or 'utf-16be' instead for explicit byte order."
85+
),
86+
)
87+
elif encoding not in UTF16_ENCODINGS:
88+
# Non-UTF-16 encodings are not supported with SQL_WCHAR
89+
logger.warning(
90+
"Non-UTF-16 encoding %s attempted with %s", sanitize_user_input(encoding), context
91+
)
92+
93+
# Generate context-appropriate error messages
94+
if "ctype" in context:
95+
driver_error = f"SQL_WCHAR ctype only supports UTF-16 encodings"
96+
ddbc_context = "SQL_WCHAR ctype"
97+
else:
98+
driver_error = f"SQL_WCHAR only supports UTF-16 encodings"
99+
ddbc_context = "SQL_WCHAR"
100+
101+
raise ProgrammingError(
102+
driver_error=driver_error,
103+
ddbc_error=(
104+
f"Cannot use encoding '{encoding}' with {ddbc_context}. "
105+
f"SQL_WCHAR requires UTF-16 encodings (utf-16le, utf-16be)"
106+
),
107+
)
108+
109+
110+
# Note: "utf-16" with BOM is NOT included as it's problematic for SQL_WCHAR
111+
UTF16_ENCODINGS: frozenset[str] = frozenset(["utf-16le", "utf-16be"])
112+
113+
61114
def _validate_utf16_wchar_compatibility(
62115
encoding: str, wchar_type: int, context: str = "SQL_WCHAR"
63116
) -> None:

mssql_python/cursor.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2288,6 +2288,9 @@ def fetchall(self) -> List[Row]:
22882288
wchar_decoding.get("encoding", "utf-16le"),
22892289
)
22902290

2291+
# Check for errors
2292+
check_error(ddbc_sql_const.SQL_HANDLE_STMT.value, self.hstmt, ret)
2293+
22912294
if self.hstmt:
22922295
self.messages.extend(ddbc_bindings.DDBCSQLGetAllDiagRecords(self.hstmt))
22932296

mssql_python/pybind/connection/connection.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,19 @@ bool Connection::reset() {
308308
disconnect();
309309
return false;
310310
}
311+
312+
// SQL_ATTR_RESET_CONNECTION does NOT reset the transaction isolation level.
313+
// Explicitly reset it to the default (SQL_TXN_READ_COMMITTED) to prevent
314+
// isolation level settings from leaking between pooled connection usages.
315+
LOG("Resetting transaction isolation level to READ COMMITTED");
316+
ret = SQLSetConnectAttr_ptr(_dbcHandle->get(), SQL_ATTR_TXN_ISOLATION,
317+
(SQLPOINTER)SQL_TXN_READ_COMMITTED, SQL_IS_INTEGER);
318+
if (!SQL_SUCCEEDED(ret)) {
319+
LOG("Failed to reset transaction isolation level (ret=%d). Marking as dead.", ret);
320+
disconnect();
321+
return false;
322+
}
323+
311324
updateLastUsed();
312325
return true;
313326
}

mssql_python/pybind/ddbc_bindings.cpp

Lines changed: 8 additions & 4 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",

mssql_python/pybind/unix_utils.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const char* kOdbcEncoding = "utf-16-le"; // ODBC uses UTF-16LE for SQLWCHAR
1818
const size_t kUcsLength = 2; // SQLWCHAR is 2 bytes on all platforms
1919

2020
// Function to convert SQLWCHAR strings to std::wstring on macOS
21+
// THREAD-SAFE: Uses thread_local converter to avoid std::wstring_convert race conditions
2122
std::wstring SQLWCHARToWString(const SQLWCHAR* sqlwStr, size_t length = SQL_NTS) {
2223
if (!sqlwStr) {
2324
return std::wstring();
@@ -40,9 +41,13 @@ std::wstring SQLWCHARToWString(const SQLWCHAR* sqlwStr, size_t length = SQL_NTS)
4041

4142
// Convert UTF-16LE to std::wstring (UTF-32 on macOS)
4243
try {
43-
// Use C++11 codecvt to convert between UTF-16LE and wstring
44-
std::wstring_convert<std::codecvt_utf8_utf16<wchar_t, 0x10ffff, std::little_endian>>
44+
// CRITICAL FIX: Use thread_local to make std::wstring_convert thread-safe
45+
// std::wstring_convert is NOT thread-safe and its use is deprecated in C++17
46+
// Each thread gets its own converter instance, eliminating race conditions
47+
thread_local std::wstring_convert<
48+
std::codecvt_utf8_utf16<wchar_t, 0x10ffff, std::little_endian>>
4549
converter;
50+
4651
std::wstring result = converter.from_bytes(
4752
reinterpret_cast<const char*>(utf16Bytes.data()),
4853
reinterpret_cast<const char*>(utf16Bytes.data() + utf16Bytes.size()));
@@ -59,11 +64,16 @@ std::wstring SQLWCHARToWString(const SQLWCHAR* sqlwStr, size_t length = SQL_NTS)
5964
}
6065

6166
// Function to convert std::wstring to SQLWCHAR array on macOS
67+
// THREAD-SAFE: Uses thread_local converter to avoid std::wstring_convert race conditions
6268
std::vector<SQLWCHAR> WStringToSQLWCHAR(const std::wstring& str) {
6369
try {
64-
// Convert wstring (UTF-32 on macOS) to UTF-16LE bytes
65-
std::wstring_convert<std::codecvt_utf8_utf16<wchar_t, 0x10ffff, std::little_endian>>
70+
// CRITICAL FIX: Use thread_local to make std::wstring_convert thread-safe
71+
// std::wstring_convert is NOT thread-safe and its use is deprecated in C++17
72+
// Each thread gets its own converter instance, eliminating race conditions
73+
thread_local std::wstring_convert<
74+
std::codecvt_utf8_utf16<wchar_t, 0x10ffff, std::little_endian>>
6675
converter;
76+
6777
std::string utf16Bytes = converter.to_bytes(str);
6878

6979
// Convert the bytes to SQLWCHAR array

tests/test_003_connection.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2049,7 +2049,8 @@ def test_timeout_long_query(db_connection):
20492049
# Method 2: Try with WAITFOR
20502050
start_time = time.perf_counter()
20512051
cursor.execute("WAITFOR DELAY '00:00:05'")
2052-
cursor.fetchall()
2052+
# Don't call fetchall() on WAITFOR - it doesn't return results
2053+
# The execute itself should timeout
20532054
elapsed_time = time.perf_counter() - start_time
20542055

20552056
# If we still get here, try one more approach

tests/test_004_cursor.py

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10114,7 +10114,11 @@ def test_procedures_result_set_info(cursor, db_connection):
1011410114

1011510115
# Test execution of the procedures to verify they work
1011610116
cursor.execute("EXEC pytest_proc_schema.test_no_results")
10117-
assert cursor.fetchall() == [], "test_no_results should return no results"
10117+
# Procedures with no results should have no description and calling fetchall() should raise an error
10118+
assert (
10119+
cursor.description is None,
10120+
), "test_no_results should have no description (no result set)"
10121+
# Don't call fetchall() on procedures with no results - this is invalid in ODBC
1011810122

1011910123
cursor.execute("EXEC pytest_proc_schema.test_one_result")
1012010124
rows = cursor.fetchall()
@@ -14656,6 +14660,100 @@ def test_fixed_length_binary_type(cursor, db_connection):
1465614660
pytest.fail(f"Fixed-length BINARY test failed: {e}")
1465714661

1465814662

14663+
def test_fetchall_with_integrity_constraint(cursor, db_connection):
14664+
"""
14665+
Test that UNIQUE constraint errors are appropriately triggered for multi-row INSERT
14666+
statements that use OUTPUT inserted.
14667+
14668+
This test covers a specific case where SQL Server's protocol has error conditions
14669+
that do not become apparent until rows are fetched, requiring special handling
14670+
in fetchall().
14671+
"""
14672+
try:
14673+
# Setup table with unique constraint
14674+
cursor.execute("DROP TABLE IF EXISTS #uniq_cons_test")
14675+
cursor.execute(
14676+
"""
14677+
CREATE TABLE #uniq_cons_test (
14678+
id INTEGER NOT NULL IDENTITY,
14679+
data VARCHAR(50) NULL,
14680+
PRIMARY KEY (id),
14681+
UNIQUE (data)
14682+
)
14683+
"""
14684+
)
14685+
14686+
# Insert initial row - should work
14687+
cursor.execute(
14688+
"INSERT INTO #uniq_cons_test (data) OUTPUT inserted.id VALUES (?)", ("the data 1",)
14689+
)
14690+
cursor.fetchall() # Complete the operation
14691+
14692+
# Test single row duplicate - should raise IntegrityError
14693+
with pytest.raises(mssql_python.IntegrityError):
14694+
cursor.execute(
14695+
"INSERT INTO #uniq_cons_test (data) OUTPUT inserted.id VALUES (?)", ("the data 1",)
14696+
)
14697+
cursor.fetchall() # Error should be detected here
14698+
14699+
# Insert two valid rows in one statement - should work
14700+
cursor.execute(
14701+
"INSERT INTO #uniq_cons_test (data) OUTPUT inserted.id VALUES (?), (?)",
14702+
("the data 2", "the data 3"),
14703+
)
14704+
cursor.fetchall()
14705+
14706+
# Verify current state
14707+
cursor.execute("SELECT * FROM #uniq_cons_test ORDER BY id")
14708+
rows = cursor.fetchall()
14709+
expected_before = [(1, "the data 1"), (3, "the data 2"), (4, "the data 3")]
14710+
actual_before = [tuple(row) for row in rows]
14711+
assert actual_before == expected_before
14712+
14713+
# THE CRITICAL TEST: Multi-row INSERT with duplicate values
14714+
# This should raise IntegrityError during fetchall()
14715+
with pytest.raises(mssql_python.IntegrityError):
14716+
cursor.execute(
14717+
"INSERT INTO #uniq_cons_test (data) OUTPUT inserted.id VALUES (?), (?)",
14718+
("the data 4", "the data 4"),
14719+
) # Duplicate in same statement
14720+
14721+
# The error should be detected HERE during fetchall()
14722+
cursor.fetchall()
14723+
14724+
# Verify table state after failed multi-row insert
14725+
cursor.execute("SELECT * FROM #uniq_cons_test ORDER BY id")
14726+
rows = cursor.fetchall()
14727+
expected_after = [(1, "the data 1"), (3, "the data 2"), (4, "the data 3")]
14728+
actual_after = [tuple(row) for row in rows]
14729+
assert actual_after == expected_after, "Table should be unchanged after failed insert"
14730+
14731+
# Test timing: execute() should succeed, error detection happens in fetchall()
14732+
try:
14733+
cursor.execute(
14734+
"INSERT INTO #uniq_cons_test (data) OUTPUT inserted.id VALUES (?), (?)",
14735+
("the data 5", "the data 5"),
14736+
)
14737+
execute_succeeded = True
14738+
except Exception:
14739+
execute_succeeded = False
14740+
14741+
assert execute_succeeded, "execute() should succeed, error detection happens in fetchall()"
14742+
14743+
# fetchall() should raise the IntegrityError
14744+
with pytest.raises(mssql_python.IntegrityError):
14745+
cursor.fetchall()
14746+
14747+
except Exception as e:
14748+
pytest.fail(f"Integrity constraint multi-row test failed: {e}")
14749+
finally:
14750+
# Cleanup
14751+
try:
14752+
cursor.execute("DROP TABLE IF EXISTS #uniq_cons_test")
14753+
except:
14754+
pass
14755+
14756+
1465914757
def test_close(db_connection):
1466014758
"""Test closing the cursor"""
1466114759
try:

0 commit comments

Comments
 (0)