-
Notifications
You must be signed in to change notification settings - Fork 430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue#2550 - Fixed getGeneratedKeys functionality for execute API #2554
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2554 +/- ##
============================================
+ Coverage 51.11% 51.33% +0.21%
- Complexity 3924 3952 +28
============================================
Files 147 147
Lines 33505 33512 +7
Branches 5614 5616 +2
============================================
+ Hits 17126 17202 +76
+ Misses 13961 13874 -87
- Partials 2418 2436 +18 ☔ View full report in Codecov by Sentry. |
Internal tests pass |
src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/StatementTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java
Outdated
Show resolved
Hide resolved
src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/StatementTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/StatementTest.java
Outdated
Show resolved
Hide resolved
…ev/machavan/issue#2550
Suggestion: may also wish to verify that the execute API works with |
Yes, verified with execute API and added a new test case |
Description:
The execute API for INSERT was not reading off the explicit TDS_DONE token that actually contains the update count returned by the server. As a result, it was not able to correctly report back the update count for getUpdateCount and also the subsequent resultsets on getMoreResults.
Note that this works correctly for excecuteUpdate API.
Testing: