FIX: SQLDescribeParam ordinal remapping for VARBINARY/BINARY NULL

[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 90%

🎯 Overall Coverage 80%

📈 Total Lines Covered: 6765 out of 8358 📁 Project: mssql-python

---

Diff Coverage

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

• mssql_python/pybind/ddbc_bindings.cpp (90.2%): Missing lines 546-547,563,585-586,764

Summary

• Total: 61 lines
• Missing: 6 lines
• Coverage: 90%

mssql_python/pybind/ddbc_bindings.cpp

Lines 542-551

  542                 .attr("warn")(
  543                     "SQLDescribeParam failed for parameter index " + std::to_string(paramIndex) +
  544                         " (0-based). Falling back to SQL_VARCHAR for NULL binding. "
  545                         "This may cause 'Implicit conversion from data type varchar to varbinary "
! 546                         "is not allowed' errors for BINARY/VARBINARY columns. "
! 547                         "To fix: from mssql_python.constants import ConstantsDDBC; then call "
  548                         "cursor.setinputsizes() with explicit type info for every parameter. "
  549                         "Example (2 params): cursor.setinputsizes("
  550                         "[(ConstantsDDBC.SQL_INTEGER.value, 10, 0), "
  551                         "(ConstantsDDBC.SQL_VARBINARY.value, column_size, 0)])",

---

Lines 559-567

  559 }
  560 
  561 // GH-627: Resolve unknown NULL SQL types before any SQLBindParameter calls.
  562 // Some drivers remap parameter ordinals during describe when parameters have
! 563 // already been bound, so interleaving describe+bind can fail for binary NULLs.
  564 // When `params` is provided (execute path), an additional py::none check is
  565 // performed; for executemany (array path), SQL_C_DEFAULT already guarantees
  566 // all values in that column are NULL, so no Python-level check is needed.
  567 static void PreResolveUnknownNullTypes(SqlHandle& handle, SQLHANDLE hStmt,

---

Lines 581-590

  581     }
  582     if (!hasUnknownNull)
  583         return;
  584 
! 585     for (size_t paramIndex = 0; paramIndex < paramInfos.size(); ++paramIndex) {
! 586         ParamInfo& paramInfo = paramInfos[paramIndex];
  587         if (paramInfo.paramCType != SQL_C_DEFAULT || paramInfo.paramSQLType != SQL_UNKNOWN_TYPE) {
  588             continue;
  589         }
  590         // For execute(), verify the actual value is None (mixed columns possible).

---

Lines 760-768

  760             case SQL_C_DEFAULT: {
  761                 if (!py::isinstance<py::none>(param)) {
  762                     ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex));
  763                 }
! 764                 dataPtr = nullptr;  // GH-627: type resolved by PreResolveUnknownNullTypes.
  765                 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);
  766                 *strLenOrIndPtr = SQL_NULL_DATA;
  767                 bufferLength = 0;
  768                 break;

---

---

📋 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.3%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
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