Skip to content

Commit 1566c67

Browse files
author
Theekshna Kotian
committed
Merged PR 5282: Check if statement is prepared before calling SQLExecute
- If you send a statement with param markers & params list to DDBCSQLExecute, with usePrepare=false, the C++ code will bind the params & then call SQLExecute without preparing the statement. The ODBC code for SQLExecute crashes due to a memory violation. - SQLExecute is supposed to be called only after preparing a statement. Ideally this case should give an error. But it's causing a crash in python driver - RCA - after attaching debugger & stepping over the DDBC & ODBC code, it looks like the check to error out when user executes without prepare is handled by windows Driver Manager. So ODBC code assumes this as a given & doesn't have checks around it. This is causing memory violation. Since Python Driver will not use Driver Manager, this task adds a check inside Python Driver to make sure that we don't call SQLExecute API on unprepared statements. **Implementation details:** - ODBC has internal fields to tell if a statement is already prepared or not, but it does not expose these APIs to external users. So we cannot use ODBC APIs to tell if a statement is prepared. - The only other alternative is populate the is_stmt_prepared info in C++ code, but save this state for every statement in cursor class of Python code. This is what I've implemented **This PR also has following changes:** - Removed parameter binding code from SQLExecute_wrap & refactored it into BindParameters function. - Provide Release/Debug as parameter to build.bat script ---- ---- #### AI description (iteration 1) #### PR Classification Bug fix to prevent memory violation by ensuring SQLExecute is called only on prepared statements. #### PR Summary This pull request adds a check to ensure that SQLExecute is not called on unprepared statements, preventing a memory violation crash in the Python driver. - `mssql_python/pybind/ddbc_bindings.cpp`: Added a check to ensure statements are prepared before calling SQLExecute and refactored parameter binding into a separate function. - `mssql_python/pybind/build.bat`: Updated build script to support different build types (Debug/Release). <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #33723
1 parent 5d62207 commit 1566c67

4 files changed

Lines changed: 323 additions & 251 deletions

File tree

mssql_python/cursor.py

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ def __init__(self, connection) -> None:
5757
self.arraysize = 1
5858
self.buffer_length = 1024 # Default buffer length for string data
5959
self.closed = False # Flag to indicate if the cursor is closed
60+
self.last_executed_stmt = "" # Stores the last statement executed by this cursor
61+
self.is_stmt_prepared = [False] # Indicates if last_executed_stmt was prepared by ddbc shim.
62+
# Is a list instead of a bool coz bools in Python are immutable.
63+
# Hence, we can't pass around bools by reference & modify them.
64+
# Therefore, it must be a list with exactly one bool element.
6065

6166
def _is_unicode_string(self, param):
6267
"""
@@ -433,11 +438,20 @@ def execute(self, operation: str, *parameters, use_prepare: bool = True, reset_c
433438
for i, param in enumerate(parameters):
434439
paraminfo = self._create_parameter_types_list(param, ParamInfo, parameters, i)
435440
parameters_type.append(paraminfo)
441+
442+
# TODO: Use a more sophisticated string compare that handles redundant spaces etc.
443+
# Also consider storing last query's hash instead of full query string. This will help
444+
# in low-memory conditions (Ex: huge number of parallel queries with huge query string sizes)
445+
if operation != self.last_executed_stmt:
446+
# Executing a new statement. Reset is_stmt_prepared to false
447+
self.is_stmt_prepared = [False]
436448
'''
437449
Execute SQL Statement - (SQLExecute)
438450
'''
439-
ret = ddbc_bindings.DDBCSQLExecute(self.hstmt.value, operation, parameters, parameters_type, use_prepare)
451+
ret = ddbc_bindings.DDBCSQLExecute(self.hstmt.value, operation, parameters, parameters_type,
452+
self.is_stmt_prepared, use_prepare)
440453
check_error(odbc_sql_const.SQL_HANDLE_STMT.value, self.hstmt.value, ret)
454+
self.last_executed_stmt = operation
441455
except Exception as e:
442456
if ENABLE_LOGGING:
443457
logging.error("An error occurred while executing query: %s", e)
@@ -460,13 +474,23 @@ def executemany(self, operation: str, seq_of_parameters: list) -> None:
460474
# Reset the cursor once before the loop
461475
self._reset_cursor()
462476

477+
first_execution = True
463478
for parameters in seq_of_parameters:
464479
# Execute the operation with the current set of parameters without
465480
# Converting the parameters to a list
466481
parameters = list(parameters)
467482
if ENABLE_LOGGING:
468483
logging.info("Executing query with parameters: %s", parameters)
469-
self.execute(operation, parameters, use_prepare=True, reset_cursor=False)
484+
# Prepare the statement only during first execution. From second time
485+
# onwards, skip preparing and directly execute. This helps avoid
486+
# unnecessary 'prepare' network calls.
487+
if first_execution:
488+
prepare_stmt = True
489+
first_execution = False
490+
else:
491+
prepare_stmt = False
492+
# Execute statement with one parameter set
493+
self.execute(operation, parameters, use_prepare=prepare_stmt, reset_cursor=False)
470494
except Exception as e:
471495
if ENABLE_LOGGING:
472496
logging.error("An error occurred while executing multiple queries: %s", e)

mssql_python/pybind/build.bat

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,21 @@
1+
:: Usage -
2+
:: 1) "cd mssql_python/pybind"
3+
:: 2)a) "build.bat" - Generates debug build of DDBC bindings
4+
:: 2)b) "build.bat Release" - Generates release build of DDBC bindings
5+
:: 2)c) "build.bat Debug" - Generates debug build of DDBC bindings
6+
7+
rmdir /s /q build
18
mkdir build
29
cd build
310
del CMakeCache.txt
4-
cmake -DPython3_EXECUTABLE="python3" -DCMAKE_BUILD_TYPE=Release ..
5-
msbuild ddbc_bindings.sln /p:Configuration=Release
6-
move /Y Release\ddbc_bindings.pyd ..\..\ddbc_bindings.pyd
11+
12+
IF "%1"=="" (
13+
set BUILD_TYPE=Debug
14+
) ELSE (
15+
set BUILD_TYPE=%1
16+
)
17+
18+
cmake -DPython3_EXECUTABLE="python3" -DCMAKE_BUILD_TYPE=%BUILD_TYPE% ..
19+
msbuild ddbc_bindings.sln /p:Configuration=%BUILD_TYPE%
20+
move /Y %BUILD_TYPE%\ddbc_bindings.pyd ..\..\ddbc_bindings.pyd
721
cd ..
8-
rmdir /s /q build

0 commit comments

Comments
 (0)