-
Notifications
You must be signed in to change notification settings - Fork 438
Fix update count handling for multi-statement queries executed via PreparedStatement. (#2722) #2737
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2737 +/- ##
=======================================
Coverage ? 51.54%
Complexity ? 4071
=======================================
Files ? 149
Lines ? 34244
Branches ? 5719
=======================================
Hits ? 17650
Misses ? 14104
Partials ? 2490 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the changes around unnecessary catching of exceptions in new tests. Otherwise approved.
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
17707a5
Description
This PR addresses an inconsistency in how PreparedStatement handles update counts for multi-statement queries, particularly when INSERT operations are involved.
Issue
When executing multi-statement queries containing INSERT operations, PreparedStatement returned different update counts compared to Statement due to differences in Tabular Data Stream (TDS) token processing:
Example of inconsistent behavior:
PreparedStatement ps = conn.prepareStatement( "DELETE FROM TestTable; " + "INSERT INTO TestTable (c1,c2) VALUES (?, ?); " + "INSERT INTO TestTable (c1,c2) VALUES (?, ?); " + "UPDATE TestTable SET C1 = 1; " + "INSERT INTO TestTable (c1,c2) VALUES (?, ?); " + "SELECT * FROM TestTable" ); do { boolean hasResults = ps.getMoreResults(); System.out.println("update count: " + ps.getUpdateCount() + ", has result: " + hasResults); } while (true);
update count: 3, has result :false update count: 1, has result :false update count: 1, has result :false update count: 2, has result :false update count: 1, has result :false update count: -1, has result :true
update count: 3, has result :false update count: 2, has result :false update count: 1, has result :false update count: -1, has result :true
Root Cause
In PR #2554, the following logic was added to handle INSERT operations:
if ((StreamDone.CMD_INSERT == doneToken.getCurCmd()) && (-1 != doneToken.getUpdateCount()) && EXECUTE == executeMethod) { return true; }
This logic is correct for Statement but not for PreparedStatement.
For PreparedStatement, the execute API for INSERT does not require reading an additional explicit TDS_DONE token — applying the same logic caused incorrect update counts.
Fix
Added overridden methods to correctly handle update count retrieval in both PreparedStatement and SQLServerStatement, ensuring that each uses the appropriate TDS token processing logic.
Testing
Closes #2722