Skip to content

Conversation

gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented Sep 24, 2025

Work Item / Issue Reference

AB#<WORK_ITEM_ID>

GitHub Issue: #253


Summary

This pull request removes the _select_best_sample_value static method and updates how sample values are selected for type inference in the executemany method. Instead of using the old method, the code now relies on _compute_column_type to determine sample values and type information.

Refactoring and Type Inference Updates:

  • Removed the _select_best_sample_value static method from cursor.py, consolidating the logic for type inference.
  • Updated the executemany method to use _compute_column_type for determining sample values, minimum, and maximum values for each column, improving clarity and maintainability.

@Copilot Copilot AI review requested due to automatic review settings September 24, 2025 05:58
@gargsaumya gargsaumya changed the title BUG: Type inference bug FIX: Type inference bug Sep 24, 2025
Copy link
Contributor

@Copilot 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 addresses a type inference bug by removing the _select_best_sample_value static method and consolidating type inference logic into the existing _compute_column_type method.

  • Removes redundant _select_best_sample_value method that was duplicating type inference logic
  • Updates executemany method to use _compute_column_type for consistent sample value selection
  • Simplifies the codebase by eliminating duplicate logic for type inference

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 Sep 24, 2025
@github-actions github-actions bot added pr-size: small Minimal code update and removed pr-size: small Minimal code update labels Sep 24, 2025
Copy link
Contributor

@Copilot 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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


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

@bewithgaurav
Copy link
Collaborator

merging main here to test and check code coverage

Copy link

github-actions bot commented Sep 26, 2025

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

74%


📈 Total Lines Covered: 4144 out of 5560
📁 Project: mssql-python


Diff Coverage

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

No lines with coverage information in this diff.


📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.connection.connection.cpp: 68.3%
mssql_python.ddbc_bindings.py: 68.5%
mssql_python.pybind.ddbc_bindings.cpp: 69.4%
mssql_python.pybind.connection.connection_pool.cpp: 78.9%
mssql_python.cursor.py: 79.3%
mssql_python.connection.py: 81.7%
mssql_python.helpers.py: 84.7%
mssql_python.auth.py: 85.3%
mssql_python.type.py: 86.8%
mssql_python.pooling.py: 88.8%

🔗 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 a few test cases to add

@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: small Minimal code update labels Sep 29, 2025
@gargsaumya gargsaumya changed the title FIX: Type inference bug FIX: Fixed and Added tests for Type Inference Bug Sep 30, 2025
@gargsaumya gargsaumya merged commit d844cfe into main Sep 30, 2025
22 of 23 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.

3 participants