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

Fix | Fix for ResultSets being consumed when reading warnings #991

Merged
merged 8 commits into from
Apr 11, 2019

Conversation

cheenamalhotra
Copy link
Member

Fixes issue #969

@cheenamalhotra
Copy link
Member Author

Jars for testing: mssql-jdbc-PR991.zip

@codecov-io
Copy link

codecov-io commented Mar 15, 2019

Codecov Report

Merging #991 into dev will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##                dev    #991     +/-   ##
==========================================
+ Coverage     50.09%   50.2%   +0.1%     
- Complexity     2878    2888     +10     
==========================================
  Files           120     120             
  Lines         27989   27996      +7     
  Branches       4677    4680      +3     
==========================================
+ Hits          14022   14055     +33     
+ Misses        11713   11686     -27     
- Partials       2254    2255      +1
Flag Coverage Δ Complexity Δ
#JDBC42 49.77% <100%> (+0.11%) 2848 <0> (+11) ⬆️
#JDBC43 50.06% <100%> (+0.04%) 2878 <0> (+1) ⬆️
Impacted Files Coverage Δ Complexity Δ
...n/java/com/microsoft/sqlserver/jdbc/tdsparser.java 69.42% <100%> (+1.31%) 0 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 61.75% <100%> (+0.84%) 142 <0> (+5) ⬆️
...om/microsoft/sqlserver/jdbc/SimpleInputStream.java 52.23% <0%> (-1.5%) 11% <0%> (-1%)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 43.95% <0%> (-1.1%) 15% <0%> (ø)
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java 66.93% <0%> (-0.21%) 63% <0%> (ø)
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 79.72% <0%> (-0.2%) 5% <0%> (-1%)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 55.55% <0%> (+0.06%) 0% <0%> (ø) ⬇️
...ncurrentlinkedhashmap/ConcurrentLinkedHashMap.java 38.57% <0%> (+0.21%) 43% <0%> (+1%) ⬆️
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 44.17% <0%> (+0.3%) 322% <0%> (+4%) ⬆️
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 68.65% <0%> (+0.4%) 0% <0%> (ø) ⬇️
... and 3 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 a1a004e...cb05baa. Read the comment docs.

Copy link

@biasb biasb left a comment

Choose a reason for hiding this comment

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

I think testMultipleResultSets() should also test that the following is working (copied from getMoreResults() Javadoc:):

There are no more results when the following is true:
   // stmt is a Statement object
   ((stmt.getMoreResults() == false) && (stmt.getUpdateCount() == -1))

Update the test with an update of any value (or no value) that returns an update count and do another select afterwards so that a ResultSet is expected after the update count.

@biasb
Copy link

biasb commented Mar 15, 2019

I have verified that the jre8 version jar works in our test suite.

@cheenamalhotra
Copy link
Member Author

Hi @biasb

I've added the scenario as you mentioned, please let me know if I misunderstood anything.

@cheenamalhotra cheenamalhotra added this to the 7.3.1 milestone Mar 28, 2019
@cheenamalhotra cheenamalhotra added this to Under Peer Review in MSSQL JDBC Apr 9, 2019
@cheenamalhotra cheenamalhotra merged commit 8acab59 into microsoft:dev Apr 11, 2019
MSSQL JDBC automation moved this from Under Peer Review to Closed/Merged PRs Apr 11, 2019
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.

None yet

8 participants