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

Added support to read SQL Warnings after ResultSet is read completely #785

Merged
merged 4 commits into from
Aug 25, 2018

Conversation

cheenamalhotra
Copy link
Member

As of now JDBC driver does not capture any warnings after ResultSet is fully consumed and the TDS_MSG tokens are not consumed but are left unused in Stream.

Use Case:
Capturing warnings at the end of ResultSet is helpful for PolyBase External Data tables where rows could get rejected due to bad column values. This information is sent by SQL Server after ResultSet data.

@codecov-io
Copy link

codecov-io commented Aug 24, 2018

Codecov Report

Merging #785 into dev will increase coverage by 0.02%.
The diff coverage is 52.94%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #785      +/-   ##
============================================
+ Coverage     48.16%   48.18%   +0.02%     
+ Complexity     2785     2782       -3     
============================================
  Files           116      116              
  Lines         27858    27862       +4     
  Branches       4639     4641       +2     
============================================
+ Hits          13418    13426       +8     
+ Misses        12230    12208      -22     
- Partials       2210     2228      +18
Flag Coverage Δ Complexity Δ
#JDBC42 47.65% <52.94%> (+0.05%) 2736 <0> (-3) ⬇️
#JDBC43 48.1% <52.94%> (+0.04%) 2776 <0> (-2) ⬇️
Impacted Files Coverage Δ Complexity Δ
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 32.54% <0%> (-0.2%) 254 <0> (-2)
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 52.23% <50%> (ø) 217 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 59.71% <66.66%> (-0.06%) 136 <0> (ø)
...ava/com/microsoft/sqlserver/jdbc/ScrollWindow.java 67.9% <0%> (-4.94%) 16% <0%> (-1%)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 45.05% <0%> (-4.4%) 16% <0%> (-1%)
...om/microsoft/sqlserver/jdbc/SimpleInputStream.java 51.47% <0%> (-1.48%) 11% <0%> (-1%)
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 60.25% <0%> (-0.44%) 87% <0%> (-1%)
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.6% <0%> (-0.12%) 262% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.55% <0%> (+0.23%) 0% <0%> (ø) ⬇️
... and 4 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 f415394...2bbfbdd. Read the comment docs.

// Clear out previous results
clearLastResult();
// Clear out previous results only when clearFlag = true
if (clearFlag)
Copy link
Contributor

Choose a reason for hiding this comment

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

add brackets even if there's only one line in the if statement

@@ -1368,7 +1368,7 @@ final void clearLastResult() {
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Modify javadoc now that clearFlag is a parameter

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@cheenamalhotra cheenamalhotra added this to the 7.1.0 milestone Aug 24, 2018
@cheenamalhotra cheenamalhotra merged commit 6580dee into microsoft:dev Aug 25, 2018
@cheenamalhotra cheenamalhotra changed the title Changes to read SQL Warnings after ResultSet is read completely Added support to read SQL Warnings after ResultSet is read completely Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants