FIX: Cursor.describe invalid data #355

dlevy-msft-sql · 2025-12-01T17:19:35Z

Work Item / Issue Reference

GitHub Issue: #352

Summary

Add handling for spatial data types and change cursor.describe to return sql type int per dbapi2 spec

…ins an LOB

Copilot

Pull request overview

This PR fixes issue #352 by implementing two main improvements: (1) correcting cursor.description to return SQL type codes (integers) instead of Python type objects, ensuring DB-API 2.0 specification compliance, and (2) adding support for SQL Server spatial data types (geography, geometry, hierarchyid) by handling the SQL_SS_UDT type code (-151).

Key changes:

Fixed cursor.description[i][1] to return SQL type integer codes (e.g., 4, -9) instead of Python types (int, str) per DB-API 2.0 spec
Added SQL_SS_UDT (-151) support for SQL Server User-Defined Types including spatial data types
Updated output converter lookup to use SQL type codes consistently throughout the codebase

Reviewed changes

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

Show a summary per file

File	Description
mssql_python/cursor.py	Changed `cursor.description` to return SQL type codes instead of Python types; added `_column_metadata` storage; updated `_build_converter_map` to use SQL type codes; added mappings for UDT, XML, DATETIME2, SMALLDATETIME types
mssql_python/constants.py	Added `SQL_SS_UDT = -151` constant for SQL Server User-Defined Types
mssql_python/pybind/ddbc_bindings.cpp	Added C++ constants for SQL_SS_UDT, SQL_DATETIME2, SQL_SMALLDATETIME; implemented UDT handling in SQLGetData_wrap, FetchBatchData, FetchMany_wrap, and FetchAll_wrap for LOB streaming
tests/test_004_cursor.py	Added `lob_wvarchar_column` to test schema; updated `test_cursor_description` to expect SQL type codes; added comprehensive geography type test suite (14 new tests); separated LOB and non-LOB fetch tests; fixed output converter test for UTF-16LE encoding
tests/test_003_connection.py	Simplified converter integration tests to use SQL type constants directly instead of dynamic type detection

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

tests/test_004_cursor.py

mssql_python/constants.py

mssql_python/cursor.py

…ins an LOB

…ython into Issue-352

github-actions · 2026-01-05T19:27:13Z

📊 Code Coverage Report

🔥 Diff Coverage 98%	🎯 Overall Coverage 76%	📈 Total Lines Covered: `5483` out of `7135` 📁 Project: `mssql-python`

Diff Coverage

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

mssql_python/init.py (100%)
mssql_python/constants.py (100%)
mssql_python/cursor.py (97.4%): Missing lines 129
mssql_python/pybind/ddbc_bindings.cpp (100%)

Summary

Total: 61 lines
Missing: 1 line
Coverage: 98%

mssql_python/cursor.py

Lines 125-133

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.cpp: 69.6%
mssql_python.pybind.ddbc_bindings.h: 69.7%
mssql_python.pybind.connection.connection.cpp: 73.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 84.1%
mssql_python.__init__.py: 84.9%

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

mssql_python/cursor.py

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 16 comments.

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

tests/test_004_cursor.py

- Add geography/geometry/hierarchyid invalid parsing tests - Add concurrent cursor thread safety test for _column_metadata - Add sequential query isolation test for column metadata

Copilot

Pull request overview

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

Comments suppressed due to low confidence (159)

tests/test_004_cursor.py:1244

This comment appears to contain commented-out code.

#     assert row[8] == TEST_DATA[8], "Nvarchar max mismatch"
#     assert row[9] == TEST_DATA[9], "Time mismatch"
#     assert row[10] == TEST_DATA[10], "Datetime mismatch"
#     assert row[11] == TEST_DATA[11], "Date mismatch"
#     assert round(row[12], 5) == round(TEST_DATA[12], 5), "Real mismatch"


def test_arraysize(cursor):
    """Test arraysize"""
    cursor.arraysize = 10
    assert cursor.arraysize == 10, "Arraysize mismatch"
    cursor.arraysize = 5
    assert cursor.arraysize == 5, "Arraysize mismatch after change"


def test_description(cursor):
    """Test description"""
    cursor.execute("SELECT * FROM #pytest_all_data_types WHERE id = 1")
    desc = cursor.description

tests/test_004_cursor.py:1270

This comment appears to contain commented-out code.

    db_connection.commit()
    data = [(i,) for i in range(1, 12)]
    cursor.executemany("INSERT INTO #pytest_all_data_types (id) VALUES (?)", data)
    cursor.execute("SELECT COUNT(*) FROM #pytest_all_data_types")
    count = cursor.fetchone()[0]
    assert count == 11, "Executemany failed"

tests/test_004_cursor.py:636

Testing for None should use the 'is' operator.

        assert rows[0] == [

tests/test_004_cursor.py:679

Testing for None should use the 'is' operator.

        assert rows[0] == [

tests/test_004_cursor.py:720

Testing for None should use the 'is' operator.

        assert rows[0] == ["ABCDEFGHI"], "SQL_LONGVARCHAR parsing failed for fetchone - row 0"

tests/test_004_cursor.py:753

Testing for None should use the 'is' operator.

        assert rows[0] == ["ABCDEFGHI"], "SQL_LONGWVARCHAR parsing failed for fetchone - row 0"

tests/test_004_cursor.py:786

Testing for None should use the 'is' operator.

        assert rows[0] == [

tests/test_004_cursor.py:2698

Variable value is not used.

        cursor.execute("SELECT * FROM #pytest_row_comparison_test WHERE col1 = 20")

tests/test_004_cursor.py:3005

Variable cursor_ref is not used.
tests/test_004_cursor.py:3844
Variable row3 is not used.
tests/test_004_cursor.py:3849
Variable row4 is not used.

        assert remaining_rows[0][0] == 5 and remaining_rows[5][0] == 10, "Should have rows 5-10"

tests/test_004_cursor.py:3854

Variable row5 is not used.

        cursor.execute("DROP TABLE #pytest_rownumber_mixed_test")

tests/test_004_cursor.py:4133

Variable row1 is not used.
tests/test_004_cursor.py:4135
Variable row2 is not used.
tests/test_004_cursor.py:4137
Variable row3 is not used.

        pytest.fail(f"Safe nextset() different sizes test failed: {e}")

tests/test_004_cursor.py:4352

Variable has_next is not used.

            count = cursor.fetchone()[0]

tests/test_004_cursor.py:4790

Variable result is not used.

        # Create test table

tests/test_004_cursor.py:7103

Variable is_unix is not used.

        drop_table_if_exists(cursor, "#pytest_utf16_validation")

tests/test_004_cursor.py:10454

Variable first_duration is not used.

        ).fetchall()

tests/test_004_cursor.py:10462

Variable second_duration is not used.

        assert hasattr(proc, "num_output_params"), "Result should have num_output_params column"

tests/test_004_cursor.py:12343

Variable col_names_lower is not used.
tests/test_004_cursor.py:15485
Variable guid_value is not used.
tests/test_004_cursor.py:15540
Variable guid_value is not used.

        drop_table_if_exists(cursor, "#pytest_lob_char")

tests/test_004_cursor.py:16665

Variable cursor is not used.
tests/test_004_cursor.py:873
This assignment to 'test_rowcount' is unnecessary as it is redefined before this value is used.

    try:

tests/test_004_cursor.py:4320

This assignment to 'has_next' is unnecessary as it is redefined before this value is used.

    try:

tests/test_004_cursor.py:8673

This assignment to 'test_decimal_separator_with_multiple_values' is unnecessary as it is redefined before this value is used.

        cursor.execute("SELECT id, decimal_value FROM #pytest_decimal_separator_test")

tests/test_004_cursor.py:8728

This assignment to 'test_decimal_separator_calculations' is unnecessary as it is redefined before this value is used.
tests/test_004_cursor.py:8782
This assignment to 'test_decimal_separator_function' is unnecessary as it is redefined before this value is used.

        assert "username" in column_names2, "Column names should be lowercase when lowercase=True"

tests/test_004_cursor.py:8836

This assignment to 'test_decimal_separator_basic_functionality' is unnecessary as it is redefined before this value is used.
tests/test_004_cursor.py:8872
This assignment to 'test_lowercase_attribute' is unnecessary as it is redefined before this value is used.

        # Test setting to other valid separators

tests/test_004_cursor.py:8947

This assignment to 'test_decimal_separator_function' is unnecessary as it is redefined before this value is used.
tests/test_004_cursor.py:9001
This assignment to 'test_decimal_separator_basic_functionality' is unnecessary as it is redefined before this value is used.

        # Create a temp table

tests/test_004_cursor.py:9092

This assignment to 'test_decimal_separator_calculations' is unnecessary as it is redefined before this value is used.

            ),  # min offset

tests/test_004_cursor.py:9615

This assignment to 'test_lowercase_attribute' is unnecessary as it is redefined before this value is used.

            mssql_python.setDecimalSeparator(123)  # Not a string

tests/test_004_cursor.py:9690

This assignment to 'test_decimal_separator_function' is unnecessary as it is redefined before this value is used.

        INSERT INTO #pytest_decimal_calc_test VALUES (1, 10.25, 5.75)

tests/test_004_cursor.py:9744

This assignment to 'test_decimal_separator_basic_functionality' is unnecessary as it is redefined before this value is used.

    cursor.execute("SELECT * FROM #test_inputsizes")

tests/test_004_cursor.py:9780

This assignment to 'test_decimal_separator_with_multiple_values' is unnecessary as it is redefined before this value is used.
tests/test_004_cursor.py:9835
This assignment to 'test_decimal_separator_calculations' is unnecessary as it is redefined before this value is used.
tests/test_004_cursor.py:12463
This assignment to 'test_columns_nonexistent' is unnecessary as it is redefined before this value is used.
tests/test_004_cursor.py:15708
This assignment to 'test_all_numeric_types_with_nulls' is unnecessary as it is redefined before this value is used.

    """Test DECIMAL/NUMERIC type conversion including edge cases"""

tests/test_004_cursor.py:15762

This assignment to 'test_lob_data_types' is unnecessary as it is redefined before this value is used.

    try:

tests/test_004_cursor.py:15804

This assignment to 'test_lob_char_column_types' is unnecessary as it is redefined before this value is used.

        cursor.execute("INSERT INTO #pytest_binary_test VALUES (1, 0x0102030405)")

tests/test_004_cursor.py:15838

This assignment to 'test_lob_wchar_column_types' is unnecessary as it is redefined before this value is used.

            CREATE TABLE #pytest_all_numeric_nulls (

tests/test_004_cursor.py:15872

This assignment to 'test_lob_binary_column_types' is unnecessary as it is redefined before this value is used.

        assert rows[1][4] == True, "BIT column should be True"

tests/test_004_cursor.py:15906

This assignment to 'test_zero_length_complex_types' is unnecessary as it is redefined before this value is used.

            (1, large_text, large_ntext, large_binary),

tests/test_004_cursor.py:15943

This assignment to 'test_guid_with_nulls' is unnecessary as it is redefined before this value is used.

        cursor.execute("SELECT id, char_lob FROM #pytest_lob_char")

tests/test_004_cursor.py:15977

This assignment to 'test_datetimeoffset_with_nulls' is unnecessary as it is redefined before this value is used.
tests/test_004_cursor.py:16011
This assignment to 'test_decimal_conversion_edge_cases' is unnecessary as it is redefined before this value is used.

        assert row[1] == large_binary_data, "VARBINARY(MAX) LOB data should match"

tests/test_004_cursor.py:16066

This assignment to 'test_fixed_length_char_type' is unnecessary as it is redefined before this value is used.

        db_connection.commit()

tests/test_004_cursor.py:16086

This assignment to 'test_fixed_length_nchar_type' is unnecessary as it is redefined before this value is used.
tests/test_004_cursor.py:16106
This assignment to 'test_fixed_length_binary_type' is unnecessary as it is redefined before this value is used.

        cursor.execute("SELECT id, dto_col FROM #pytest_dto_nulls ORDER BY id")

tests/test_004_cursor.py:15

Module 'decimal' is imported with both 'import' and 'import from'.

import decimal

tests/test_004_cursor.py:14

Import of 'time_module' is not used.

import time as time_module

tests/test_004_cursor.py:2013

This import of module uuid is redundant, as it was previously imported on line 18.

                    f"thread_{thread_id}_row_{i}" if i % 3 != 0 else None,

tests/test_004_cursor.py:3147

Except block directly handles BaseException.

        # Create test table

tests/test_004_cursor.py:3219

Except block directly handles BaseException.

        # Test execute().fetchmany() chaining with size parameter

tests/test_004_cursor.py:3253

Except block directly handles BaseException.
tests/test_004_cursor.py:3287
Except block directly handles BaseException.

        "SELECT 1 as int_col, 'test' as str_col, GETDATE() as date_col"

tests/test_004_cursor.py:3331

Except block directly handles BaseException.

        assert all_rows[1] == ["second"], "Second row should be 'second'"

tests/test_004_cursor.py:3389

Except block directly handles BaseException.

        # Test iteration over execute() result (should work because cursor implements __iter__)

tests/test_004_cursor.py:3424

Except block directly handles BaseException.

        # Insert test data

tests/test_004_cursor.py:3465

Except block directly handles BaseException.

        # Test 6: Test with empty result set

tests/test_004_cursor.py:3537

Except block directly handles BaseException.

        assert next_row is None, "No more rows should return None"

tests/test_004_cursor.py:3597

Except block directly handles BaseException.

        # try:

tests/test_004_cursor.py:3686

Except block directly handles BaseException.

        except:

tests/test_004_cursor.py:3740

Except block directly handles BaseException.
tests/test_004_cursor.py:3806
Except block directly handles BaseException.

        assert (

tests/test_004_cursor.py:3870

Except block directly handles BaseException.

        # Try fetchmany on empty result

tests/test_004_cursor.py:3944

Except block directly handles BaseException.
tests/test_004_cursor.py:4012
Except block directly handles BaseException.
tests/test_004_cursor.py:4057
Except block directly handles BaseException.

            (4, "B"),

tests/test_004_cursor.py:4198

Except block directly handles BaseException.

    finally:

tests/test_004_cursor.py:4258

Except block directly handles BaseException.

def test_nextset_error_conditions_safe(cursor, db_connection):

tests/test_004_cursor.py:4310

Except block directly handles BaseException.
tests/test_004_cursor.py:4524
Except block directly handles BaseException.

def test_fetchval_multiple_columns(cursor, db_connection):

tests/test_004_cursor.py:4552

Except block directly handles BaseException.

            cursor.execute("DROP TABLE #pytest_fetchval_multi")

tests/test_004_cursor.py:4580

Except block directly handles BaseException.

        try:

tests/test_004_cursor.py:4614

Except block directly handles BaseException.
tests/test_004_cursor.py:4643
Except block directly handles BaseException.

        next_row = cursor.fetchone()

tests/test_004_cursor.py:4712

Except block directly handles BaseException.

        result = cursor.fetchval()

tests/test_004_cursor.py:4751

Except block directly handles BaseException.

        # Insert some test data

tests/test_004_cursor.py:4839

Except block directly handles BaseException.

        cursor.execute("CREATE TABLE #pytest_cursor_rollback (id INTEGER, name VARCHAR(50))")

tests/test_004_cursor.py:4886

Except block directly handles BaseException.

        # Create two cursors

tests/test_004_cursor.py:4935

Except block directly handles BaseException.

        except:

tests/test_004_cursor.py:4995

Except block directly handles BaseException.
tests/test_004_cursor.py:5053
Except block directly handles BaseException.

        # Test 2: Use connection.commit()

tests/test_004_cursor.py:5140

Except block directly handles BaseException.
tests/test_004_cursor.py:5197
Except block directly handles BaseException.

            cursor.commit()  # This might succeed depending on when the constraint is checked

tests/test_004_cursor.py:5232

Except block directly handles BaseException.

        original_autocommit = db_connection.autocommit

tests/test_004_cursor.py:5284

Except block directly handles BaseException.

    if is_azure_sql_connection(conn_str):

tests/test_004_cursor.py:5337

Except block directly handles BaseException.

        # Verify data integrity

tests/test_004_cursor.py:5410

Except block directly handles BaseException.

        db_connection.autocommit = False

tests/test_004_cursor.py:5461

Except block directly handles BaseException.

        final_stage = cursor.fetchval()

tests/test_004_cursor.py:5531

Except block directly handles BaseException.

        # Verify batch distribution

tests/test_004_cursor.py:5612

Except block directly handles BaseException.

        assert count == 1, "Should have only the committed record"

tests/test_004_cursor.py:5685

Except block directly handles BaseException.
tests/test_004_cursor.py:5762
Except block directly handles BaseException.

        # Verify data consistency after rollback

tests/test_004_cursor.py:5854

Except block directly handles BaseException.

        assert baseline_data == "baseline", "Baseline data should be unchanged"

tests/test_004_cursor.py:5932

Except block directly handles BaseException.

def test_cursor_skip_closed_cursor(db_connection):

tests/test_004_cursor.py:6938

Except block directly handles BaseException.

        error_msg = str(e).lower()

tests/test_004_cursor.py:11376

Except block directly handles BaseException.

        ], f"Invalid pseudo_column value: {col.pseudo_column}"

tests/test_004_cursor.py:11443

Except block directly handles BaseException.

    # Should return empty list, not error

tests/test_004_cursor.py:13507

Except block directly handles BaseException.

        cursor.execute(

tests/test_004_cursor.py:13552

Except block directly handles BaseException.

        db_connection.commit()

tests/test_004_cursor.py:16652

Except block directly handles BaseException.
tests/test_004_cursor.py:16132
This except block handling Exception is unreachable; as this except block also handles Exception.

        # Insert various decimal values including edge cases

tests/test_004_cursor.py:3219

'except' clause does nothing but pass and there is no explanatory comment.

        # Test execute().fetchmany() chaining with size parameter

tests/test_004_cursor.py:3253

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:3287
'except' clause does nothing but pass and there is no explanatory comment.

        "SELECT 1 as int_col, 'test' as str_col, GETDATE() as date_col"

tests/test_004_cursor.py:3331

'except' clause does nothing but pass and there is no explanatory comment.

        assert all_rows[1] == ["second"], "Second row should be 'second'"

tests/test_004_cursor.py:3389

'except' clause does nothing but pass and there is no explanatory comment.

        # Test iteration over execute() result (should work because cursor implements __iter__)

tests/test_004_cursor.py:3424

'except' clause does nothing but pass and there is no explanatory comment.

        # Insert test data

tests/test_004_cursor.py:3465

'except' clause does nothing but pass and there is no explanatory comment.

        # Test 6: Test with empty result set

tests/test_004_cursor.py:3537

'except' clause does nothing but pass and there is no explanatory comment.

        assert next_row is None, "No more rows should return None"

tests/test_004_cursor.py:3597

'except' clause does nothing but pass and there is no explanatory comment.

        # try:

tests/test_004_cursor.py:3686

'except' clause does nothing but pass and there is no explanatory comment.

        except:

tests/test_004_cursor.py:3740

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:3806
'except' clause does nothing but pass and there is no explanatory comment.

        assert (

tests/test_004_cursor.py:3870

'except' clause does nothing but pass and there is no explanatory comment.

        # Try fetchmany on empty result

tests/test_004_cursor.py:3944

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:4012
'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:4057
'except' clause does nothing but pass and there is no explanatory comment.

            (4, "B"),

tests/test_004_cursor.py:4198

'except' clause does nothing but pass and there is no explanatory comment.

    finally:

tests/test_004_cursor.py:4258

'except' clause does nothing but pass and there is no explanatory comment.

def test_nextset_error_conditions_safe(cursor, db_connection):

tests/test_004_cursor.py:4310

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:4344
'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:4524
'except' clause does nothing but pass and there is no explanatory comment.

def test_fetchval_multiple_columns(cursor, db_connection):

tests/test_004_cursor.py:4552

'except' clause does nothing but pass and there is no explanatory comment.

            cursor.execute("DROP TABLE #pytest_fetchval_multi")

tests/test_004_cursor.py:4580

'except' clause does nothing but pass and there is no explanatory comment.

        try:

tests/test_004_cursor.py:4614

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:4643
'except' clause does nothing but pass and there is no explanatory comment.

        next_row = cursor.fetchone()

tests/test_004_cursor.py:4712

'except' clause does nothing but pass and there is no explanatory comment.

        result = cursor.fetchval()

tests/test_004_cursor.py:4751

'except' clause does nothing but pass and there is no explanatory comment.

        # Insert some test data

tests/test_004_cursor.py:4839

'except' clause does nothing but pass and there is no explanatory comment.

        cursor.execute("CREATE TABLE #pytest_cursor_rollback (id INTEGER, name VARCHAR(50))")

tests/test_004_cursor.py:4886

'except' clause does nothing but pass and there is no explanatory comment.

        # Create two cursors

tests/test_004_cursor.py:4935

'except' clause does nothing but pass and there is no explanatory comment.

        except:

tests/test_004_cursor.py:4995

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:5053
'except' clause does nothing but pass and there is no explanatory comment.

        # Test 2: Use connection.commit()

tests/test_004_cursor.py:5140

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:5197
'except' clause does nothing but pass and there is no explanatory comment.

            cursor.commit()  # This might succeed depending on when the constraint is checked

tests/test_004_cursor.py:5232

'except' clause does nothing but pass and there is no explanatory comment.

        original_autocommit = db_connection.autocommit

tests/test_004_cursor.py:5284

'except' clause does nothing but pass and there is no explanatory comment.

    if is_azure_sql_connection(conn_str):

tests/test_004_cursor.py:5337

'except' clause does nothing but pass and there is no explanatory comment.

        # Verify data integrity

tests/test_004_cursor.py:5410

'except' clause does nothing but pass and there is no explanatory comment.

        db_connection.autocommit = False

tests/test_004_cursor.py:5461

'except' clause does nothing but pass and there is no explanatory comment.

        final_stage = cursor.fetchval()

tests/test_004_cursor.py:5531

'except' clause does nothing but pass and there is no explanatory comment.

        # Verify batch distribution

tests/test_004_cursor.py:5612

'except' clause does nothing but pass and there is no explanatory comment.

        assert count == 1, "Should have only the committed record"

tests/test_004_cursor.py:5685

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:5762
'except' clause does nothing but pass and there is no explanatory comment.

        # Verify data consistency after rollback

tests/test_004_cursor.py:5854

'except' clause does nothing but pass and there is no explanatory comment.

        assert baseline_data == "baseline", "Baseline data should be unchanged"

tests/test_004_cursor.py:5932

'except' clause does nothing but pass and there is no explanatory comment.

def test_cursor_skip_closed_cursor(db_connection):

tests/test_004_cursor.py:5941

'except' clause does nothing but pass and there is no explanatory comment.

        "closed" in str(exc_info.value).lower()

tests/test_004_cursor.py:11376

'except' clause does nothing but pass and there is no explanatory comment.

        ], f"Invalid pseudo_column value: {col.pseudo_column}"

tests/test_004_cursor.py:11443

'except' clause does nothing but pass and there is no explanatory comment.

    # Should return empty list, not error

tests/test_004_cursor.py:13507

'except' clause does nothing but pass and there is no explanatory comment.

        cursor.execute(

tests/test_004_cursor.py:15174

'except' clause does nothing but pass and there is no explanatory comment.

        drop_table_if_exists(cursor, "#pytest_uuid_braces")

tests/test_004_cursor.py:16652

'except' clause does nothing but pass and there is no explanatory comment.
tests/test_004_cursor.py:3011
This statement is unreachable.

            final_count = cursor.fetchone()[0]

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

mssql_python/pybind/ddbc_bindings.cpp

tests/test_004_cursor.py

dlevy-msft-sql · 2026-01-20T17:13:56Z

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 16 comments.

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

Copilot Review Response

Issues Fixed in This PR

Comment	Status	Action Taken
Redundant `import mssql_python` inside test functions	✅ Fixed	Removed local imports - module already imported at line 17
Empty `except` blocks without comments	✅ Fixed	Rewrote tests with proper cleanup pattern (setup before try, explicit rollback after each error, deterministic finally)
Wrong table name in finally block	✅ Fixed	Changed `#pytest_hierarchyid_invalid` → `#pytest_geometry_invalid`

Pre-existing Issues (Not Introduced by This PR)

The following issues flagged by Copilot exist in main and are out of scope for this PR:

Issue	Evidence	Tracking
Duplicate test functions (4x each)	Lines 8545-12648 - existed before this PR	Separate issue filed
20+ bare `except: pass` blocks	Lines 3097-4464 - existed before this PR	Separate issue filed
Module imported with both `import` and `from import`	Top-level `import mssql_python` + function-level `from mssql_python import connect` is intentional (different use cases)	N/A - not an issue

Verification

# Confirm we REMOVED a bare except (not added)
git diff main...HEAD -- [test_004_cursor.py](http://_vscodecontentref_/0) | grep "except:"
# Output: -        except:   (minus = removed)

# All our added except blocks have proper handling
git diff main...HEAD -- [test_004_cursor.py](http://_vscodecontentref_/1) | grep "^\+.*except"
# Output: +        except Exception as e:  (proper exception type)

Copilot

Pull request overview

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

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

mssql_python/constants.py

Copilot

Pull request overview

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

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

Copilot

Pull request overview

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

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

dlevy-msft-sql · 2026-01-21T14:57:39Z

Left a few comments...

Thinking we're closing in on ready to merge.

Copilot

Pull request overview

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

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

Copilot · 2026-01-30T02:03:51Z

mssql_python/cursor.py

+                ddbc_sql_const.SQL_NUMERIC.value: decimal.Decimal,
+                ddbc_sql_const.SQL_DATE.value: datetime.date,
+                ddbc_sql_const.SQL_TIMESTAMP.value: datetime.datetime,
+                ddbc_sql_const.SQL_TIME.value: datetime.time,


SQLTypeCode._type_map doesn’t include ODBC 3.x date/time codes (e.g., SQL_TYPE_DATE=91, SQL_TYPE_TIME=92, SQL_TYPE_TIMESTAMP=93, SQL_TYPE_TIMESTAMP_WITH_TIMEZONE=95). If SQLDescribeCol returns these codes, SQLTypeCode will default to str and cursor.description[i][1] == datetime.date/time/datetime.datetime will be incorrect. Add these codes to the mapping (and unit tests) to keep python_type/equality accurate for date/time columns.

Suggested change

ddbc_sql_const.SQL_TIME.value: datetime.time,

ddbc_sql_const.SQL_TIME.value: datetime.time,

# ODBC 3.x date/time type codes (SQL_TYPE_DATE/TIME/TIMESTAMP/TIMESTAMP_WITH_TIMEZONE)

91: datetime.date, # SQL_TYPE_DATE

92: datetime.time, # SQL_TYPE_TIME

93: datetime.datetime, # SQL_TYPE_TIMESTAMP

95: datetime.datetime, # SQL_TYPE_TIMESTAMP_WITH_TIMEZONE

Copilot · 2026-01-30T02:03:51Z

mssql_python/cursor.py

+    def __hash__(self):
+        return hash(self.type_code)


SQLTypeCode defines hash as hash(type_code), but eq can return True when compared to objects with unrelated hash values (e.g., SQLTypeCode(-9) == str). This violates Python’s hash/eq contract and will make set/dict behavior inconsistent. Consider making SQLTypeCode unhashable (remove/set hash = None) or narrowing equality semantics so only same-kind objects compare equal.

Suggested change

def __hash__(self):

return hash(self.type_code)

# Instances are intentionally unhashable because __eq__ allows

# comparisons to both Python types and integer SQL codes, and

# there is no single hash value that can be consistent with both.

__hash__ = None

Copilot · 2026-01-30T02:03:51Z

mssql_python/cursor.py

+    def __int__(self):
+        return self.type_code


SQLTypeCode is not an int, so passing cursor.description[i][1] into APIs that expect an integer SQL type (e.g., add_output_converter) will register/look up converters under a non-int key and break matching when the driver later uses raw integer SQL type codes. Consider normalizing such APIs to int(sqltype) for SQLTypeCode inputs and/or implementing index / an int-subclass approach.

Copilot · 2026-01-30T02:03:52Z

tests/test_002_types.py

+    def test_sqltypecode_hash(self):
+        """Test SQLTypeCode is hashable (for use in dicts/sets)."""
+        from mssql_python import SQLTypeCode
+
+        tc1 = SQLTypeCode(4)
+        tc2 = SQLTypeCode(4)
+        tc3 = SQLTypeCode(-9)
+        # Same code should have same hash
+        assert hash(tc1) == hash(tc2)
+        # Different codes may have different hashes (not guaranteed but typical)
+        s = {tc1, tc3}
+        assert len(s) == 2
+


This test requires SQLTypeCode to be hashable, but SQLTypeCode is also designed to compare equal to both SQL integer codes and Python type objects. Those targets have unrelated hash values, so this combination violates Python’s hash/eq contract and will cause inconsistent set/dict behavior. Prefer making SQLTypeCode unhashable (and drop this test) or change SQLTypeCode equality semantics.

Suggested change

def test_sqltypecode_hash(self):

"""Test SQLTypeCode is hashable (for use in dicts/sets)."""

from mssql_python import SQLTypeCode

tc1 = SQLTypeCode(4)

tc2 = SQLTypeCode(4)

tc3 = SQLTypeCode(-9)

# Same code should have same hash

assert hash(tc1) == hash(tc2)

# Different codes may have different hashes (not guaranteed but typical)

s = {tc1, tc3}

assert len(s) == 2

Copilot · 2026-01-30T02:03:52Z

tests/test_004_cursor.py

+        # Note: With executemany, we need to insert the geography using a subquery or convert inline
+        # This test uses direct WKT strings and converts in a separate step
+        cursor.executemany(
+            "INSERT INTO #pytest_geography_batch (name) VALUES (?);",
+            [(name,) for _, name in test_data],
+        )
+        db_connection.commit()
+
+        rows = cursor.execute("SELECT name FROM #pytest_geography_batch ORDER BY id;").fetchall()
+        assert len(rows) == len(test_data), "Should have inserted all rows"


This executemany inserts only the name column and never populates/asserts geo_col, so it doesn’t actually test executemany behavior for GEOGRAPHY values as the test name/docstring claims. Either batch-insert geo_col (e.g., via geography::STGeomFromText) and assert it round-trips, or rename/re-scope the test.

Suggested change

# Note: With executemany, we need to insert the geography using a subquery or convert inline

# This test uses direct WKT strings and converts in a separate step

cursor.executemany(

"INSERT INTO #pytest_geography_batch (name) VALUES (?);",

[(name,) for _, name in test_data],

)

db_connection.commit()

rows = cursor.execute("SELECT name FROM #pytest_geography_batch ORDER BY id;").fetchall()

assert len(rows) == len(test_data), "Should have inserted all rows"

# Insert both geography (from WKT) and name using executemany

cursor.executemany(

"INSERT INTO #pytest_geography_batch (geo_col, name) "

"VALUES (geography::STGeomFromText(?, 4326), ?);",

[(wkt, name) for wkt, name in test_data],

)

db_connection.commit()

rows = cursor.execute(

"SELECT geo_col, name FROM #pytest_geography_batch ORDER BY id;"

).fetchall()

assert len(rows) == len(test_data), "Should have inserted all rows"

for (expected_wkt, expected_name), (geo_value, name_value) in zip(test_data, rows):

# Geography values should be returned as bytes, consistent with other geography tests

assert isinstance(geo_value, bytes), "Each geography value should be bytes"

assert name_value == expected_name, "Names should round-trip correctly"

dlevy-msft-sql added 4 commits November 24, 2025 20:05

Fix for FetchMany(number of rows) ignores batch size when table conta…

c403899

…ins an LOB

First round of code review fixes

f96d14c

Fix test_fetchone

e135e40

Fix for issue 352

38966d0

Copilot AI review requested due to automatic review settings December 1, 2025 17:19

Copilot started reviewing on behalf of dlevy-msft-sql December 1, 2025 17:20 View session

Copilot finished reviewing on behalf of dlevy-msft-sql December 1, 2025 17:23

Copilot AI reviewed Dec 1, 2025

View reviewed changes

tests/test_004_cursor.py Outdated Show resolved Hide resolved

tests/test_004_cursor.py Outdated Show resolved Hide resolved

mssql_python/constants.py Outdated Show resolved Hide resolved

mssql_python/cursor.py Outdated Show resolved Hide resolved

dlevy-msft-sql added bug Something isn't working pr-size: medium Moderate update size labels Dec 1, 2025

dlevy-msft-sql added 6 commits December 1, 2025 11:31

Apply fixes from copilot review

3f52dd7

Fix for FetchMany(number of rows) ignores batch size when table conta…

38b041b

…ins an LOB

First round of code review fixes

139994a

Fix test_fetchone

6fd823e

Fix for issue 352

a7d9b8e

Apply fixes from copilot review

646ef04

dlevy-msft-sql force-pushed the Issue-352 branch from 3f52dd7 to 646ef04 Compare December 9, 2025 20:56

dlevy-msft-sql added 2 commits January 5, 2026 13:01

Merge branch 'Issue-352' of https://github.com/dlevy-msft-sql/mssql-p…

8018872

…ython into Issue-352

Merge branch 'main' into Issue-352

67268e5

dlevy-msft-sql added 4 commits January 7, 2026 22:49

Merge branch 'main' into Issue-352

d335ed0

Merge branch 'main' into Issue-352

2dc93f3

Merge branch 'main' into Issue-352

eee5256

Merge branch 'microsoft:main' into Issue-352

d289a9b

dlevy-msft-sql added pr-size: small Minimal code update pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Jan 16, 2026

sumitmsft reviewed Jan 19, 2026

View reviewed changes

mssql_python/cursor.py Outdated Show resolved Hide resolved

sumitmsft reviewed Jan 19, 2026

View reviewed changes

mssql_python/cursor.py Outdated Show resolved Hide resolved

sumitmsft reviewed Jan 19, 2026

View reviewed changes

mssql_python/cursor.py Show resolved Hide resolved

Copilot AI reviewed Jan 20, 2026

View reviewed changes

dlevy-msft-sql added 3 commits January 20, 2026 10:10

test: Add spatial type error handling and thread safety tests

466a4d5

- Add geography/geometry/hierarchyid invalid parsing tests - Add concurrent cursor thread safety test for _column_metadata - Add sequential query isolation test for column metadata

test: Add spatial type error handling and thread safety tests

5e67a1b

- Add geography/geometry/hierarchyid invalid parsing tests - Add concurrent cursor thread safety test for _column_metadata - Add sequential query isolation test for column metadata

Merge remote branch, keep table name fix

f484e8d

dlevy-msft-sql requested a review from Copilot January 20, 2026 16:18

Copilot started reviewing on behalf of dlevy-msft-sql January 20, 2026 16:19 View session

Ran black --line-length=100 mssql_python/ tests/ to fix issue

c0c2117

Copilot AI reviewed Jan 20, 2026

View reviewed changes

mssql_python/pybind/ddbc_bindings.cpp Show resolved Hide resolved

tests/test_004_cursor.py Show resolved Hide resolved

tests/test_004_cursor.py Show resolved Hide resolved

tests/test_004_cursor.py Show resolved Hide resolved

dlevy-msft-sql mentioned this pull request Jan 20, 2026

Code quality: Remove duplicate test functions and bare except blocks #409

Open

dlevy-msft-sql requested a review from Copilot January 20, 2026 17:14

Copilot started reviewing on behalf of dlevy-msft-sql January 20, 2026 17:15 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

mssql_python/constants.py Show resolved Hide resolved

docs: Add sync comment for SQL Server constants

b5460dd

dlevy-msft-sql requested a review from Copilot January 20, 2026 17:30

Copilot started reviewing on behalf of dlevy-msft-sql January 20, 2026 17:30 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

dlevy-msft-sql added 2 commits January 20, 2026 11:37

fix: Use per-thread connections in thread safety test (MARS not enabled)

bced470

style: Fix black formatting

3d09f1a

dlevy-msft-sql requested a review from Copilot January 20, 2026 17:46

Copilot started reviewing on behalf of dlevy-msft-sql January 20, 2026 17:47 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

dlevy-msft-sql added 2 commits January 21, 2026 18:48

Merge branch 'microsoft:main' into Issue-352

fe2a042

Merge branch 'main' into Issue-352

9913467

dlevy-msft-sql requested a review from sumitmsft January 27, 2026 05:32

Merge branch 'main' into Issue-352

c0b619e

dlevy-msft-sql requested a review from Copilot January 30, 2026 01:30

Copilot started reviewing on behalf of dlevy-msft-sql January 30, 2026 01:53 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

-                ddbc_sql_const.SQL_TIME.value: datetime.time,
+                ddbc_sql_const.SQL_TIME.value: datetime.time,
+                # ODBC 3.x date/time type codes (SQL_TYPE_DATE/TIME/TIMESTAMP/TIMESTAMP_WITH_TIMEZONE)
+: datetime.date,        # SQL_TYPE_DATE
+: datetime.time,        # SQL_TYPE_TIME
+: datetime.datetime,    # SQL_TYPE_TIMESTAMP
+: datetime.datetime,    # SQL_TYPE_TIMESTAMP_WITH_TIMEZONE

-    def __hash__(self):
-        return hash(self.type_code)
+    # Instances are intentionally unhashable because __eq__ allows
+    # comparisons to both Python types and integer SQL codes, and
+    # there is no single hash value that can be consistent with both.
+    __hash__ = None

FIX: Cursor.describe invalid data #355

Are you sure you want to change the base?

FIX: Cursor.describe invalid data #355

Conversation

dlevy-msft-sql commented Dec 1, 2025

Work Item / Issue Reference

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Code Coverage Report

🔥 Diff Coverage

98%

🎯 Overall Coverage

76%

Diff Coverage

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

Summary

mssql_python/cursor.py

📋 Files Needing Attention

🔗 Quick Links

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dlevy-msft-sql commented Jan 20, 2026

Pull request overview

Copilot Review Response

Issues Fixed in This PR

Pre-existing Issues (Not Introduced by This PR)

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

dlevy-msft-sql commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 5, 2026 •

edited

Loading