FIX: SQLDescribeParam ordinal remapping for VARBINARY/BINARY NULL

[Jahnvi Thakkar (jahnvi480)]

Work Item / Issue Reference

AB#45521

GitHub Issue: #627

---

Summary

This pull request significantly improves how the driver resolves SQL types for NULL parameters, especially when inserting into temp tables or table variables. It proactively resolves unknown NULL parameter types before binding, emits actionable Python warnings when automatic type resolution fails, and provides clear guidance on using setinputsizes to avoid SQL Server implicit conversion errors. Additionally, the code is refactored for clarity and efficiency, and minor formatting improvements are made.

Improvements to NULL parameter type resolution:

• Added a new PreResolveUnknownNullTypes function to proactively resolve unknown NULL SQL types for both single and batch execution paths, ensuring all types are determined before any parameters are bound. This prevents driver errors with binary NULLs and improves reliability.
• When SQL type resolution falls back to SQL_VARCHAR (because SQLDescribeParam fails), the code now emits a Python warning with detailed guidance on how to use cursor.setinputsizes() to specify the correct type, preventing common SQL Server errors with BINARY/VARBINARY columns.

Documentation and user guidance:

• Expanded the setinputsizes docstring in cursor.py to document the temp table/NULL issue and provide a concrete usage example for binary columns.

Code structure and clarity:

• Refactored parameter binding code to avoid redundant type resolution during binding, since types are now resolved in advance.
• Added a bool isFallback field to DescribedParamInfo to track when a fallback type is used.

Minor improvements:

• Made minor formatting and whitespace changes for improved readability.

[Copilot]

Pull request overview

This PR addresses GH-627 by preventing SQLDescribeParam ordinal remapping failures when NULL parameters (notably BINARY/VARBINARY) are described after earlier parameters have already been bound. It does this by pre-resolving unknown NULL parameter SQL types before any binding occurs, adds an end-user warning when SQLDescribeParam fails and the driver must fall back to SQL_VARCHAR, and adds regression coverage and documentation.

Changes:

• Add a pre-scan (PreResolveUnknownNullTypes) that resolves unknown NULL parameter SQL types ahead of binding for both execute() and executemany() paths.
• Emit a Python UserWarning when SQLDescribeParam fails (fallback to SQL_VARCHAR) with guidance to use setinputsizes.
• Add GH-627 regression tests and expand setinputsizes documentation with guidance for temp table/table variable NULL binary cases.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
mssql_python/pybind/ddbc_bindings.cpp	Pre-resolves unknown NULL types before binding; adds SQLDescribeParam fallback warning and refactors binding to avoid interleaved describe+bind.
mssql_python/pybind/ddbc_bindings.h	Extends DescribedParamInfo with isFallback metadata.
mssql_python/cursor.py	Updates setinputsizes docstring with GH-627 guidance and example usage.
tests/test_004_cursor.py	Adds regression tests covering execute/executemany ordering, reuse path, and temp-table warning/workaround behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 87%

🎯 Overall Coverage 80%

📈 Total Lines Covered:6738 out of 8326 📁 Project:mssql-python

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

• mssql_python/pybind/ddbc_bindings.cpp (87.9%): Missing lines 542,552-553,731

Summary

• Total: 33 lines
• Missing: 4 lines
• Coverage: 87%

mssql_python/pybind/ddbc_bindings.cpp

Lines 538-546

538 }
539540// GH-627: Resolve unknown NULL SQL types before any SQLBindParameter calls.541// Some drivers remap parameter ordinals during describe when parameters have
! 542// already been bound, so interleaving describe+bind can fail for binary NULLs.543// When `params` is provided (execute path), an additional py::none check is544// performed; for executemany (array path), SQL_C_DEFAULT already guarantees545// all values in that column are NULL, so no Python-level check is needed.546staticvoidPreResolveUnknownNullTypes(SqlHandle& handle, SQLHANDLE hStmt,

---

Lines 548-557

548const py::list* params = nullptr) {
549if (paramInfos.empty())
550return;
551 ! 552for (size_t paramIndex = 0; paramIndex < paramInfos.size(); ++paramIndex) {
! 553 ParamInfo& paramInfo = paramInfos[paramIndex];
554if (paramInfo.paramCType != SQL_C_DEFAULT || paramInfo.paramSQLType != SQL_UNKNOWN_TYPE) {
555continue;
556 }
557// For execute(), verify the actual value is None (mixed columns possible).

---

Lines 727-735

727caseSQL_C_DEFAULT: {
728if (!py::isinstance<py::none>(param)) {
729ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex));
730 }
! 731 dataPtr = nullptr; // GH-627: type resolved by PreResolveUnknownNullTypes.732 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);
733 *strLenOrIndPtr = SQL_NULL_DATA;
734 bufferLength = 0;
735break;

---

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.pybind.ddbc_bindings.cpp: 76.2%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%

---

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

[Gaurav Sharma (bewithgaurav)]

core fix is right, and solves the issue
comments mostly are all on the warning introduced and impl around it which can cause future problems, requesting changes

[Gaurav Sharma (bewithgaurav)]

lgtm