Skip to content

Conversation

@jahnvi480
Copy link
Contributor

@jahnvi480 jahnvi480 commented Oct 7, 2025

Work Item / Issue Reference

AB#39059


Summary

This pull request introduces a new global setting, native_uuid, to the mssql_python package, allowing users to control whether UUID values are returned as uuid.UUID objects or as strings. The implementation includes updates to the package initialization, row processing logic, and a comprehensive set of tests to verify the new behavior and ensure backward compatibility.

UUID Handling Improvements:

  • Added a native_uuid setting to the global configuration in mssql_python/__init__.py, defaulting to False for backward compatibility. This setting controls whether UUIDs are returned as uuid.UUID objects or as strings.
  • Updated the Row class in mssql_python/row.py to check the native_uuid setting and convert UUID values to strings when native_uuid is False, ensuring consistent output based on configuration.

Testing Enhancements:

  • Updated and extended tests in tests/test_004_cursor.py to verify correct UUID handling for both native_uuid=True and native_uuid=False, including new tests for the setting and resetting of the native_uuid option.

Internal Refactoring:

  • Removed unused UUID mapping logic in mssql_python/cursor.py that is now handled via the new setting and row processing logic.
  • Minor import and code organization cleanups in affected modules for clarity and maintainability.

These changes provide greater flexibility and control over how UUIDs are handled in query results, improving the usability of the package in different application contexts.

Copilot AI review requested due to automatic review settings October 7, 2025 06:25
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 introduces a new native_uuid global setting to control whether UUID values are returned as uuid.UUID objects or strings, with a default value of False for backward compatibility.

Key changes:

  • Added native_uuid setting to global configuration with backward-compatible default
  • Updated row processing logic to convert UUID objects to strings when native_uuid=False
  • Enhanced test coverage to verify UUID handling behavior for both setting states

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
mssql_python/init.py Added native_uuid setting initialization and module-level access
mssql_python/row.py Implemented UUID conversion logic based on native_uuid setting
mssql_python/cursor.py Removed unused UUID mapping code
tests/test_004_cursor.py Added comprehensive tests for native_uuid setting and updated existing UUID tests

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

@github-actions github-actions bot added the pr-size: medium Moderate update size label Oct 7, 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.

Left a few comments

@github-actions
Copy link

github-actions bot commented Oct 15, 2025

📊 Code Coverage Report

🔥 Diff Coverage

75%


🎯 Overall Coverage

75%


📈 Total Lines Covered: 4326 out of 5726
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/init.py (93.9%): Missing lines 279-280
  • mssql_python/cursor.py (92.9%): Missing lines 1061
  • mssql_python/row.py (62.3%): Missing lines 28-29,44,82,84-87,90-92,94,96-102,169,171-172,181,196,206-207

Summary

  • Total: 116 lines
  • Missing: 29 lines
  • Coverage: 75%

mssql_python/init.py

Lines 275-284

  275 for attr_name in dir(old_module):
  276     if attr_name != "__class__":
  277         try:
  278             setattr(new_module, attr_name, getattr(old_module, attr_name))
! 279         except AttributeError:
! 280             pass
  281 
  282 # Replace the module in sys.modules
  283 sys.modules[__name__] = new_module

mssql_python/cursor.py

Lines 1057-1065

  1057                 if desc and desc[1] == uuid.UUID:  # Column type code at index 1
  1058                     self._uuid_indices.append(i)
  1059                 # Verify we have complete description tuples (7 items per PEP-249)
  1060                 elif desc and len(desc) != 7:
! 1061                     log('warning', f"Column description at index {i} has incorrect tuple length: {len(desc)}")
  1062             self.rowcount = -1
  1063             self._reset_rownumber()
  1064         else:
  1065             self.rowcount = ddbc_bindings.DDBCSQLRowCount(self.hstmt)

mssql_python/row.py

Lines 24-33

  24         # Use settings snapshot if provided, otherwise fallback to global settings
  25         if settings_snapshot is not None:
  26             self._settings = settings_snapshot
  27         else:
! 28             settings = get_settings()
! 29             self._settings = {
  30                 'lowercase': settings.lowercase,
  31                 'native_uuid': settings.native_uuid
  32             }
  33         # Create mapping of column names to indices first

Lines 40-48

  40                     if self._settings.get('lowercase'):
  41                         col_name = col_name.lower()
  42                     self._column_map[col_name] = i
  43         else:
! 44             self._column_map = column_map
  45         
  46         # First make a mutable copy of values
  47         processed_values = list(values)
  48         

Lines 78-106

   78                 for i in uuid_indices:
   79                     if i < len(processed_values) and processed_values[i] is not None:
   80                         value = processed_values[i]
   81                         if isinstance(value, str):
!  82                             try:
   83                                 # Remove braces if present
!  84                                 clean_value = value.strip('{}')
!  85                                 processed_values[i] = uuid.UUID(clean_value)
!  86                             except (ValueError, AttributeError):
!  87                                 pass  # Keep original if conversion fails
   88             # Fallback to scanning all columns if indices weren't pre-identified
   89             else:
!  90                 for i, value in enumerate(processed_values):
!  91                     if value is None:
!  92                         continue
   93                     
!  94                     if i < len(description) and description[i]:
   95                         # Check SQL type for UNIQUEIDENTIFIER (-11)
!  96                         sql_type = description[i][1]
!  97                         if sql_type == -11:  # SQL_GUID
!  98                             if isinstance(value, str):
!  99                                 try:
! 100                                     processed_values[i] = uuid.UUID(value.strip('{}'))
! 101                                 except (ValueError, AttributeError):
! 102                                     pass
  103         # When native_uuid is False, convert UUID objects to strings
  104         else:
  105             for i, value in enumerate(processed_values):
  106                 if isinstance(value, uuid.UUID):

Lines 165-176

  165                         try:
  166                             # Use signed=True to properly handle negative values
  167                             value_bytes = value.to_bytes(byte_size, byteorder='little', signed=True)
  168                             converted_values[i] = converter(value_bytes)
! 169                         except OverflowError as e:
  170                             # Log specific overflow error with details to help diagnose the issue
! 171                             if hasattr(self._cursor, 'log'):
! 172                                 self._cursor.log('warning', 
  173                                     f'Integer overflow: value {value} does not fit in {byte_size} bytes for SQL type {sql_type}')
  174                             # Keep the original value in this case
  175                     else:
  176                         # Pass the value directly for other types

Lines 177-185

  177                         converted_values[i] = converter(value)
  178                 except Exception as e:
  179                     # Log the exception for debugging without leaking sensitive data
  180                     if hasattr(self._cursor, 'log'):
! 181                         self._cursor.log('warning', f'Exception in output converter: {type(e).__name__} for SQL type {sql_type}')
  182                     # If conversion fails, keep the original value
  183         
  184         return converted_values

Lines 192-200

  192         Allow accessing by column name as attribute: row.column_name
  193         """
  194         # _column_map should already be set in __init__, but check to be safe
  195         if not hasattr(self, '_column_map'):
! 196             self._column_map = {}
  197             
  198         # Try direct lookup first
  199         if name in self._column_map:
  200             return self._values[self._column_map[name]]

Lines 202-211

  202         # Use the snapshot lowercase setting instead of global
  203         if self._settings.get('lowercase'):
  204             # If lowercase is enabled, try case-insensitive lookup
  205             name_lower = name.lower()
! 206             if name_lower in self._column_map:
! 207                 return self._values[self._column_map[name_lower]]
  208         
  209         raise AttributeError(f"Row has no attribute '{name}'")
  210     
  211     def __eq__(self, other):


📋 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: 70.5%
mssql_python.row.py: 76.3%
mssql_python.connection.py: 81.7%
mssql_python.cursor.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: 87.5%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

sumitmsft
sumitmsft previously approved these changes Oct 16, 2025
@jahnvi480 jahnvi480 merged commit 10a8815 into main Oct 17, 2025
21 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.

5 participants