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

[gargsaumya]

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]

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]

📊 Code Coverage Report

🔥 Diff Coverage 72%

🎯 Overall Coverage 77%

📈 Total Lines Covered: 5682 out of 7341 📁 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.pybind.ddbc_bindings.h: 67.8%
mssql_python.row.py: 70.5%
mssql_python.pybind.connection.connection.cpp: 75.3%
mssql_python.__init__.py: 77.1%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.2%
mssql_python.cursor.py: 85.6%

---

🔗 Quick Links

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

[subrata-ms]

I think we should not map NULL to a SQL_UNKNOWN_TYPE. In SQL , NULL is not a data type and hence mapping it to SQL_UNKNOWN_TYPE is wrong. Can you please check whether you can use SQL_NULL_DATA for representing the NULL.

[gargsaumya]

Thanks for the review @subrata-ms .
I understand your semantic concern, NULL in SQL represents the absence of a value, not an “unknown type.” However, SQL_NULL_DATA cannot be used here because it serves a completely different purpose within the ODBC API.

In ODBC, SQL_UNKNOWN_TYPE (value 0) is used as the SQL type in SQLBindParameter to indicate that the driver should determine the actual parameter type. In contrast, SQL_NULL_DATA (value -1) is not a type at all, it is an indicator used in the StrLen_or_IndPtr buffer to signal that the parameter value is NULL.

In the current implementation, this distinction is handled correctly:

1. The Python layer returns SQL_UNKNOWN_TYPE as the SQL type, which triggers the C++ layer to resolve the actual type.
2. The C++ layer calls SQLDescribeParam to discover the target column type (e.g., VARBINARY, INT, NVARCHAR).
3. The C++ layer sets *strLenOrIndPtr = SQL_NULL_DATA, which correctly communicates that the value is NULL.

So, SQL_NULL_DATA is already being used exactly where it should be, as an indicator of a NULL value. Meanwhile, SQL_UNKNOWN_TYPE is intentionally used to trigger type discovery.
If we attempted to pass SQL_NULL_DATA (-1) as the SQL type, it would not behave as intended. Instead, it would be interpreted as SQL_LONGVARCHAR (which also has the value -1), leading to incorrect type binding.

[sumitmsft]

For INSERT into a non-VARCHAR column where SQLDescribeParam fails for an unexpected reason (driver bug, network issue, etc.), silently falling back to SQL_VARCHAR could mask the real error and cause a different, harder-to-debug type mismatch later. Can we consider logging at a WARN level (not just LOG) when falling back because silent fallback could be surprising

[sumitmsft]

Can you add a test similar to this:

def test_execute_none_into_varbinary_column(db_connection):
    """Regression test for #458: None into VARBINARY should not raise varchar->varbinary error."""
    cursor = db_connection.cursor()
    drop_table_if_exists(cursor, "#test_varbinary_null")

    try:
        cursor.execute("CREATE TABLE #test_varbinary_null (data VARBINARY(100))")
        cursor.execute("INSERT INTO #test_varbinary_null (data) VALUES (?)", None)
        cursor.execute("SELECT data FROM #test_varbinary_null")
        row = cursor.fetchone()
        assert row[0] is None
    finally:
        drop_table_if_exists(cursor, "#test_varbinary_null")
        cursor.close()