Skip to content

Conversation

@gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented Oct 31, 2025

Work Item / Issue Reference

AB#40022

GitHub Issue: #<ISSUE_NUMBER>


Summary

This pull request improves the handling of decimal values, especially those in scientific notation, when converting to SQL VARCHAR types. The changes ensure that decimals are consistently formatted as fixed-point strings rather than potentially problematic scientific notation, preventing SQL Server conversion errors. Additionally, new tests have been added to verify correct behavior for a variety of decimal values.

Decimal formatting and conversion improvements:

  • Updated _map_sql_type in mssql_python/cursor.py to use format(param, 'f') instead of str(param) for MONEY and SMALLMONEY types, ensuring decimals are always converted to fixed-point strings. [1] [2]
  • Modified executemany in mssql_python/cursor.py to format decimal values as fixed-point strings before insertion when mapped to SQL_VARCHAR.

Test suite enhancements:

  • Added test_decimal_scientific_notation_to_varchar in tests/test_004_cursor.py to verify that decimals (including those with scientific notation) are correctly converted and stored as VARCHAR without causing conversion errors.

Test cleanup and simplification:

  • Removed exception handling logic in test_numeric_leading_zeros_precision_loss and test_numeric_extreme_exponents_precision_loss that previously skipped tests on conversion errors, as these errors should no longer occur with the improved formatting. [1] [2]

Copilot AI review requested due to automatic review settings October 31, 2025 10:41
@github-actions github-actions bot added the pr-size: medium Moderate update size label Oct 31, 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 fixes Decimal to VARCHAR conversion issues by preventing scientific notation from being passed to SQL Server. The driver now converts Decimal values to fixed-point notation using format(param, 'f') instead of str(param), which previously preserved scientific notation and caused SQL Server conversion errors.

Key changes:

  • Modified Decimal to VARCHAR conversion to use fixed-point notation format
  • Removed error-handling workarounds from tests that are no longer needed
  • Added comprehensive test coverage for Decimal scientific notation scenarios

Reviewed Changes

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

File Description
mssql_python/cursor.py Changed Decimal to VARCHAR conversion from str() to format(param, 'f') for MONEY/SMALLMONEY and other VARCHAR-bound decimals to prevent scientific notation
tests/test_004_cursor.py Removed obsolete exception handlers, added new test for Decimal scientific notation to VARCHAR conversion, minor whitespace fix

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

@github-actions
Copy link

github-actions bot commented Oct 31, 2025

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

77%


📈 Total Lines Covered: 4616 out of 5986
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/cursor.py (100%)

Summary

  • Total: 3 lines
  • Missing: 0 lines
  • Coverage: 100%

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.ddbc_bindings.cpp: 70.9%
mssql_python.pybind.connection.connection_pool.cpp: 78.9%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection.cpp: 81.2%
mssql_python.connection.py: 82.9%
mssql_python.auth.py: 87.1%
mssql_python.pooling.py: 87.7%
mssql_python.helpers.py: 88.9%
mssql_python.__init__.py: 90.7%
mssql_python.exceptions.py: 92.1%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

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.

Left few comments

@gargsaumya
Copy link
Contributor Author

As discussed offline, I’ve created a work item (40169) for correct handling of Decimal types (Numeric, Money, and SmallMoney) based on proper precision and scale.

@gargsaumya gargsaumya merged commit 622f95b into main Nov 6, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants