Skip to content

Commit 6a2aad3

Browse files
committed
Resolving comments
1 parent ee5b295 commit 6a2aad3

File tree

4 files changed

+131
-41
lines changed

4 files changed

+131
-41
lines changed

mssql_python/__init__.py

Lines changed: 43 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import threading
77
import locale
88
import sys
9+
import types
910

1011
# Exceptions
1112
# https://www.python.org/dev/peps/pep-0249/#exceptions
@@ -41,13 +42,8 @@ def __init__(self):
4142
def get_settings():
4243
"""Return the global settings object"""
4344
with _settings_lock:
44-
_settings.lowercase = bool(lowercase)
45-
_settings.native_uuid = bool(native_uuid)
4645
return _settings
4746

48-
lowercase = _settings.lowercase # Default is False
49-
native_uuid = _settings.native_uuid # Default is False
50-
5147
# Set the initial decimal separator in C++
5248
from .ddbc_bindings import DDBCSetDecimalSeparator
5349
DDBCSetDecimalSeparator(_settings.decimal_separator)
@@ -172,33 +168,6 @@ def pooling(max_size=100, idle_timeout=600, enabled=True):
172168

173169
_original_module_setattr = sys.modules[__name__].__setattr__
174170

175-
def _custom_setattr(name, value):
176-
if name == 'lowercase':
177-
# Strict boolean type check for lowercase
178-
if not isinstance(value, bool):
179-
raise ValueError("lowercase must be a boolean value (True or False)")
180-
181-
with _settings_lock:
182-
_settings.lowercase = value
183-
# Update the module's lowercase variable
184-
_original_module_setattr(name, _settings.lowercase)
185-
elif name == 'native_uuid':
186-
187-
# Strict boolean type check for native_uuid
188-
if not isinstance(value, bool):
189-
raise ValueError("native_uuid must be a boolean value (True or False)")
190-
191-
with _settings_lock:
192-
_settings.native_uuid = value
193-
# Update the module's native_uuid variable
194-
_original_module_setattr(name, _settings.native_uuid)
195-
else:
196-
_original_module_setattr(name, value)
197-
198-
# Replace the module's __setattr__ with our custom version
199-
sys.modules[__name__].__setattr__ = _custom_setattr
200-
201-
202171
# Export SQL constants at module level
203172
SQL_CHAR = ConstantsDDBC.SQL_CHAR.value
204173
SQL_VARCHAR = ConstantsDDBC.SQL_VARCHAR.value
@@ -274,3 +243,45 @@ def get_info_constants():
274243
"""
275244
return {name: member.value for name, member in GetInfoConstants.__members__.items()}
276245

246+
# Create a custom module class that uses properties instead of __setattr__
247+
class _MSSQLModule(types.ModuleType):
248+
@property
249+
def native_uuid(self):
250+
return _settings.native_uuid
251+
252+
@native_uuid.setter
253+
def native_uuid(self, value):
254+
if not isinstance(value, bool):
255+
raise ValueError("native_uuid must be a boolean value")
256+
with _settings_lock:
257+
_settings.native_uuid = value
258+
259+
@property
260+
def lowercase(self):
261+
return _settings.lowercase
262+
263+
@lowercase.setter
264+
def lowercase(self, value):
265+
if not isinstance(value, bool):
266+
raise ValueError("lowercase must be a boolean value")
267+
with _settings_lock:
268+
_settings.lowercase = value
269+
270+
# Replace the current module with our custom module class
271+
old_module = sys.modules[__name__]
272+
new_module = _MSSQLModule(__name__)
273+
274+
# Copy all existing attributes to the new module
275+
for attr_name in dir(old_module):
276+
if attr_name != "__class__":
277+
try:
278+
setattr(new_module, attr_name, getattr(old_module, attr_name))
279+
except AttributeError:
280+
pass
281+
282+
# Replace the module in sys.modules
283+
sys.modules[__name__] = new_module
284+
285+
# Initialize property values
286+
lowercase = _settings.lowercase
287+
native_uuid = _settings.native_uuid

mssql_python/cursor.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,8 @@ def execute(
10001000
# After successful execution, initialize description if there are results
10011001
column_metadata = []
10021002
try:
1003+
# ODBC specification guarantees that column metadata is available immediately after
1004+
# a successful SQLExecute/SQLExecDirect for the first result set
10031005
ddbc_bindings.DDBCSQLDescribeCol(self.hstmt, column_metadata)
10041006
self._initialize_description(column_metadata)
10051007
except Exception as e:
@@ -1014,11 +1016,15 @@ def execute(
10141016
'lowercase': settings.lowercase,
10151017
'native_uuid': settings.native_uuid
10161018
}
1017-
# Identify UUID columns (SQL_GUID = -11)
1019+
# Identify UUID columns based on Python type in description[1]
1020+
# This relies on _map_data_type correctly mapping SQL_GUID to uuid.UUID
10181021
self._uuid_indices = []
10191022
for i, desc in enumerate(self.description):
10201023
if desc and desc[1] == uuid.UUID: # Column type code at index 1
10211024
self._uuid_indices.append(i)
1025+
# Verify we have complete description tuples (7 items per PEP-249)
1026+
elif desc and len(desc) != 7:
1027+
log('warning', f"Column description at index {i} has incorrect tuple length: {len(desc)}")
10221028
self.rowcount = -1
10231029
self._reset_rownumber()
10241030
else:

mssql_python/row.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from mssql_python import get_settings
2+
from mssql_python.constants import ConstantsDDBC
23
import uuid
34

45
class Row:
@@ -122,6 +123,18 @@ def _apply_output_converters(self, values):
122123

123124
converted_values = list(values)
124125

126+
# Map SQL type codes to appropriate byte sizes
127+
int_size_map = {
128+
# SQL_TINYINT
129+
ConstantsDDBC.SQL_TINYINT.value: 1,
130+
# SQL_SMALLINT
131+
ConstantsDDBC.SQL_SMALLINT.value: 2,
132+
# SQL_INTEGER
133+
ConstantsDDBC.SQL_INTEGER.value: 4,
134+
# SQL_BIGINT
135+
ConstantsDDBC.SQL_BIGINT.value: 8
136+
}
137+
125138
for i, (value, desc) in enumerate(zip(values, self._description)):
126139
if desc is None or value is None:
127140
continue
@@ -135,7 +148,6 @@ def _apply_output_converters(self, values):
135148
# If no converter found for the SQL type but the value is a string or bytes,
136149
# try the WVARCHAR converter as a fallback
137150
if converter is None and isinstance(value, (str, bytes)):
138-
from mssql_python.constants import ConstantsDDBC
139151
converter = self._cursor.connection.get_output_converter(ConstantsDDBC.SQL_WVARCHAR.value)
140152

141153
# If we found a converter, apply it
@@ -148,18 +160,26 @@ def _apply_output_converters(self, values):
148160
value_bytes = value.encode('utf-16-le')
149161
converted_values[i] = converter(value_bytes)
150162
elif isinstance(value, int):
151-
# For integers, we'll convert to bytes
152-
value_bytes = value.to_bytes(8, byteorder='little')
153-
converted_values[i] = converter(value_bytes)
163+
# Get appropriate byte size for this integer type
164+
byte_size = int_size_map.get(sql_type, 8)
165+
try:
166+
# Use signed=True to properly handle negative values
167+
value_bytes = value.to_bytes(byte_size, byteorder='little', signed=True)
168+
converted_values[i] = converter(value_bytes)
169+
except OverflowError as e:
170+
# Log specific overflow error with details to help diagnose the issue
171+
if hasattr(self._cursor, 'log'):
172+
self._cursor.log('warning',
173+
f'Integer overflow: value {value} does not fit in {byte_size} bytes for SQL type {sql_type}')
174+
# Keep the original value in this case
154175
else:
155176
# Pass the value directly for other types
156177
converted_values[i] = converter(value)
157178
except Exception as e:
158179
# Log the exception for debugging without leaking sensitive data
159180
if hasattr(self._cursor, 'log'):
160-
self._cursor.log('debug', f'Exception occurred in output converter: {type(e).__name__}', exc_info=True)
181+
self._cursor.log('warning', f'Exception in output converter: {type(e).__name__} for SQL type {sql_type}')
161182
# If conversion fails, keep the original value
162-
pass
163183

164184
return converted_values
165185

tests/test_001_globals.py

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import random
1515

1616
# Import global variables from the repository
17-
from mssql_python import apilevel, threadsafety, paramstyle, lowercase, getDecimalSeparator, setDecimalSeparator
17+
from mssql_python import apilevel, threadsafety, paramstyle, lowercase, getDecimalSeparator, setDecimalSeparator, native_uuid
1818

1919
def test_apilevel():
2020
# Check if apilevel has the expected value
@@ -553,4 +553,57 @@ def separator_reader_worker():
553553
finally:
554554
# Always make sure to clean up
555555
stop_event.set()
556-
setDecimalSeparator(original_separator)
556+
setDecimalSeparator(original_separator)
557+
558+
def test_native_uuid_type_validation():
559+
"""Test that native_uuid only accepts boolean values"""
560+
# Save original value
561+
original = mssql_python.native_uuid
562+
563+
try:
564+
# Test valid values
565+
mssql_python.native_uuid = True
566+
assert mssql_python.native_uuid is True
567+
568+
mssql_python.native_uuid = False
569+
assert mssql_python.native_uuid is False
570+
571+
# Test invalid types
572+
invalid_values = [
573+
1, 0, "True", "False", None, [], {},
574+
"yes", "no", "t", "f"
575+
]
576+
577+
for value in invalid_values:
578+
with pytest.raises(ValueError, match="native_uuid must be a boolean value"):
579+
mssql_python.native_uuid = value
580+
581+
finally:
582+
# Restore original value
583+
mssql_python.native_uuid = original
584+
585+
def test_lowercase_type_validation():
586+
"""Test that lowercase only accepts boolean values"""
587+
# Save original value
588+
original = mssql_python.lowercase
589+
590+
try:
591+
# Test valid values
592+
mssql_python.lowercase = True
593+
assert mssql_python.lowercase is True
594+
595+
mssql_python.lowercase = False
596+
assert mssql_python.lowercase is False
597+
598+
# Test invalid types
599+
invalid_values = [
600+
1, 0, "True", "False", None, [], {},
601+
"yes", "no", "t", "f"
602+
]
603+
604+
for value in invalid_values:
605+
with pytest.raises(ValueError, match="lowercase must be a boolean value"):
606+
mssql_python.lowercase = value
607+
finally:
608+
# Restore original value
609+
mssql_python.lowercase = original

0 commit comments

Comments
 (0)