Skip to content

FIX: NULL parameter type mapping for VARBINARY columns (#458)#466

Open
gargsaumya wants to merge 1 commit intomainfrom
saumya/gh-458
Open

FIX: NULL parameter type mapping for VARBINARY columns (#458)#466
gargsaumya wants to merge 1 commit intomainfrom
saumya/gh-458

Conversation

@gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented Mar 3, 2026

Work Item / Issue Reference

AB#42894

GitHub Issue: #458


Summary

This pull request makes a targeted fix to the parameter type mapping logic for NULL values in the mssql_python/cursor.py file. The change improves compatibility and prevents implicit conversion errors when executing queries with NULL parameters.

Parameter mapping improvement:

  • Changed the mapping for NULL parameters in the _map_sql_type function to use SQL_UNKNOWN_TYPE instead of SQL_VARCHAR, allowing the driver to correctly determine the column type and avoid conversion errors.

Copilot AI review requested due to automatic review settings March 3, 2026 10:40
@gargsaumya gargsaumya changed the title Fix NULL parameter type mapping for VARBINARY columns (#458) FIX: NULL parameter type mapping for VARBINARY columns (#458) Mar 3, 2026
@github-actions github-actions bot added the pr-size: small Minimal code update label Mar 3, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug (issue #458) where inserting a Python None into a VARBINARY column via a bound parameter would raise an implicit conversion error from varchar to varbinary. The root cause was that the _map_sql_type function mapped None parameters to SQL_VARCHAR, causing the ODBC driver to assume a varchar type. The fix changes this to SQL_UNKNOWN_TYPE (value 0), which triggers the C++ layer (in the SQL_C_DEFAULT handler) to call SQLDescribeParam to determine the actual target column type, allowing correct NULL binding for any column type.

Changes:

  • Updated _map_sql_type in cursor.py to use SQL_UNKNOWN_TYPE instead of SQL_VARCHAR when binding a None (NULL) parameter, enabling the driver to infer the correct target column type via SQLDescribeParam.
Comments suppressed due to low confidence (1)

mssql_python/cursor.py:400

  • No test covers the exact bug scenario from issue #458: calling execute('insert into bytestest values (?)', None) with None as a single bound parameter to a VARBINARY column. The existing test_varbinarymax_insert_fetch_null uses CAST(NULL AS VARBINARY(MAX)) as a SQL literal rather than a Python None parameter binding, and test_only_null_and_empty_binary uses executemany where the column type is inferred from non-null bytes values in the same column. A test that specifically exercises execute with a standalone None parameter against a VARBINARY column (the regression case from the bug report) should be added to prevent future regressions.
        if param is None:
            logger.debug("_map_sql_type: NULL parameter - index=%d", i)
            return (
                ddbc_sql_const.SQL_UNKNOWN_TYPE.value,
                ddbc_sql_const.SQL_C_DEFAULT.value,
                1,
                0,
                False,
            )

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

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

📊 Code Coverage Report

🔥 Diff Coverage

72%


🎯 Overall Coverage

77%


📈 Total Lines Covered: 5597 out of 7264
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/cursor.py (100%)
  • mssql_python/pybind/ddbc_bindings.cpp (62.5%): Missing lines 483-485

Summary

  • Total: 11 lines
  • Missing: 3 lines
  • Coverage: 72%

mssql_python/pybind/ddbc_bindings.cpp

Lines 479-489

  479                         sqlType = SQL_VARCHAR;
  480                         columnSize = 1;
  481                         decimalDigits = 0;
  482                     } else {
! 483                         sqlType = describedType;
! 484                         columnSize = describedSize;
! 485                         decimalDigits = describedDigits;
  486                     }
  487                 }
  488                 dataPtr = nullptr;
  489                 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);


📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.h: 67.8%
mssql_python.pybind.connection.connection.cpp: 75.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.__init__.py: 84.9%
mssql_python.cursor.py: 84.9%
mssql_python.connection.py: 85.1%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: small Minimal code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants