Skip to content

Conversation

@gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented Aug 28, 2025

Work Item / Issue Reference

AB#38339

GitHub Issue: #<ISSUE_NUMBER>


Summary

This pull request adds support for using the SQLDescribeParam ODBC API to determine the correct SQL type for parameters with unknown types (such as None values), improving parameter binding accuracy—especially for binary columns. The changes include adding the necessary function pointer, loading it from the driver, and updating the parameter binding logic to call SQLDescribeParam when needed.

ODBC API Integration:

  • Added the SQLDescribeParamFunc typedef and declared/defined the SQLDescribeParam_ptr function pointer to enable calling SQLDescribeParam from the driver. [1] [2] [3] [4] [5]

Parameter Binding Improvements:

  • Updated the logic in BindParameters to call SQLDescribeParam for parameters with unknown types, ensuring correct SQL type, column size, and decimal digits are used for None values and improving support for binary columns.

Minor Cleanups:

  • Removed a TODO comment in _map_sql_type as type detection is now handled more robustly.

Copilot AI review requested due to automatic review settings August 28, 2025 11:06
@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Aug 28, 2025
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 adds support for using the ODBC SQLDescribeParam API to properly handle parameter binding for None values, particularly improving type detection for binary columns. Previously, None parameters used a default SQL type which could cause issues with binary columns.

  • Added ODBC SQLDescribeParam function pointer integration to query parameter metadata
  • Updated parameter binding logic to call SQLDescribeParam when parameter type is unknown
  • Removed TODO comment related to binary column handling since the issue is now resolved

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
mssql_python/pybind/ddbc_bindings.h Added SQLDescribeParam function typedef and pointer declaration
mssql_python/pybind/ddbc_bindings.cpp Implemented SQLDescribeParam loading and parameter binding logic using the API
mssql_python/cursor.py Removed TODO comment about adding SQLDescribeParam support

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Aug 28, 2025
@gargsaumya gargsaumya force-pushed the saumya/handle-null-cases branch from c3185ed to 0b7e699 Compare August 29, 2025 03:19
@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Aug 29, 2025
Copy link
Contributor

@sumitmsft sumitmsft left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Aug 29, 2025
@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Aug 29, 2025
@gargsaumya gargsaumya merged commit 9eb92ed into main Sep 1, 2025
18 checks passed
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.

4 participants