FEAT: Adding cursor.lastrowid attribute

[jahnvi480]

Work Item / Issue Reference

AB#34897

---

Summary

This pull request adds support for the lastrowid attribute to the Cursor class, allowing users to retrieve the row ID of the last inserted row after single-row INSERT operations. The implementation ensures compliance with DB-API semantics and includes comprehensive tests to verify correct behavior in various scenarios.

Enhancements to Cursor functionality:

• Added a read-only lastrowid property to the Cursor class, tracking the row ID of the last modified row, and ensuring it is set only for single-row INSERT operations.
• Updated the execute method to set lastrowid using the result of SELECT @@IDENTITY after a successful single-row INSERT, and reset it to None at the start of each execution.
• Ensured that lastrowid is reset to None at the start of executemany, as semantics are undefined for multi-row operations.

Testing improvements:

• Added a comprehensive suite of tests for lastrowid, covering single and multiple inserts, executemany, non-INSERT operations, tables without identity columns, and enforcement of the read-only property.

[Copilot]

Pull Request Overview

This pull request implements the lastrowid attribute for the Cursor class, providing DB-API 2.0 compliance by allowing retrieval of the last inserted row's identity value. The implementation ensures lastrowid is only set for single-row INSERT operations and remains None for other operations or batch executions.

• Added a read-only lastrowid property that queries @@IDENTITY after successful single-row INSERTs
• Reset lastrowid to None for non-INSERT operations, multi-row INSERTs, and executemany calls
• Comprehensive test coverage for various scenarios including identity and non-identity tables

Reviewed Changes

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

File	Description
mssql_python/cursor.py	Implements lastrowid property with logic to detect single-row INSERTs and query @@IDENTITY
tests/test_004_cursor.py	Adds extensive test cases covering all lastrowid scenarios and edge cases

Comments suppressed due to low confidence (1)

tests/test_004_cursor.py:1359

• This test doesn't verify that the INSERT actually succeeded and affected multiple rows. Consider adding an assertion to check cursor.rowcount == 3 to ensure the test is validating the correct scenario.

        cursor.execute("INSERT INTO #test_lastrowid (name) VALUES ('test1'), ('test2'), ('test3')")

[bewithgaurav]

There are a few limitations on ODBC w.r.t implementation and usage of this attribute, added comments in detail. I believe we can discuss this in detail and park it for now.
Additionally, this is an optional DBAPI extension - https://peps.python.org/pep-0249/#lastrowid and is not implemented by pyodbc due to the above mentioned limitations. Lets discuss this in next meeting.

[sumitmsft]

Left a comment for @@IDENTITY VS SCOPE_IDENTITY() .. Let's discuss where the challenge is @jahnvi480 @@IDENTITY can return unexpected results if there are triggers on the table that perform additional inserts.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 82%

🎯 Overall Coverage 74%

📈 Total Lines Covered: 4204 out of 5624 📁 Project: mssql-python

---

Diff Coverage

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

• mssql_python/cursor.py (82.4%): Missing lines 1073,1075-1076

Summary

• Total: 17 lines
• Missing: 3 lines
• Coverage: 82%

mssql_python/cursor.py

Lines 1069-1080

  1069                 if (fetch_ret == ddbc_sql_const.SQL_SUCCESS.value and 
  1070                     row_data and row_data[0] is not None):
  1071                     self._lastrowid = int(row_data[0])
  1072 
! 1073         except Exception:
  1074             # If we can't get the identity, leave lastrowid as None
! 1075             self._lastrowid = None
! 1076             log('debug', "Could not retrieve lastrowid: %s", e)
  1077 
  1078         # Return self for method chaining
  1079         return self

---

---

📋 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.7%
mssql_python.pybind.connection.connection_pool.cpp: 78.9%
mssql_python.cursor.py: 79.7%
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: 87.5%

---

🔗 Quick Links

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

[sumitmsft]

Can you check if these tests are added:

• Multi-values insert (should yield None).
• INSERT ... SELECT with multiple rows (should yield None).
• Insert with trigger inserting into another identity table (demonstrates @@IDENTITY vs SCOPE_IDENTITY() difference).
• Failed INSERT (constraint violation) ensuring lastrowid stays None.

[sumitmsft]

Fetching @@IDENTITY before we execute the query is not as per DBAPI standards,

• Execute the user’s INSERT statement first (the statement that is supposed to generate the identity).
• Only after that statement has completed successfully (and you know it affected exactly one row) retrieve the identity value that that INSERT produced.
• Then set cursor.lastrowid to that value (or leave it None if conditions are not met).

[sumitmsft]

@jahnvi480 @bewithgaurav - For lastrowid I don't feel confident to ship in GA unless we have a concrete reason to ship it. My reservations are primarily due to non-compliance to DBAPI and the fact that there is a huge possibility of it returning stale or incorrect data.

[bewithgaurav]

As discussed, we can park this implementation for now owing to the ambiguous definition inside DB API 2.0 and non availability of a reliant method in SQL Server to do this. We researched on other drivers (pscopg as well as pyodbc) thoroughly but couldn't find a very good implementation and usecase ask of lastrowid, we can revisit this discussion later.