FIX: GH Issue #333 "cursor.execute() with INSERT statement containing OUTPUT inserted + multiple VALUES entries does not raise IntegrityError" #338
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes GitHub Issue #333, which addresses a bug where fetchall() does not properly raise IntegrityError when executing INSERT statements with OUTPUT clauses that violate integrity constraints. The fix adds error checking after the fetch operation to ensure errors are properly detected and raised.
Key Changes:
- Added
check_error()call afterDDBCSQLFetchAll()in thefetchall()method to properly handle and raise errors returned by the fetch operation - This brings
fetchall()into consistency with other DDBC binding calls throughout the codebase that usecheck_error()for error handling
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.cpp: 67.1%
mssql_python.helpers.py: 67.5%
mssql_python.pybind.connection.connection.cpp: 74.7%
mssql_python.pybind.ddbc_bindings.h: 76.9%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 82.5%
mssql_python.cursor.py: 83.8%🔗 Quick Links
|
gargsaumya
force-pushed
the
saumya/integrity-error
branch
from
9f1f3cb to
4a0f5f6
Compare
gargsaumya
force-pushed
the
saumya/integrity-error
branch
from
1168cd6 to
8273dcc
Compare
|
Considering the fix is for making sure that the errors are thrown in fetch* API, a test should be added to validate this behavior. |
saurabh500
left a comment
saurabh500
left a comment
There was a problem hiding this comment.
Need tests for conditions where the silent failure is now avoided
gargsaumya
force-pushed
the
saumya/integrity-error
branch
from
befc9ef to
b1b70f4
Compare
github-actions
bot
added
pr-size: medium
gargsaumya
force-pushed
the
saumya/integrity-error
branch
from
87c4059 to
e91baeb
Compare
github-actions
bot
added
pr-size: small
gargsaumya
force-pushed
the
saumya/integrity-error
branch
2 times, most recently
from
4aa4445 to
cce66f0
Compare
github-actions
bot
added
pr-size: medium
gargsaumya
force-pushed
the
saumya/integrity-error
branch
from
62636b2 to
237d0b5
Compare
sumitmsft
left a comment
sumitmsft
left a comment
There was a problem hiding this comment.
As discussed, please resolve this PR and add new issues for other fetch* apis on GH and corresponding bugs in ADO. Please label the GH issues appropriately.
gargsaumya
dismissed
saurabh500’s stale review
Added the requested test. For the other fetch APIs, the return-code checks will be added in an upcoming PR that also fixes an issue we’ve identified in those APIs.
Thanks.
Work Item / Issue Reference
Summary
This pull request introduces an error handling improvement to the
fetchallmethod inmssql_python/cursor.py. The change ensures that errors from the fetch operation are properly checked and handled.Error handling enhancement:
check_errorafter fetching data in thefetchallmethod to verify and handle any errors returned byDDBCSQLFetchAll.