Skip to content

Conversation

@gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented Nov 27, 2025

Work Item / Issue Reference

AB#37803

GitHub Issue: #<ISSUE_NUMBER>


Summary

This pull request introduces improvements to the build configuration and code safety for the mssql_python/pybind module. The main changes focus on enforcing stricter warning and error handling in the build system, improving cross-platform compatibility, and ensuring safe type casting in parameter binding.

Build system improvements:

  • Enforced treating CMake warnings and deprecated features as errors by setting CMAKE_ERROR_DEPRECATED and CMAKE_WARN_DEPRECATED to TRUE in CMakeLists.txt, ensuring deprecated usage is caught early.
  • Added compiler warning flags for GCC and Clang (-Werror, -Wattributes, -Wint-to-pointer-cast) to treat warnings as errors and catch visibility and type casting issues in CMakeLists.txt.

Code safety and compatibility:

  • Suppressed visibility attribute warnings for the ParamInfo struct on Linux using GCC diagnostic pragmas, while maintaining compatibility with Windows. [1] [2]
  • Updated type casting in the BindParameters function to use reinterpret_cast and static_cast for safe conversion of numeric precision and scale values to SQLPOINTER, preventing potential type safety issues.

Copilot AI review requested due to automatic review settings November 27, 2025 12:41
@github-actions github-actions bot added the pr-size: small Minimal code update label Nov 27, 2025
@github-actions
Copy link

github-actions bot commented Nov 27, 2025

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

75%


📈 Total Lines Covered: 5089 out of 6748
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/pybind/ddbc_bindings.cpp (100%)

Summary

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

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.cpp: 67.1%
mssql_python.helpers.py: 67.5%
mssql_python.pybind.connection.connection.cpp: 74.7%
mssql_python.pybind.ddbc_bindings.h: 76.9%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 82.5%
mssql_python.cursor.py: 83.6%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

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 pull request enhances build safety and cross-platform compatibility for the mssql_python pybind module by enabling stricter compiler warnings and fixing related issues in Linux builds. The PR adds support for treating warnings as errors on Linux/macOS and addresses casting issues that arose from enabling these strict checks.

  • Enabled strict warning enforcement for GCC/Clang compilers with -Werror flag
  • Fixed integer-to-pointer casting issues in numeric parameter binding for ODBC calls
  • Added targeted warning suppression for pybind11 struct visibility attributes on GCC

Reviewed changes

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

File Description
mssql_python/pybind/CMakeLists.txt Added CMake deprecation warnings as errors and compiler flags for GCC/Clang to treat warnings as errors, though contains issues with invalid CMAKE variable and Clang-incompatible flag
mssql_python/pybind/ddbc_bindings.cpp Added GCC pragma directives to suppress -Wattributes warnings for ParamInfo struct and updated type casting for numeric precision/scale values to use reinterpret_cast with intermediate integer types, though the scale casting has a signedness issue

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

@gargsaumya gargsaumya merged commit a896a93 into main Nov 28, 2025
27 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