Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 53
PERF: Optimize execute() hot path: soft reset, prepare caching, and guarded diagnostics #528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
88329a08a9e16ba1632bf18b7517File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -12,6 +12,7 @@ | ||
| # pylint: disable=too-many-lines # Large file due to comprehensive DB-API 2.0 implementation | ||
| import decimal | ||
| import logging | ||
| import uuid | ||
| import datetime | ||
| import warnings | ||
| @@ -747,6 +748,24 @@ def _reset_cursor(self) -> None: | ||
| # Reinitialize the statement handle | ||
| self._initialize_cursor() | ||
| self.is_stmt_prepared = [False] | ||
| def _soft_reset_cursor(self) -> None: | ||
| """Lightweight reset: close cursor and unbind params without freeing the HSTMT. | ||
| Preserves the prepared statement plan on the server so repeated | ||
| executions of the same SQL skip SQLPrepare entirely. | ||
| """ | ||
| if self.hstmt: | ||
| ret = ddbc_bindings.DDBCSQLResetStmt(self.hstmt) | ||
| try: | ||
| check_error(ddbc_sql_const.SQL_HANDLE_STMT.value, self.hstmt, ret) | ||
| except Exception: | ||
| logger.warning("_soft_reset_cursor failed; falling back to full reset") | ||
| self._reset_cursor() | ||
| self.last_executed_stmt = "" | ||
| return | ||
| self._clear_rownumber() | ||
| def close(self) -> None: | ||
| """ | ||
| @@ -1363,8 +1382,10 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state | ||
| self._check_closed() # Check if the cursor is closed | ||
| if reset_cursor: | ||
saurabh500 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| logger.debug("execute: Resetting cursor state") | ||
| self._reset_cursor() | ||
| if self.hstmt: | ||
| self._soft_reset_cursor() | ||
| else: | ||
| self._reset_cursor() | ||
| else: | ||
| # Close just the ODBC cursor (not the statement handle) so the | ||
| # prepared plan can be reused. SQLFreeStmt(SQL_CLOSE) releases | ||
| @@ -1409,20 +1430,22 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state | ||
| # Check if single parameter is a nested container that should be unwrapped | ||
| # e.g., execute("SELECT ?", (value,)) vs execute("SELECT ?, ?", ((1, 2),)) | ||
| if isinstance(parameters, tuple) and len(parameters) == 1: | ||
| # Could be either (value,) for single param or ((tuple),) for nested | ||
| # Check if it's a nested container | ||
| if isinstance(parameters[0], (tuple, list, dict)): | ||
| actual_params = parameters[0] | ||
| else: | ||
| actual_params = parameters | ||
| else: | ||
| actual_params = parameters | ||
| # Convert parameters based on detected style | ||
| operation, converted_params = detect_and_convert_parameters(operation, actual_params) | ||
| # Convert back to list format expected by the binding code | ||
| parameters = list(converted_params) | ||
| # Skip detect_and_convert_parameters when re-executing the same SQL — | ||
| # the parameter style (qmark vs pyformat) won't change between calls. | ||
| if operation == self.last_executed_stmt and isinstance(actual_params, (tuple, list)): | ||
| parameters = list(actual_params) | ||
| else: | ||
| operation, converted_params = detect_and_convert_parameters( | ||
| operation, actual_params | ||
| ) | ||
| parameters = list(converted_params) | ||
bewithgaurav marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| else: | ||
| parameters = [] | ||
| @@ -1450,35 +1473,36 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state | ||
| paraminfo = self._create_parameter_types_list(param, param_info, parameters, i) | ||
| parameters_type.append(paraminfo) | ||
| # TODO: Use a more sophisticated string compare that handles redundant spaces etc. | ||
| # Also consider storing last query's hash instead of full query string. This will help | ||
| # in low-memory conditions | ||
| # (Ex: huge number of parallel queries with huge query string sizes) | ||
| if operation != self.last_executed_stmt: | ||
| # Executing a new statement. Reset is_stmt_prepared to false | ||
| # Prepare caching: skip SQLPrepare when re-executing the same SQL | ||
| # with parameters. The HSTMT is reused via _soft_reset_cursor, so the | ||
| # server-side plan from the previous SQLPrepare is still valid. | ||
| same_sql = parameters and operation == self.last_executed_stmt and self.is_stmt_prepared[0] | ||
| if not same_sql: | ||
| self.is_stmt_prepared = [False] | ||
| effective_use_prepare = use_prepare and not same_sql | ||
saurabh500 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| for i, param in enumerate(parameters): | ||
| logger.debug( | ||
| """Parameter number: %s, Parameter: %s, | ||
| Param Python Type: %s, ParamInfo: %s, %s, %s, %s, %s""", | ||
| i + 1, | ||
| param, | ||
| str(type(param)), | ||
| parameters_type[i].paramSQLType, | ||
| parameters_type[i].paramCType, | ||
| parameters_type[i].columnSize, | ||
| parameters_type[i].decimalDigits, | ||
| parameters_type[i].inputOutputType, | ||
| ) | ||
| if logger.isEnabledFor(logging.DEBUG): | ||
| for i, param in enumerate(parameters): | ||
| logger.debug( | ||
| """Parameter number: %s, Parameter: %s, | ||
| Param Python Type: %s, ParamInfo: %s, %s, %s, %s, %s""", | ||
| i + 1, | ||
| param, | ||
| str(type(param)), | ||
| parameters_type[i].paramSQLType, | ||
| parameters_type[i].paramCType, | ||
| parameters_type[i].columnSize, | ||
| parameters_type[i].decimalDigits, | ||
| parameters_type[i].inputOutputType, | ||
| ) | ||
| ret = ddbc_bindings.DDBCSQLExecute( | ||
| self.hstmt, | ||
| operation, | ||
| parameters, | ||
| parameters_type, | ||
| self.is_stmt_prepared, | ||
| use_prepare, | ||
| effective_use_prepare, | ||
| encoding_settings, | ||
| ) | ||
| # Check return code | ||
| @@ -1491,8 +1515,12 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state | ||
| self._reset_cursor() | ||
| raise | ||
| # Capture any diagnostic messages (SQL_SUCCESS_WITH_INFO, etc.) | ||
| if self.hstmt: | ||
| # Capture diagnostic messages only on SQL_SUCCESS_WITH_INFO. | ||
| # SQL_SUCCESS has no records — calling DDBCSQLGetAllDiagRecords on it | ||
| # costs ~10ms/call (driver scans internal state to find nothing). | ||
| # SQL_ERROR is already handled by check_error() above which extracts | ||
| # diagnostics and raises. | ||
| if ret == ddbc_sql_const.SQL_SUCCESS_WITH_INFO.value and self.hstmt: | ||
bewithgaurav marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| self.messages.extend(ddbc_bindings.DDBCSQLGetAllDiagRecords(self.hstmt)) | ||
| self.last_executed_stmt = operation | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1378,6 +1378,32 @@ void SqlHandle::close_cursor() { | ||
| } | ||
| } | ||
| SQLRETURN SQLResetStmt_wrap(SqlHandlePtr statementHandle) { | ||
| if (!statementHandle || !statementHandle->get()) { | ||
| return SQL_INVALID_HANDLE; | ||
| } | ||
| if (statementHandle->isImplicitlyFreed()) { | ||
| return SQL_INVALID_HANDLE; | ||
| } | ||
| if (!SQLFreeStmt_ptr) { | ||
| DriverLoader::getInstance().loadDriver(); | ||
| } | ||
| SQLHANDLE hStmt = statementHandle->get(); | ||
| SQLRETURN rc; | ||
| { | ||
| py::gil_scoped_release release; | ||
| rc = SQLFreeStmt_ptr(hStmt, SQL_CLOSE); | ||
| if (SQL_SUCCEEDED(rc)) { | ||
bewithgaurav marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| rc = SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); | ||
| } | ||
| if (SQL_SUCCEEDED(rc) && SQLSetStmtAttr_ptr) { | ||
| rc = SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_PARAMSET_SIZE, (SQLPOINTER)1, 0); | ||
| } | ||
| } | ||
| return rc; | ||
| } | ||
| SQLRETURN SQLGetTypeInfo_Wrapper(SqlHandlePtr StatementHandle, SQLSMALLINT DataType) { | ||
| if (!SQLGetTypeInfo_ptr) { | ||
| ThrowStdException("SQLGetTypeInfo function not loaded"); | ||
| @@ -5800,6 +5826,7 @@ PYBIND11_MODULE(ddbc_bindings, m) { | ||
| py::arg("wcharEncoding") = "utf-16le"); | ||
| m.def("DDBCSQLFetchArrowBatch", &FetchArrowBatch_wrap, "Fetch an arrow batch of given length from the result set"); | ||
| m.def("DDBCSQLFreeHandle", &SQLFreeHandle_wrap, "Free a handle"); | ||
| m.def("DDBCSQLResetStmt", &SQLResetStmt_wrap, "Close cursor and unbind params without freeing HSTMT"); | ||
| m.def("DDBCSQLCheckError", &SQLCheckError_Wrap, "Check for driver errors"); | ||
| m.def("DDBCSQLGetAllDiagRecords", &SQLGetAllDiagRecords, | ||
| "Get all diagnostic records for a handle", py::arg("handle")); | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.