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

Fixes issue with PreparedStatement.setBigDecimal() in the driver when no scale is passed #684

Merged
merged 9 commits into from
May 2, 2018

Conversation

cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Apr 23, 2018

Fix for issue #683.

Issue occurs when BigDecimal value is created from double [e.g. new BigDecimal(5.55)] or as it is exceeds in Precision than maxAllowedPrecision (38) in SQL Server, and no scale is passed from Client. The driver in this case fails to round off digits and passes BigDecimal value as String, which when received by SQL Server fails with Exception:
com.microsoft.sqlserver.jdbc.SQLServerException: Error converting data type nvarchar to decimal.

This fix makes sure BigDecimal values passed to SQL Server are within precision limits and scaled accordingly.

@cheenamalhotra cheenamalhotra mentioned this pull request Apr 23, 2018
@codecov-io
Copy link

codecov-io commented Apr 23, 2018

Codecov Report

Merging #684 into dev will decrease coverage by <.01%.
The diff coverage is 45.45%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #684      +/-   ##
============================================
- Coverage     48.13%   48.13%   -0.01%     
+ Complexity     2576     2573       -3     
============================================
  Files           113      113              
  Lines         26561    26569       +8     
  Branches       4435     4438       +3     
============================================
+ Hits          12786    12789       +3     
+ Misses        11647    11642       -5     
- Partials       2128     2138      +10
Flag Coverage Δ Complexity Δ
#JDBC42 48% <45.45%> (-0.05%) 2567 <0> (-4)
#JDBC43 48.1% <45.45%> (+0.05%) 2572 <0> (+2) ⬆️
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 63.41% <45.45%> (+0.23%) 0 <0> (ø) ⬇️
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 42.69% <0%> (-4.5%) 14% <0%> (-3%)
...n/java/com/microsoft/sqlserver/jdbc/Parameter.java 61.92% <0%> (-0.84%) 63% <0%> (ø)
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 44.96% <0%> (-0.45%) 107% <0%> (ø)
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 50.6% <0%> (-0.25%) 157% <0%> (ø)
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 59.51% <0%> (-0.11%) 131% <0%> (-1%)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 54.79% <0%> (+0.19%) 0% <0%> (ø) ⬇️
...om/microsoft/sqlserver/jdbc/SimpleInputStream.java 52.59% <0%> (+1.48%) 12% <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 4a9c840...b9062ba. Read the comment docs.

Copy link
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 would put the if statement at line 2215 in a bracket (we should apply brackets to if statements even if it only has one line).

Otherwise looks good to me. We won't have to worry about the line 2216 turning up negative since SQLServerConnectionmaxDecimalPrecision will always be larger than bigDecimalValue.precision().

@cheenamalhotra cheenamalhotra added this to Under Peer Review in MSSQL JDBC Apr 24, 2018
@cheenamalhotra cheenamalhotra added this to the 6.5.2 milestone Apr 28, 2018
Integer inScale = dtv.getScale();
if (null != inScale && inScale != bigDecimalValue.scale())
bigDecimalValue = bigDecimalValue.setScale(inScale, RoundingMode.DOWN);
Integer dtvScale, biScale = bigDecimalValue.scale();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please apply the formatter, otherwise looks good!

@cheenamalhotra cheenamalhotra merged commit 55bc4a0 into microsoft:dev May 2, 2018
MSSQL JDBC automation moved this from Under Peer Review to Closed/Merged PRs May 2, 2018
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

5 participants