Skip to content
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

Tvp server cursor fix #234

Merged
merged 20 commits into from
Apr 25, 2017
Merged

Conversation

xiangyushawn
Copy link
Contributor

@xiangyushawn xiangyushawn commented Apr 5, 2017

fixes #224

}

private static void dropTables() throws SQLException {
stmt.executeUpdate("if object_id('" + srcTable + "','U') is not null" + " drop table " + srcTable);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use Utils.dropTableIfExists to drop source & dest tables ?

*/
@Test
public void testServerCursors() throws SQLException {
conn = DriverManager.getConnection(connectionString);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add one test case for 3 result sets (2 TVPType and one Non-TVP) & 2 / 3 statements for these resultsets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@codecov-io
Copy link

codecov-io commented Apr 5, 2017

Codecov Report

Merging #234 into dev will increase coverage by 0.95%.
The diff coverage is 92.98%.

Impacted file tree graph

@@             Coverage Diff             @@
##                dev    #234      +/-   ##
===========================================
+ Coverage     33.45%   34.4%   +0.95%     
- Complexity     1487    1551      +64     
===========================================
  Files            97     101       +4     
  Lines         23390   23656     +266     
  Branches       3840    3878      +38     
===========================================
+ Hits           7824    8139     +315     
+ Misses        14002   13917      -85     
- Partials       1564    1600      +36
Flag Coverage Δ Complexity Δ
#JDBC41 34.2% <92.98%> (+0.9%) 1542 <1> (+67) ⬆️
#JDBC42 34.32% <92.98%> (+0.93%) 1547 <1> (+62) ⬆️
Impacted Files Coverage Δ Complexity Δ
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java 32.25% <ø> (+2.63%) 37 <0> (+1) ⬆️
...om/microsoft/sqlserver/jdbc/SQLServerResource.java 100% <ø> (ø) 4 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 26.2% <100%> (+2.19%) 184 <1> (+14) ⬆️
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 39.38% <92.85%> (+2.43%) 0 <0> (ø) ⬇️
...va/com/microsoft/sqlserver/jdbc/SQLServerBlob.java 26.06% <0%> (-0.29%) 13% <0%> (ø)
...t/sqlserver/jdbc/SQLServerConnectionPoolProxy.java 13.61% <0%> (-0.22%) 12% <0%> (ø)
...m/microsoft/sqlserver/jdbc/KerbAuthentication.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
.../microsoft/sqlserver/jdbc/SQLServerXAResource.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
...com/microsoft/sqlserver/jdbc/dns/DNSRecordSRV.java 0% <0%> (ø) 0% <0%> (?)
...com/microsoft/sqlserver/jdbc/dns/DNSUtilities.java 0% <0%> (ø) 0% <0%> (?)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a0f330...c23e499. Read the comment docs.

@Suraiya-Hameed Suraiya-Hameed self-requested a review April 6, 2017 22:03
@Suraiya-Hameed
Copy link
Contributor

In ResultSet+storedProcedure+Cursor combination, no exception is thrown if there is no storedProcedure or if invalid/incompatible storedProcedure name is passed.

@xiangyushawn
Copy link
Contributor Author

@v-suhame fixed exception issue with invalid SP/TVP name


// reset command status which have been overwritten
if (tdsWritterCached) {
command.setRequestComplete(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of resetting them to default, it will be safe to catch original value of these 3 variables and reset them. Also update these values in synchronized way just as in the existing code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thank you for the findings

@xiangyushawn xiangyushawn merged commit 18a9265 into microsoft:dev Apr 25, 2017
@xiangyushawn xiangyushawn deleted the TVP-Server-Cursor-Fix branch April 25, 2017 19:51
@lilgreenbird lilgreenbird added this to Closed/Merged PRs in MSSQL JDBC Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
MSSQL JDBC
  
Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

TVP error with server cursor
5 participants