Skip to content

Fixed issues in BulkCopy exception handling#801

Merged
ulvii merged 2 commits intomicrosoft:devfrom
ulvii:BulkCopyErrorHandling
Sep 24, 2018
Merged

Fixed issues in BulkCopy exception handling#801
ulvii merged 2 commits intomicrosoft:devfrom
ulvii:BulkCopyErrorHandling

Conversation

@ulvii
Copy link
Copy Markdown
Contributor

@ulvii ulvii commented Sep 15, 2018 •
edited
Loading

  1. Removing the catch block, because the driver should not blindly send attention packets. To cancel a query, attention packets should only be sent if the client has already sent a request, which has the last packet with EOM bit (0x01) set in status. If a complete request has not been sent to the server, the driver MUST send the next packet with both ignore bit (0x02) and EOM bit (0x01) set in the status to cancel the request (2.2.1.7 of TDS Technical Documentation). This is already properly handled in IOBuffer.execute(), which is the only method that calls doInsertBulk().
  2. Adding this.tdsMessageType == TDS.PKT_BULK, because prior to TDS.PKT_BULK, the driver has already sent INSERT BULK SQL statement to the server, which in case of an exception needs to be cancelled too.

@ulvii ulvii added this to the 7.1.1 milestone Sep 17, 2018
lilgreenbird
lilgreenbird reviewed Sep 19, 2018
View reviewed changes
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java Outdated
lilgreenbird
lilgreenbird reviewed Sep 19, 2018
View reviewed changes
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java
@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 19, 2018

Codecov Report

Merging #801 into dev will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##                dev    #801      +/-   ##
===========================================
+ Coverage     48.11%   48.2%   +0.08%     
+ Complexity     2783    2781       -2     
===========================================
  Files           116     116              
  Lines         27862   27853       -9     
  Branches       4641    4640       -1     
===========================================
+ Hits          13407   13426      +19     
+ Misses        12235   12203      -32     
- Partials       2220    2224       +4
Flag Coverage Δ Complexity Δ
#JDBC42 47.71% <100%> (+0.08%) 2738 <0> (-3) ⬇️
#JDBC43 48% <100%> (-0.03%) 2773 <0> (-2)
Impacted Files Coverage Δ Complexity Δ
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.58% <ø> (-0.08%) 262 <0> (-1)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 54.09% <100%> (+0.62%) 0 <0> (ø) ⬇️
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 32.38% <0%> (-0.31%) 253% <0%> (-3%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.25% <0%> (-0.18%) 0% <0%> (ø)
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 59.61% <0%> (-0.1%) 135% <0%> (-1%)
...ncurrentlinkedhashmap/ConcurrentLinkedHashMap.java 39% <0%> (+0.21%) 43% <0%> (ø) ⬇️
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 60.9% <0%> (+0.64%) 88% <0%> (+1%) ⬆️
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 45.73% <0%> (+1.96%) 107% <0%> (+1%) ⬆️
... and 1 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 0a1759f...a84e519. Read the comment docs.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 19, 2018

Codecov Report

Merging #801 into dev will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #801      +/-   ##
============================================
+ Coverage     48.11%   48.18%   +0.06%     
- Complexity     2783     2785       +2     
============================================
  Files           116      116              
  Lines         27862    27853       -9     
  Branches       4641     4640       -1     
============================================
+ Hits          13407    13422      +15     
+ Misses        12235    12211      -24     
  Partials       2220     2220
Flag Coverage Δ Complexity Δ
#JDBC42 47.71% <100%> (+0.08%) 2745 <0> (+4) ⬆️
#JDBC43 48.06% <100%> (+0.03%) 2775 <0> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.46% <ø> (-0.2%) 263 <0> (ø)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 53.72% <100%> (+0.26%) 0 <0> (ø) ⬇️
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 79.28% <0%> (-0.2%) 4% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.49% <0%> (+0.05%) 0% <0%> (ø) ⬇️
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 46.38% <0%> (+2.62%) 108% <0%> (+2%) ⬆️
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 46.15% <0%> (+3.29%) 16% <0%> (+1%) ⬆️

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 0a1759f...a84e519. Read the comment docs.

2 similar comments
@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 19, 2018

Codecov Report

Merging #801 into dev will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #801      +/-   ##
============================================
+ Coverage     48.11%   48.18%   +0.06%     
- Complexity     2783     2785       +2     
============================================
  Files           116      116              
  Lines         27862    27853       -9     
  Branches       4641     4640       -1     
============================================
+ Hits          13407    13422      +15     
+ Misses        12235    12211      -24     
  Partials       2220     2220
Flag Coverage Δ Complexity Δ
#JDBC42 47.71% <100%> (+0.08%) 2745 <0> (+4) ⬆️
#JDBC43 48.06% <100%> (+0.03%) 2775 <0> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.46% <ø> (-0.2%) 263 <0> (ø)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 53.72% <100%> (+0.26%) 0 <0> (ø) ⬇️
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 79.28% <0%> (-0.2%) 4% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.49% <0%> (+0.05%) 0% <0%> (ø) ⬇️
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 46.38% <0%> (+2.62%) 108% <0%> (+2%) ⬆️
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 46.15% <0%> (+3.29%) 16% <0%> (+1%) ⬆️

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 0a1759f...a84e519. Read the comment docs.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 19, 2018

Codecov Report

Merging #801 into dev will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #801      +/-   ##
============================================
+ Coverage     48.11%   48.18%   +0.06%     
- Complexity     2783     2785       +2     
============================================
  Files           116      116              
  Lines         27862    27853       -9     
  Branches       4641     4640       -1     
============================================
+ Hits          13407    13422      +15     
+ Misses        12235    12211      -24     
  Partials       2220     2220
Flag Coverage Δ Complexity Δ
#JDBC42 47.71% <100%> (+0.08%) 2745 <0> (+4) ⬆️
#JDBC43 48.06% <100%> (+0.03%) 2775 <0> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.46% <ø> (-0.2%) 263 <0> (ø)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 53.72% <100%> (+0.26%) 0 <0> (ø) ⬇️
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 79.28% <0%> (-0.2%) 4% <0%> (-1%)
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 62.49% <0%> (+0.05%) 0% <0%> (ø) ⬇️
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 46.38% <0%> (+2.62%) 108% <0%> (+2%) ⬆️
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 46.15% <0%> (+3.29%) 16% <0%> (+1%) ⬆️

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 0a1759f...a84e519. Read the comment docs.

peterbae
peterbae reviewed Sep 19, 2018
View reviewed changes
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
peterbae
peterbae approved these changes Sep 19, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@peterbae peterbae left a comment

Choose a reason for hiding this comment

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

I looked pretty vigorously into this to see if the changes break anything, but I couldn't find anything. lgtm

@ulvii ulvii merged commit ca7141f into microsoft:dev Sep 24, 2018
@ulvii ulvii deleted the BulkCopyErrorHandling branch September 24, 2018 19:44
@ulvii ulvii mentioned this pull request Oct 10, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cheenamalhotra cheenamalhotra cheenamalhotra approved these changes

+3 more reviewers

@rene-ye rene-ye rene-ye approved these changes

@lilgreenbird lilgreenbird lilgreenbird approved these changes

@peterbae peterbae peterbae approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

7.1.1

Development

Successfully merging this pull request may close these issues.

6 participants

@ulvii @codecov-io @rene-ye @lilgreenbird @cheenamalhotra @peterbae