Skip to content

Conversation

@gargsaumya
Copy link
Contributor

This reverts commit 10a8815.

Work Item / Issue Reference

AB#<WORK_ITEM_ID>

GitHub Issue: #<ISSUE_NUMBER>


Summary

This pull request refactors the global settings management and row handling logic for the mssql_python package. The main improvements include simplifying how global settings like lowercase and decimal separator are managed, removing the custom module class, and streamlining the Row object to reduce memory usage and complexity. The changes also clarify the output converter logic and remove redundant code related to settings snapshots and UUID handling.

Settings and Global Configuration Improvements:

  • Replaced the custom module class and property-based settings management with a simpler global Settings object, protected by a threading lock, and a custom module-level __setattr__ for direct attribute updates [1] [2] [3].
  • Added new functions setDecimalSeparator and getDecimalSeparator for explicit decimal separator control, with input validation and C++ backend integration.

Row Object and Data Handling Simplification:

  • Refactored the Row class to remove the settings snapshot and native UUID conversion logic, reducing per-row memory usage and complexity; column mapping is now optimized for future improvements [1] [2].
  • Updated cursor fetch methods (fetchone, fetchmany, fetchall) to no longer pass or rely on settings snapshots, simplifying their signatures and usage [1] [2] [3].

Output Converter Logic Cleanup:

  • Streamlined output converter logic in the Row class, removing unnecessary integer byte size mapping and improving error handling and logging for converter exceptions [1] [2].

Cursor Execution and Description Handling:

  • Cleaned up cursor execution logic by removing redundant settings snapshot and UUID index management, and ensuring description is initialized after execution for all result types [1] [2].

(References: [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

Copilot AI review requested due to automatic review settings November 3, 2025 15:40
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 removes the native_uuid global setting feature from the mssql_python package, simplifying UUID handling to always return UUID objects from the database. The PR also cleans up related code in the Row class and removes tests that validated the now-removed feature.

Key changes:

  • Removed native_uuid setting and all related validation/toggle logic
  • Simplified Row class by removing UUID conversion and settings snapshot logic
  • Removed tests for native_uuid setting behavior and validation
  • Cleaned up duplicate code in cursor.py's execute method

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
mssql_python/init.py Removed native_uuid property, validation, and module-level exports; simplified Settings class to only include lowercase and decimal_separator
mssql_python/row.py Removed UUID processing logic, settings snapshots, and related imports; simplified __init__ and removed _process_uuid_values method
mssql_python/cursor.py Removed UUID indices tracking and settings snapshots from execute method; simplified Row object creation
tests/test_001_globals.py Removed native_uuid import and all tests validating native_uuid behavior and type validation
tests/test_004_cursor.py Removed native_uuid toggle logic from UUID-related tests and removed comprehensive tests for native_uuid setting behavior

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

@gargsaumya gargsaumya changed the title Revert "FEAT: Support for Native_UUID Attribute (#282)" FIX: Revert "FEAT: Support for Native_UUID Attribute (#282)" Nov 3, 2025
@github-actions github-actions bot added the pr-size: large Substantial code update label Nov 3, 2025
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

📊 Code Coverage Report

🔥 Diff Coverage

79%


🎯 Overall Coverage

77%


📈 Total Lines Covered: 4576 out of 5899
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/init.py (100%)
  • mssql_python/cursor.py (71.4%): Missing lines 1147,1149
  • mssql_python/helpers.py (100%)
  • mssql_python/row.py (80.0%): Missing lines 105,133-135

Summary

  • Total: 29 lines
  • Missing: 6 lines
  • Coverage: 79%

mssql_python/cursor.py

Lines 1143-1153

  1143         column_metadata = []
  1144         try:
  1145             ddbc_bindings.DDBCSQLDescribeCol(self.hstmt, column_metadata)
  1146             self._initialize_description(column_metadata)
! 1147         except Exception as e:
  1148             # If describe fails, it's likely there are no results (e.g., for INSERT)
! 1149             self.description = None
  1150 
  1151         self._reset_inputsizes()  # Reset input sizes after execution
  1152         # Return self for method chaining
  1153         return self

mssql_python/row.py

Lines 101-109

  101                         converted_values[i] = converter(value)
  102                 except Exception:
  103                     # Log the exception for debugging without leaking sensitive data
  104                     if hasattr(self._cursor, 'log'):
! 105                         self._cursor.log('debug', 'Exception occurred in output converter', exc_info=True)
  106                     # If conversion fails, keep the original value
  107                     pass
  108         
  109         return converted_values

Lines 129-139

  129         
  130         # If lowercase is enabled on the cursor, try case-insensitive lookup
  131         if hasattr(self._cursor, 'lowercase') and self._cursor.lowercase:
  132             name_lower = name.lower()
! 133             for col_name in self._column_map:
! 134                 if col_name.lower() == name_lower:
! 135                     return self._values[self._column_map[col_name]]
  136         
  137         raise AttributeError(f"Row has no attribute '{name}'")
  138 
  139     def __eq__(self, other: Any) -> bool:


📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.ddbc_bindings.cpp: 70.7%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection.cpp: 81.2%
mssql_python.connection.py: 82.9%
mssql_python.cursor.py: 83.4%
mssql_python.pybind.connection.connection_pool.cpp: 84.8%
mssql_python.auth.py: 87.1%
mssql_python.pooling.py: 87.7%
mssql_python.helpers.py: 88.9%
mssql_python.row.py: 90.9%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

@gargsaumya gargsaumya merged commit 00fbc3c into main Nov 4, 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: large Substantial code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants